Gabriel39 commented on issue #66497: URL: https://github.com/apache/doris/issues/66497#issuecomment-5211673675
Thanks for the v3 revision. After checking the confirmed 4.2 release boundary and re-reading the baseline implementation in #65730, I need to correct part of my previous review and narrow the proposal. There are two confirmed product constraints: 1. **Doris 4.2 is based on branch-4.1, not master.** Therefore this work should target branch-4.1. There is no need for a master-first or dual-track implementation in this issue. 2. **No functionality that requires extending lance-c will be included in this release.** Such work is explicitly outside the 4.2 scope. These constraints materially change the appropriate Phase-1 design. ## Query syntax: keep the existing vector_search() contract I rechecked #65730. The baseline deliberately exposes Lance vector search only through the `vector_search()` TVF: https://github.com/apache/doris/pull/65730 The implementation and regression tests use: ~~~sql SELECT row_id, _distance FROM vector_search( "table" = "catalog.db.table", "column" = "embedding", "query_vector" = "[0,0,0,0]", "metric" = "l2", "top_k" = "5" ); ~~~ - Implementation: https://github.com/apache/doris/blob/e3289c1a5df7558cb8e63d80379d4edebf9c498c/fe/fe-core/src/main/java/org/apache/doris/tablefunction/VectorSearchTableValuedFunction.java - Regression coverage: https://github.com/apache/doris/blob/e3289c1a5df7558cb8e63d80379d4edebf9c498c/regression-test/suites/external_table_p0/lance/test_lance_vector_search.groovy There is no implementation or stated plan in the baseline to translate internal-table ANN query forms such as `l2_distance_approximate(...)` or `inner_product_approximate(...)` into a Lance search. Therefore: - Lance queries should continue to use `vector_search()`. - This index-management issue should not add compatibility with the internal-table ANN query syntax. - Reusing `USING ANN` as a category in CREATE INDEX is only DDL/parser reuse; it does not imply query-syntax reuse. - For Q2, I now recommend the Lance-native property already used by the existing query surface: `"metric" = "l2|cosine|dot"` (option **c**), rather than introducing `metric_type` with internal-table values. Query behavior is relevant to this issue only as end-to-end verification: an index created by the lifecycle API must be consumed correctly by the existing `vector_search(..., "use_index"="true")` path, with a compatible metric and correct results. This issue should not redesign the query syntax or optimizer rules. ## Scope the implementation to the existing lance-c 0.1.2 APIs The pinned lance-c 0.1.2 already exposes the one-shot lifecycle APIs required by the original issue: - `lance_dataset_create_vector_index` - `lance_dataset_create_scalar_index` - `lance_dataset_drop_index` - `lance_dataset_index_count` - `lance_dataset_index_list_json` https://github.com/lance-format/lance-c/blob/v0.1.2/include/lance/lance.h#L490-L541 The 4.2 design should use those existing APIs, with the heavy direct-Dataset operation executed on a selected BE for Directory Catalog, and use the existing Namespace operations for the supported REST cases. The limitations of the one-shot API—no native progress reporting, cooperative cancellation, distributed fragment build, or job-specific transaction provenance—should be documented honestly rather than solved by expanding lance-c in this release. The following v3 items depend on new lance-c capabilities and should be removed from the 4.2 implementation scope: - fragment-scoped distributed builds; - shared IVF centroid/PQ-codebook distribution to BE workers; - BE-to-FE uncommitted segment metadata transfer; - coordinator commit of worker-built segments; - physical segment merge; - native progress and cooperative cancellation; - per-segment coverage/statistics extensions; - incremental BUILD over uncovered fragments. In particular, the original #66497 scope requires replacing or rebuilding an index with the same name. The existing `replace=true` path provides a full rebuild. Incremental `BUILD INDEX` was added during review, but it is not required by the original issue and should be deferred because the intended distributed/incremental implementation requires APIs outside the 4.2 boundary. ## Revised Phase-1 lifecycle For 4.2, I recommend limiting the lifecycle to: - `CREATE INDEX` for the verified vector and scalar types; - `CREATE OR REPLACE INDEX` mapped to the existing full-build `replace=true` behavior; - `SHOW INDEX`, using the existing list/count metadata and grouping physical entries by logical name where necessary; - `DROP INDEX`; - restricted REST behavior based only on the pinned Namespace request/response models; - metadata refresh, privileges, version rules, concurrency limitations, and failure behavior appropriate to these existing one-shot operations. Do not promise a reliable native CANCEL operation if the underlying call cannot be interrupted. Likewise, SHOW should not promise coverage fields that require a new lance-c API. UUID, columns, type, and dataset version are already available and satisfy the original metadata scope. The query-side acceptance test should be limited to proving that a created IVF_PQ index is used by the existing `vector_search()` path and that scalar indexes are consumed by the existing Lance predicate path. FTS SQL design, internal ANN query syntax, distributed recall/model strategy, and future physical merge behavior are outside this issue. ## Answers to the v3 questions - **Q1:** target **branch-4.1 only**. This is the confirmed base for Doris 4.2. - **Q2:** use the existing Lance-facing `metric` key and values (`l2`, `cosine`, `dot`), matching `vector_search()`. Do not require internal ANN query/property syntax compatibility. - **Q3:** lance-c extensions are not prerequisites for this version; they should be tracked as future work. Any feature that requires such an extension, including per-segment coverage, distributed BUILD, progress, cancellation, or segment merge, must be deferred rather than gating #66497. With these scope corrections, v3 should be simplified substantially. The goal of this issue is index lifecycle management using the capabilities already present in the 4.1 baseline and pinned SDKs, not a distributed index-build framework or a unified internal/external vector-query syntax. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
