mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 18:25:44 +00:00
master
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
5def5ced4a |
enhance: enforce function-field binding principle for function DDL (#51360)
## What Enforce a function–field binding principle for function DDL (add / drop / alter function on an existing collection), so a function and its output field stay coupled and already-stored vectors can never be silently invalidated by DDL. BM25 and MinHash follow the strict binding: add/drop only via the unified `add_function_field` / `drop_function_field`, output field always coupled. **TextEmbedding is a temporary carve-out** — its legacy `AddCollectionFunction` / `DropCollectionFunction` paths are retained, so attach-over-existing-field and detach are still possible for it, because `add_function_field` embedding backfill is not yet implemented. It will be folded into the unified path in a follow-up. So the "always coupled" invariant currently holds for BM25/MinHash, not TextEmbedding. ## Changes - **AlterCollectionSchema invariant**: reject standalone add-function (a function must be added together with its new output field), reject detaching a function without dropping its output field, and reject function cascade (a new function's input being another function's output). - **Legacy RPCs**: `AddCollectionFunction` / `DropCollectionFunction` are rejected for BM25/MinHash (must use `add_function_field` / `drop_function_field`) and retained only for TextEmbedding (see carve-out). `AlterCollectionFunction` is kept and gains the whitelist below. In the alter and legacy-add paths, function field IDs are always re-derived from field names (never trusted from the request), so a request cannot inject an unrelated field ID that a later `drop_function_field` would delete. - **alter_function whitelist**: only connection/runtime params may change (TextEmbedding: `url`, `credential`, `timeout_ms`, `max_client_batch_size`, `region`, `location`, `projectid`, `user`); BM25/MinHash have no alterable params. Function identity (type, name, input/output fields) and output-shaping params (`dim`, `model_name`, `endpoint` — the TEI model identity, `normalize`, `truncate*`, prompts, ...) are immutable. Params are normalized (keys lowercased, duplicate keys rejected) so a crafted duplicate key cannot bypass the diff. - **drop_function_field**: allow any output-producing function (dropping needs no backfill), removing the previous BM25/MinHash-only restriction. ## Not in scope / follow-up - Extending `add_function_field` to TextEmbedding (post-creation add would require backfilling every existing row through the external embedding model) — deferred; folding the TextEmbedding legacy paths into the unified binding follows this. - MinHash input-field analyzer immutability lives on a different DDL path (`AlterCollectionField`); tracked as a separate follow-up. ## Breaking changes - `DropCollectionFunction` on a **MinHash** function (detach — remove the function but keep its output field) is no longer supported; use `drop_function_field` instead (drops the function together with its output field). If a released version supported MinHash detach, migrate any such call to `drop_function_field`. issue: #51348 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
b16c821b36 |
enhance: carry TEXT LOB refs through schema-bump full-rewrite compaction (#51125)
### What & why `bump_schema_version`'s full-rewrite path (`runFullSchemaRewrite`, reached only for a **drop-field** schema bump) builds a from-scratch output manifest via `NewBinlogRecordWriter` and did **not** carry the source segment's TEXT LOB references — leaving dangling refs for an out-of-line (`>=64KB`) TEXT column. This was the `TODO(#50021)`. ### What this does Wire **REUSE_ALL** LOB handling into `runFullSchemaRewrite`, mirroring sort compaction. Both are `1->1` with a single output manifest, so REUSE_ALL is always correct: a schema bump never changes the existing TEXT LOB data — only its references are carried. - `internal/compaction/lob_compaction.go`: `GetForcedStrategy` forces REUSE_ALL for `BumpSchemaVersionCompaction`. - `internal/datanode/compactor/bump_schema_version_compactor.go`: collect the source LOB files into a `LOBCompactionContext`, pass `storage.WithTextRefsAsBinary()`, update per-LOB-file `valid_rows` (`SetSegmentRowStats`), and merge the LOB references into the output manifest (`applyLOBCompaction`) **before** building the text-match index — `createTextIndex` reads the TEXT column through this manifest, so the carried LOB files must already be referenced (matches sort/mix ordering). Removed the `TODO(#50021)`. - Unit tests: forced strategy, init/apply LOB wiring, the applyLOBCompaction-before-createTextIndex ordering (`TestFullRewriteMergesLOBRefsBeforeBuildingTextIndex`), and a real add-nullable-TEXT + drop-field full-rewrite regression driving the actual packed writer (`TestFullRewriteFillsMissingNullableTextAsBinary`). ### Also: guard add_function_field behind StorageV3 (#51167) Adding a function to an existing collection must backfill the new output field into **pre-existing** sealed segments; that backfill runs through `bump_schema_version` compaction, which only works on a StorageV3 segment. A pre-existing V2 segment is only backfilled after the storage-version upgrade compaction rewrites it to V3, and that upgrade runs only when **both** `common.storage.useLoonFFI` and `dataCoord.compaction.storageVersion.enabled` are on. The proxy now rejects add-function unless both are enabled (`validateAddFunctionRequiresStorageV3`, wired into `addCollectionFunctionTask` and `alterCollectionSchemaTask`). `create_collection` with a function is unaffected. Full reasoning in #51167. ### Notes - Reuses the LOB machinery from **#50784**. - The `GenerateEmptyArrayFromSchema` binary-TEXT production fix landed independently on master via **#51124**; this PR keeps the regression test guarding the bump path. issue: #50021, #51167 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
sijie-ni-0214andGitHub
|
ea76e2a7ec |
fix: support alter schema add request validation (#50118)
## Summary - Allow AlterCollectionSchema AddRequest to carry a field-only add request while keeping the added field as a non-function output. - Support function-only add requests against existing output fields, and validate the merged function schema through the shared function validator. - Keep the BM25 field-plus-function path explicit, reject invalid BM25 add forms, and normalize TIMESTAMPTZ default values before broadcasting field-only schema changes. issue: #50119 ## Test Plan - [x] source scripts/setenv.sh && go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${MILVUS_WORK_DIR}/cmake_build/lib -r ${MILVUS_WORK_DIR}/internal/core/output/lib" ./internal/proxy -run 'TestAlterCollectionSchemaTask($|_)' -timeout 300s - [x] gofumpt -l internal/proxy/function_task.go internal/proxy/function_task_test.go internal/proxy/task.go internal/proxy/task_test.go internal/proxy/util.go internal/proxy/util_test.go internal/rootcoord/ddl_callbacks_alter_collection_schema.go internal/rootcoord/ddl_callbacks_alter_collection_schema_test.go internal/util/function/validator/validator.go - [x] git diff --check upstream/master..HEAD - [ ] source scripts/setenv.sh && go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${MILVUS_WORK_DIR}/cmake_build/lib -r ${MILVUS_WORK_DIR}/internal/core/output/lib" ./internal/rootcoord -run 'TestDDLCallbacksBroadcastAlterCollectionSchema|TestDDLCallbacksAlterCollectionSchemaAddSkipsSchemaDropReady' -timeout 300s (blocked locally at link time by stale C++/Loon symbols such as _FreeCFieldMemSizeList and _loon_segment_writer_* in local core libraries) Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
wei liuandGitHub
|
7bdd40d692 |
enhance: [ExternalTable Part10] enable function output fields on external collections (#49307)
issue: #45881 ## Summary Part 10 of the External Table series. Enables BM25 / MinHash / TextEmbedding function output fields on external collections, and makes external-table text_match work with persisted text indexes. Related: [#45881](https://github.com/milvus-io/milvus/issues/45881) (External Collection Lakehouse Integration tracking). ### What's in this PR **Schema & segcore** - Allow `Function` declarations on external schemas; function-output fields skip `external_field_mapping` validation - `Schema` tracks function-output field IDs; `ChunkedSegmentSealedImpl` and `ManifestGroupTranslator` resolve function-output columns by field name - Fast paths for retrieve / search load function-output columns by field name; external columns continue using `external_field_mapping` - `Util.cpp::GetFieldDatasFromManifest` resolves function-output columns by field name so indexbuilder reads the correct packed column **Refresh pipeline (format-agnostic via loon FFI)** 1. `CreateSegmentManifestWithBasePath` creates the input manifest that references external original files as CG0 2. `FFIPackedReader(v1)` streams input columns into `InsertData` via the shared `ArrowRecordToInsertData` helper 3. `embedding.RunAll` populates function-output fields in-place 4. `FFIPackedWriter` writes output fields as a new column group on top of v1 5. `AddStatsToManifest` registers BM25 stats into the final manifest 6. Memory peak stays around one Arrow batch, defaulting to 64 MiB **RootPath isolation** - Function-output input manifests, output manifests, BM25 stats, and text index artifacts now use the same `RootPath/insert_log/<collection>/<partition>/<segment>` layout as normal external table StorageV3 manifests - This avoids writing function-output artifacts under bucket-root `external/...` paths when multiple clusters share the same bucket **Text match** - External text fields with `enable_match` persist text index artifacts during refresh and load them through the regular external segment path - English and Chinese analyzer coverage are both included in the E2E test **Cross-bucket** - `NewPackedFFIReaderWithManifest` accepts `ExternalReaderContext` and injects per-collection `extfs.{collID}.*` aliases when `external_source` is set. Existing callers pass `{}` and are unaffected **VirtualPK** - `VirtualPKChunkedColumn` implements `GetChunk` / `GetAllChunks` so proxy requery filter-by-PK works for external collections with virtual primary keys **Shared embedding runner** - New `internal/util/function/embedding/runner.go` provides canonical `RunAll(ctx, schema, data, opts)` shared by import and external-table refresh paths - Supports BM25 output vector types including Float, BFloat16, Float16, Binary, and Sparse ### Test Plan - [x] Go unit tests for `internal/datanode/external` with 99.8% package coverage - [x] Go unit tests for changed import, packed, embedding, and schema helpers from the existing branch validation - [x] C++ rebuild + `milvus_storage` / `milvus_core` lib install from the existing branch validation - [x] E2E function-output tests in `tests/go_client/testcases/external_table_function_test.go` - `TestExternalTableBM25Function` - `TestExternalTableMinHashFunction` - `TestExternalTableTextEmbeddingFunction` - [x] `TestExternalTableTextMatch` with 10 files x 5000 rows = 50000 rows, English and Chinese text fields, persisted text index object checks in MinIO, load readiness check, and query correctness checks Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
congqixiaandGitHub
|
7311d450a0 |
enhance: bump Go dependencies to v3 modules (#49485)
Related to #49398 Bump Go module references from pkg/v2 and milvus-proto/go-api/v2 to pkg/v3 and milvus-proto/go-api/v3 so the client tracks the Milvus 3.x release line. This prepares the repository for the upcoming 3.x.y release by aligning imports, module dependencies, and proto API references with the new major-version module paths. --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
|
87b04e8a63 |
fix: remove golangci-lint v2 exclusion rules and fix ~1500 lint violations (#48586)
## Summary Remove all temporary exclusion rules added during the golangci-lint v1→v2 upgrade (PR #48286), fixing ~1500 lint violations: - **QF series (~1080)**: auto-fixed with `--fix` (embedded field selectors, if/else→switch, De Morgan's law) - **ST1005 (148)**: lowercase error strings per Go conventions - **gosec (116)**: fix G118 context cancel leaks, G306 file permissions; nolint for G602/G120/G705 false positives - **revive (41)**: `Json`→`JSON`, `Url`→`URL` naming conventions - **ineffassign (14)**: remove unused assignments - **unconvert (13)**: remove unnecessary type conversions - **depguard (9)**: replace banned `errors` import with `cockroachdb/errors` - **gocritic (14)**: fix ruleguard violations - **Other staticcheck (12)**: S1034, S1008, SA3001 etc. Kept disabled: govet `printf` analyzer (known Go 1.25 + golangci-lint v2 panic bug) issue: #48574 pr: #48286 ## Test plan - [x] `golangci-lint run` passes with 0 issues locally - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Li Liu <li.liu@zilliz.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
junjiejiangjjjandGitHub
|
102481e53f |
feat: Support add_function/alter_function/drop_function (#44895)
https://github.com/milvus-io/milvus/issues/44053 Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com> |