[BUG] Use InternalUpdateConfiguration in Rust, correctly update configuration in go

[jairad26]

Description of changes

Previously, the go code did not update the configuration str for collections because it was not passed as part of generateCollectionUpdatesWithoutID

This was fixed, and tests were added. It also cleans up the existing go types, and updates the rust types to use a similar InternalCollectionConfiguration for updates.

Test plan

How are these changes tested?

Tests were added to validate that both the modify worked, and on future fetches maintains the values. Db tests were also added to the dao

• [x ] Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?

[jairad26]

• [BUG] Use InternalUpdateConfiguration in Rust, correctly update configuration in go #5069 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[propel-code-bot]

Fix Collection Configuration Updates and Unify Config Types Across Go and Rust

This PR addresses an issue in the Go implementation where collection configuration updates (specifically, ConfigurationJsonStr) were not persisted during modifications. It remedies this, enhances the collection update logic in Go, and brings Go/Rust type and update logic into closer alignment by introducing InternalUpdateCollectionConfiguration in Rust and refactoring related code paths. Additional tests have been added to Go and Rust to ensure that configuration updates, round-tripping, and persistence behave as expected.

Key Changes

• Go: Fixes logic in generateCollectionUpdatesWithoutID to ensure ConfigurationJsonStr is updated when modifying a collection.
• Go: Cleans up collection configuration/type structure (VectorIndexConfiguration and EmbeddingFunctionConfiguration) to match Rust conventions; type fields are removed, and configuration structurally normalized.
• Go: Expands configuration update test coverage in table_catalog_test.go and collection_test.go (multiple new tests)
• Rust: Adds type InternalUpdateCollectionConfiguration for consistent update application, deprecating direct use of UpdateCollectionConfiguration for update logic.
• Rust: Refactors the collection_configuration.rs update logic to use InternalUpdateCollectionConfiguration, ensuring update/merge semantics are clear and match the Go side.
• Rust: Updates all relevant API, sysdb, and Python bindings interfaces to use InternalUpdateCollectionConfiguration for updates.
• Rust: Adds extensive test coverage for configuration merging (including edge cases for HNSW, SPANN, and embedding functions).
• Go/Rust: Unifies configuration update semantics and brings test coverage and field-level mapping between the two implementations.

Affected Areas

• Go: sysdb/coordinator/model/collection_configuration.go
• Go: sysdb/coordinator/table_catalog.go and table_catalog_test.go
• Go: sysdb/metastore/db/dao/collection.go and collection_test.go
• Rust: types/src/collection_configuration.rs, types/src/api_types.rs, types/src/spann_configuration.rs
• Rust: sysdb (sysdb.rs, sqlite.rs), frontend/server.rs, python_bindings
• Rust/Go: test coverage in both codebases
• Python: Integration tests for configuration persistence (test_collection_configuration.py)

This summary was automatically generated by @propel-code-bot

[propel-code-bot]

[BestPractice]

The logic to re-fetch a collection and verify its configuration is duplicated in this test and also in test_configuration_spann_updates and test_spann_update_from_json. To improve maintainability and reduce code duplication, consider extracting this verification logic into a helper function.

For example:

def _verify_config_persistence(client: ClientAPI, collection_name: str, index_type: str, expected_params: Dict[str, Any]) -> None:
    coll = client.get_collection(name=collection_name)
    loaded_config = coll.configuration_json
    assert loaded_config and isinstance(loaded_config, dict)
    
    index_config = loaded_config.get(index_type, {})
    assert isinstance(index_config, dict)

    for key, value in expected_params.items():
        assert index_config.get(key) == value

This would make the tests cleaner and more maintainable.

[sanketkedia]

when is this used v/s updateCollectionConfiguration in table_catalog.go? Why do we need this if sysdb already does a Read modify write

[jairad26]

this is used for sqlite sysdb. in that, it does the same logic of reading, then updating the config if applicable

[sanketkedia]

Just to confirm, sqlite did not have this bug right?

[HammadB]

what is the difference between the internal type and not? a comment would be useful

[HammadB]

Maybe we could have more tests on the rust and go update paths