Skip to content

feat: add support for alter schema drop vector index - #1991

Open
antas-marcin wants to merge 7 commits into
mainfrom
alter-schema-drop-vector-index
Open

feat: add support for alter schema drop vector index#1991
antas-marcin wants to merge 7 commits into
mainfrom
alter-schema-drop-vector-index

Conversation

@antas-marcin

@antas-marcin antas-marcin commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add delete_vector_index(vector_name) method to collection config, allowing users to drop a named vector's index via DELETE /v1/schema/{className}/vectors/{vectorIndexName}/index
  • Follows the same pattern as the existing delete_property_index method
  • Includes both sync and async type stubs

Closes #1990

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds client support for dropping a named vector’s index via the schema REST endpoint, mirroring the existing “delete property index” capability in the collections config API.

Changes:

  • Added delete_vector_index(vector_name) to the collection config executor, issuing DELETE /v1/schema/{className}/vectors/{vectorIndexName}/index.
  • Added sync and async type stubs for delete_vector_index(...).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
weaviate/collections/config/executor.py Implements the new delete-vector-index operation via the config executor.
weaviate/collections/config/sync.pyi Exposes the new method in the sync config type stub.
weaviate/collections/config/async_.pyi Exposes the new method in the async config type stub.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread weaviate/collections/config/executor.py Outdated
Comment thread weaviate/collections/config/executor.py Outdated
Comment thread weaviate/collections/config/executor.py
Comment thread weaviate/collections/config/executor.py
@jfrancoa
jfrancoa changed the base branch from main to bump-integration-tests-1.39 July 22, 2026 11:25
@jfrancoa
jfrancoa force-pushed the alter-schema-drop-vector-index branch from d728dd9 to 7e5cd7f Compare July 22, 2026 11:25
@jfrancoa
jfrancoa requested a review from a team as a code owner July 22, 2026 11:25

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread weaviate/collections/classes/config_methods.py Outdated
Comment on lines +39 to 42
NONE: The index of this vector has been dropped, see ``collection.config.delete_vector_index``.
The vector data is still stored, but it cannot be searched. This value is reported by the
server only, it cannot be used to configure a vector.
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving as-is for now: no public Configure.VectorIndex.* factory emits NONE (only the internal create models reference the field), and the server rejects "none" on both create and update, so the exposure is constructing private classes directly. Can add a validator in a follow-up if users actually hit it.

@jfrancoa
jfrancoa force-pushed the alter-schema-drop-vector-index branch from 7e5cd7f to 4e2a580 Compare July 22, 2026 11:53
@jfrancoa
jfrancoa force-pushed the bump-integration-tests-1.39 branch from 736a7cd to be6c8dc Compare July 22, 2026 12:05
@jfrancoa
jfrancoa force-pushed the alter-schema-drop-vector-index branch from 4e2a580 to 2dbfaed Compare July 22, 2026 12:07
@bevzzz

bevzzz commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

thought: Do you think this PR could do with fewer tests? The new endpoint is very simple and it seems to me like one "happy path" test will catch most things.

Other tests verify Weaviate's behavior more than client's logic, and some may add up to 30s of pipeline time.

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@jfrancoa

jfrancoa commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@bevzzz fair point — slimmed in b92dbb7, keeping the coverage but moving it down the pyramid: the integration test is now a single happy-path journey (create → drop → poll → sibling index intact + searchable). The unknown-name case moved to the mock suite (422 → UnexpectedStatusCodeError), the invalid-input case was already covered there and its integration duplicate is deleted, and the list_all parsing assertion was already pinned by the parser unit tests. On the 30s: that's the failure-path bound of the poll — in the green path it returns as soon as the drop converges (~1-2s for a 1-object collection), so it only costs pipeline time when the feature is genuinely broken.

@jfrancoa

jfrancoa commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@copilot review

@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.55%. Comparing base (95b5d76) to head (6a7ad49).
⚠️ Report is 81 commits behind head on main.

Files with missing lines Patch % Lines
integration/test_collection_config.py 84.61% 4 Missing ⚠️
weaviate/collections/classes/config.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1991      +/-   ##
==========================================
+ Coverage   86.64%   88.55%   +1.91%     
==========================================
  Files         300      304       +4     
  Lines       23172    23597     +425     
==========================================
+ Hits        20077    20897     +820     
+ Misses       3095     2700     -395     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jfrancoa
jfrancoa force-pushed the alter-schema-drop-vector-index branch from 9d7b916 to ccad4cc Compare August 7, 2026 10:52
antas-marcin and others added 2 commits August 25, 2026 13:15
Follow-up to the `delete_vector_index` support, addressing review findings.

After a successful drop, Weaviate keeps the vector in the schema as
`vectorIndexType: "none"` with no `vectorIndexConfig`. The client asserted that
every named vector has an index config, so `collection.config.get()` and
`client.collections.list_all()` raised `AssertionError` for every collection in
the cluster once any vector index had been dropped. `_NamedVectorConfig.
vector_index_config` is now optional, `VectorIndexType` gained a server-reported
`NONE` member and `to_dict()` round-trips it.

`collection.config.update()` on a dropped vector raised a bare
`KeyError: 'vectorIndexConfig'` from the schema merge. Both the current and the
deprecated merge paths now go through one helper that raises a
`WeaviateInvalidInputError` explaining that a dropped index cannot be re-created.

The docstring claimed the index could be regenerated and that a missing vector
raises `WeaviateInvalidInputError`. Neither is true: Weaviate rejects
re-creating a dropped index, and an unknown vector name comes back as a 422.
It now also documents that the endpoint is experimental and needs
`ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true`, that only
named vectors can be dropped, and that the drop is applied asynchronously. The
error message no longer blames a missing vector for what is usually a disabled
endpoint.

Tests: unit coverage for parsing, exporting and updating a dropped vector, mock
coverage for the request path and the disabled-endpoint response, and
integration coverage gated at 1.39.0. The CI compose file enables the
experimental endpoint; that flag can be dropped once 1.39.0 is GA.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jfrancoa and others added 2 commits August 25, 2026 13:16
- Replace the assert on dropped-vector schema shape with an explicit
  SchemaValidationError so a named vector missing vectorIndexConfig fails
  fast even under python -O; pinned by a new parser unit test
- Slim the integration test to the happy path: the unknown-name error
  contract moved to the mock suite (422 -> UnexpectedStatusCodeError) and
  the redundant list_all/invalid-input assertions are covered by the
  existing unit and mock tests

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single-vector (non-named) collection whose vector index is dropped with
`collection.config.delete_vector_index` comes back from the server with no
top-level `vectorizer` (and no `vectorConfig`, `vectorIndexType` or
`vectorIndexConfig`). `__get_vectorizer` accessed `schema["vectorizer"]`
unguarded and raised `KeyError: 'vectorizer'`, so both `config.get()` and
`collections.list_all()` crashed on such a collection. Return `None` when the
key is absent, matching how a dropped named vector yields
`vector_index_config is None`. Add parser tests for both entry points.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jfrancoa
jfrancoa force-pushed the alter-schema-drop-vector-index branch from ccad4cc to 16ea348 Compare August 25, 2026 11:17
Verified against the Weaviate server source that these comments were wrong:

- The endpoint is still experimental and off by default in current main; it is
  not enabled by default at 1.39.0 GA. The server rejects the drop unless
  ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true
  (usecases/schema/property.go:255). Fix the misleading ci/docker-compose.yml
  comment.

- A legacy single-vector collection cannot reach the "no vectorConfig, no
  vectorizer" shape: the server rejects dropping its index because
  len(class.VectorConfig) == 0 (property.go:288), and setClassDefaults always
  forces a non-empty top-level vectorizer for legacy classes (class.go:750).
  That shape can only come from a named-vector collection whose vectors were all
  dropped. Reword the __get_vectorizer guard comment and rename the parser tests
  accordingly. Behavior and assertions are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@dirkkul dirkkul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few smaller things, none of them blocking:

  • the delete_vector_index docstring goes stale as soon as the endpoint is promoted or the status code changes: it names ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true and says the server answers 500 without it. someone reading it after that sets an env var the server no longer looks at. could we say the endpoint is experimental and may be disabled server-side, without naming the flag or the status code?

  • in test/collection/test_config_methods.py, test_collection_config_simple_from_json_with_dropped_vector_index and test_collection_config_simple_from_json_all_vectors_dropped parse the same schemas as the two non-simple tests above them - __get_vector_config(schema, simple) never reads simple, and __get_vectorizer takes no such parameter. the simple path does build a different dataclass, so could we keep one of the two as the list_all() guard and drop the other?

  • test_delete_vector_index_endpoint_disabled overlaps the 422 case already inside test_delete_vector_index - both assert that a non-OK status comes back as UnexpectedStatusCodeError carrying the status code. the one thing it adds is that the server's error message reaches the exception, so could that move into test_delete_vector_index as another status/message case instead of standing up its own mock server?

reviewed with claude code, I went through every comment below myself

update.merge_with_existing(schema)


def test_updating_vector_next_to_dropped_vector_index() -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a test that should be in Weaviate and not the python client

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's basically testing a python functionality, the merge_with_existing. If this test would go as a go acceptance test it wouldn't invoke the same logic imho. It's not even using a Weaviate instance.

Comment thread weaviate/collections/classes/config.py Outdated
Comment thread weaviate/collections/classes/config_methods.py
jfrancoa and others added 2 commits August 27, 2026 12:45
- config.py: __existing_vector_index_config raised a raw KeyError when the
  collection had no vectors left (server omits vectorConfig once every named
  vector is dropped). Guard the key so the intended WeaviateInvalidInputError is
  raised instead. Add a regression test.

- config_methods.py: __get_vector_config reported "no vectorIndexConfig" for a
  vectorIndexType the client does not know, even though the config was present
  (an older client against a newer server). Branch on "vectorIndexConfig" in the
  named vector and give the unknown-type case its own message. Add a regression
  test.

- executor.py: delete_vector_index no longer names the env flag or the 500 status
  in its docstring (both go stale when the endpoint is promoted); it now returns
  None instead of a bool that could only ever be True. Regenerate stubs.

- tests: fold the disabled-endpoint case into test_delete_vector_index (its only
  unique check is that the server message reaches the exception) and drop the
  redundant simple-parser test; the all-vectors-dropped simple test remains the
  list_all() guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The round-4 commit changed delete_vector_index to return None but only updated
the mock test and stubs; the integration assertion still expected True and would
fail on every >=1.39 CI job.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for delete vector index operation

6 participants