[feature](lance) Support SHOW INDEX for Lance Directory tables - #66637
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
|
@u70b3 the new added regression test failed,please take a look |
|
Thanks for pointing this out. The issue has been fixed and pushed in commit |
0ef748c to
d327255
Compare
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
|
run buildall |
fd01824 to
3304d2d
Compare
|
@yiguolei @zhangstar333 friendly ping 🙂 Could one of you help approve the pending CI workflows on this PR when convenient? Seven checks (License Check, Code Formatter, FE Code Style Checker, Dependency License Review, BE UT macOS, Build Third Party Libraries, Build Extensions) are waiting on maintainer approval. The regression failure flagged earlier has been fixed and verified in a clean environment. Thanks a lot! |
|
/review |
There was a problem hiding this comment.
Requesting changes for four correctness gaps in the new Lance SHOW INDEX path.
Critical checkpoints:
- Correctness and compatibility: internal-table behavior and the four SHOW aliases remain intact, but valid nested-field indexes are rejected, PQ/HNSW properties are silently discarded, and valid Lance system indexes can abort the all-index description.
- Privilege, security, and lifecycle: SHOW privilege is checked before catalog I/O; table-vended credential overlay/redaction, one-Dataset snapshot consistency, allocator ownership, and cleanup look sound. The REST unsupported gate is too late and can perform namespace/table I/O first.
- Tests and scope: the new tests cover flat schemas, formatting, aliases, permissions, and basic errors, but they encode flattened provider JSON and nested-ID rejection, and miss system-plus-user-index and unreachable-REST routes. No additional user focus was supplied.
- Review completeness: the full 13-file authoritative diff and the relevant exact upstream version contracts were reviewed. The second normal/risk-focused round converged with no new non-duplicate findings. Builds/tests were not run under the review constraints.
| private static final int MAX_EXTERNAL_STRING_BYTES = 1024; | ||
| private static final int MAX_PROPERTIES_BYTES = 400; | ||
|
|
||
| private static final Set<String> PROPERTY_ALLOWLIST = ImmutableSet.of( |
There was a problem hiding this comment.
[P1] Parse the SDK's nested vector details. In the pinned Lance version, IVF-PQ details are serialized with a nested compression object, and HNSW settings are nested under hnsw; none of the flattened keys in this allowlist are emitted. Consequently those objects are silently dropped—the new regression already shows only metric_type although its fixture creates PQ with 8 bits and 2 sub-vectors. Please parse a bounded allowlist within the actual nested objects and test with the SDK-shaped JSON.
There was a problem hiding this comment.
Fixed in 8b58182. normalizeProperties now parses the SDK's actual nested shape: a top-level allowlist (metric_type, target_partition_size) plus bounded nested allowlists for compression (type/num_bits/num_sub_vectors/rotation_type) and hnsw (construction_ef/max_connections/max_level). The regression now asserts the nested form, e.g. {"compression":{"num_bits":4,"num_sub_vectors":4,"type":"pq"},"metric_type":"L2"}.
| try (Dataset dataset = Dataset.open().allocator(allocator).uri(datasetUri) | ||
| .readOptions(LanceReadOptions.build(javaStorageOptions, OptionalLong.empty())).build()) { | ||
| Map<Integer, String> topLevelFieldNames = new HashMap<>(); | ||
| for (LanceField field : dataset.getLanceSchema().fields()) { |
There was a problem hiding this comment.
[P1] Resolve valid nested field IDs. Lance indexes can target nested fields, and describeIndices() returns the indexed leaf ID, but this map contains only schema roots, so normalization later rejects the ID and fails SHOW INDEX for the entire table. The pinned Lance tag exposes LanceField.getChildren() and constructs canonical escaped paths from field ancestry (including names containing dots). Please recurse the schema into a complete ID-to-path map and replace the negative nested-ID test with a real nested-index case.
There was a problem hiding this comment.
Fixed in 8b58182. buildFieldNamesById now DFS-traverses the full schema (depth ≤ 64, ≤ 16,384 fields) and maps every field ID to its canonical dotted path with backtick-escaped segments, so indexes on nested fields resolve instead of failing the whole SHOW INDEX. Covered by a real nested-index regression case (nested_label_btree on a child field whose name contains a dot).
| for (LanceField field : dataset.getLanceSchema().fields()) { | ||
| topLevelFieldNames.put(field.getId(), field.getName()); | ||
| } | ||
| return normalize(dataset.describeIndices(), topLevelFieldNames); |
There was a problem hiding this comment.
[P2] Handle valid Lance system indexes before describing all entries. In the exact pinned SDK, optimized/MemWAL datasets can contain __lance_frag_reuse or __mem_wal with empty fields. describe_indices(None) includes them, but JNI unconditionally renders each entry's details and these system detail types have no scalar plugin, so this call aborts before any user index is returned; the local empty-field check would reject them after an SDK fix as well. Please pin/use a conversion that handles system entries and deliberately exclude them from SHOW INDEX, with a system-plus-user-index regression.
There was a problem hiding this comment.
Fixed in 8b58182. Bulk describeIndices() is no longer used. describeUserIndexes() lists physical entries via listIndexes(), filters __lance_frag_reuse / __lance_mem_wal, dedupes by logical name, then describes each user index through an exact IndexCriteria.hasName lookup (validating exactly-one cardinality and name match), so system entries never reach the details path.
| .getDbOrAnalysisException(tableNameInfo.getDb()); | ||
| CatalogIf catalog = Env.getCurrentEnv().getCatalogMgr() | ||
| .getCatalogOrAnalysisException(tableNameInfo.getCtl()); | ||
| DatabaseIf db = catalog.getDbOrAnalysisException(tableNameInfo.getDb()); |
There was a problem hiding this comment.
[P2] Gate REST catalogs before database resolution. On a cache miss this call populates the external meta-cache by listing Lance namespaces, and table resolution can then list remote tables; the REST-unsupported check is not reached until loadTableIndexMetadata. An unavailable REST endpoint therefore returns an unknown-database/provider failure after unnecessary I/O instead of the fixed unsupported error promised by this change. After the privilege check, reject the locally configured REST type before resolving the database/table, and cover the route with an unreachable namespace.
There was a problem hiding this comment.
Fixed in 8b58182. handleShowIndex now rejects a configured Lance REST catalog immediately after the privilege check and before getDbOrAnalysisException (ShowIndexCommand.java:117), so the fixed unsupported error is returned without any namespace or table I/O.
e4d334e to
5b0d2a0
Compare
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
| return future.get(remainingNanos, TimeUnit.NANOSECONDS); | ||
| } catch (TimeoutException e) { | ||
| // Deliberately do not cancel or interrupt the Future. If JNI has started, the worker | ||
| // remains the sole owner of its Dataset and allocator until the native call returns. |
There was a problem hiding this comment.
do not cancel or interrupt the Future
Will this cause the thread pool to be exhausted, making it unusable for everyone?
There was a problem hiding this comment.
Good question — this boundary is the trade-off the pool is built around. Short answer: temporary exhaustion under pathological storage failure is possible and intentional (bounded fail-fast); permanent exhaustion is not, and cancelling the Future would change neither.
Why cancelling doesn't help. cancel(true) only delivers Thread.interrupt(). The worker is blocked inside a synchronous JNI call; the Lance native runtime does not observe Java interrupts, so the thread stays occupied until the native call returns regardless. Cancellation would only flip the Future's state while the worker — the sole owner of the Dataset and its task-scoped 256MB allocator — is still running, re-introducing the ownership race this boundary exists to prevent. It doesn't even free the queue slot: a cancelled FutureTask stays in the queue until a worker dequeues it.
Why the pool still recovers. A stuck native call is already time-bounded inside the pinned SDK (lance-core 9.1.0-beta.3 → object_store 0.13.2): every S3 attempt terminates via connect_timeout=5s / timeout=30s defaults, and the retry loop stops at max_retries=3 or retry_timeout=180s elapsed. So one object-store op returns in ≲3.5 min even against an endpoint that hangs every request, and one SHOW INDEX task issues only a bounded number of such ops (describeIndices reads the already-open snapshot's manifest). Worst-case worker occupancy is minutes, not forever; expired queued tasks then fail before entering JNI and the pool drains within one deadline.
What users see during that window. Submissions beyond 4 running + 16 queued fail immediately with MetadataReadCapacityException — deliberate fail-fast backpressure rather than unbounded queuing (each in-flight task owns a 256MB Arrow allocator, so an unbounded pool is a native-memory risk). The waiting caller is bounded by min(query_timeout, 60s), and the blast radius is limited to Lance metadata reads on this FE; query execution and other catalogs never touch this pool.
Residual risk I consider acceptable: file:// datasets on a hung filesystem (NFS hard mount) can pin a worker indefinitely — local disk I/O has no timeout, and no pool design, cancellation included, can reclaim a thread in uninterruptible sleep.
If you prefer a tighter bound, we can pass an explicit client_retry_timeout (e.g. 30s) with the SHOW INDEX read options as a Doris-side default, so a stuck worker returns within roughly the caller deadline instead of the 180s SDK default. WDYT?
There was a problem hiding this comment.
get, maybe keep SDK default is ok
5b0d2a0 to
5f317f2
Compare
|
/review |
|
@u70b3 some file conflicts, u could rebase code, and we merge this pr firstly. |
5f317f2 to
f54055b
Compare
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
|
run buildall |
0378e1a to
82432b0
Compare
|
run buildall |
|
/review |
Extend lance_build_preinstalled_catalog.py with doris.nested_index: a struct-typed table whose BTREE index targets the dotted child attributes.`child.with.dot` (backtick quoting is required for the path to resolve), and whose deferred-remap compaction deliberately leaves a __lance_frag_reuse system entry so SHOW INDEX and index inspection prove they filter reserved system indexes. The existing vector fixture is kept byte-identical: nested_index was added to the committed catalog in place (pylance 4.0.1 per lance_fixture_requirements.txt), and the full builder self-check passes on the augmented fixture, including the vector boundary discriminator. For apache#66497.
### What problem does this PR solve? Issue Number: apache#66497 Related PR: apache#65730 Problem Summary: Lance external tables can contain logical vector and scalar indexes, but SHOW INDEX currently only handles internal OlapTable metadata and returns an empty result for Lance tables. Read authoritative logical index descriptions from one latest Directory Dataset snapshot, resolve field IDs from the same snapshot schema, and expose deterministic bounded metadata through the existing 13-column SHOW INDEX result without changing internal-table behavior. ### Release note Support SHOW INDEX for Lance filesystem/Directory Catalog tables. Lance REST index inspection remains unsupported in this phase. ### Check List (For Author) - Test: Unit Test and Regression test - Behavior changed: Yes. SHOW INDEX now returns logical indexes for Lance Directory tables and explicitly rejects Lance REST catalogs. - Does this need documentation: Yes. Document the supported Directory-only scope and REST limitation.
- Reject tables that are not LanceExternalTable with AnalysisException instead of failing on a raw ClassCastException. - Narrow getLanceIndexRows to throw AnalysisException only. - Add unit coverage for the non-Lance table guard. ### Release note Support SHOW INDEX for Lance filesystem/Directory Catalog tables.
branch-4.1 apache#66779 replaced the Spark-built vector fixture with the committed offline catalog: doris.vector_search is now doris.vs_ivf_pq_f32 with index embedding_ivf_pq_f32 (num_bits=4, num_sub_vectors=4), and the run07 SQL plus its companion Java builder are gone. The nested_index/BTree and __lance_frag_reuse coverage now lives in lance_build_preinstalled_catalog.py and the committed fixture. Golden regenerated with -forceGenOut and verified against a fresh local cluster plus a freshly rebuilt docker MinIO fixture; the full external_table_p0/lance directory passes (7/7). For apache#66497.
82432b0 to
3fa3735
Compare
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
|
hi @u70b3 |
Document the `SHOW INDEX` support for Lance Filesystem Catalog tables added in apache/doris#66637. ## What Changed - `lakehouse/catalogs/lance-catalog.mdx` (EN + zh-CN) - Feature Overview: metadata access now lists `SHOW INDEX` (Filesystem Catalogs only); added a `System tables (table$...)` row marked "Not supported" that points users to `SHOW INDEX` for index metadata, aligning with the Paimon/Iceberg system-table model. - New "Inspect Lance Indexes / 查看 Lance 索引" section: syntax and aliases (`SHOW INDEXES` / `SHOW KEY` / `SHOW KEYS`), the 13-column result mapping for Lance tables (only `Table`, `Key_name`, `Seq_in_index`, `Column_name`, `Index_type`, and `Properties` carry values), nested-field paths with backtick quoting, the `Properties` details allowlist, output examples taken from the code PR's regression test, fail-closed semantics, the `SHOW` privilege requirement, and the fixed `SHOW INDEX is not supported for Lance REST catalogs` error. - Current Limitations and Recommendations: added a `SHOW INDEX` bullet. - `sql-manual/sql-statements/table-and-view/index/SHOW-INDEX.md` (EN + zh-CN) - Description now covers Lance logical indexes, and notes that tables in other external catalogs (Iceberg, Paimon, ...) return an empty result, with links to their system-table docs (including Paimon `table_indexes`). - Added a Lance example. `docs/` (dev) is intentionally untouched: the Lance catalog doc was removed from dev in #4086 because apache/doris master has no Lance code (the feature lives on branch-4.1).
What problem does this PR solve?
Issue Number: #66497
Related PR: #65730
Lance external tables can contain logical vector and scalar indexes, but
SHOW INDEXcurrently only handles internalOlapTablemetadata and returns an empty result for Lance tables.This change reads authoritative logical index descriptions from one latest Lance Directory Dataset snapshot, resolves field IDs against the schema from that same snapshot, and exposes deterministic, bounded metadata through the existing 13-column
SHOW INDEXresult without changing internal-table behavior.What changed?
SHOW INDEX,SHOW INDEXES,SHOW KEY, andSHOW KEYSfor Lance filesystem/Directory Catalog tables.Design walkthrough (diagrams)
SHOW INDEXfor Lance Directory tables is a small read pipeline whose main concern is object ownership across the JNI boundary: SDK objects (Dataset,IndexDescription, Arrow allocators) never escape the loader, and the command layer only sees immutableLanceLogicalIndexvalues.Layered view — three new classes (
LanceLogicalIndex,LanceIndexMetadataLoader,LanceMetadataReadExecutor) and three touch points (ShowIndexCommand,LanceExternalTable,LanceExternalCatalog):flowchart TB subgraph CMD["Command layer · Nereids"] A["ShowIndexCommand<br/>analyze() privilege check (:83)<br/>handleShowIndex() dispatch (:111)<br/>buildLanceRows() row mapping (:159)"] end subgraph EXT["External table layer"] B["LanceExternalTable<br/>loadIndexMetadata() (:69) — thin delegate"] end subgraph CAT["Catalog layer · caller thread"] C["LanceExternalCatalog<br/>isRestCatalogConfigured() (:172) early REST rejection<br/>resolveTableAccess() (:371) namespace resolution"] C2["sanitizedRootCauseMessage() (:473)<br/>URI / credential / token redaction"] end subgraph BND["Execution boundary · lance-metadata-read pool"] D["LanceMetadataReadExecutor (:50)<br/>4 concurrent · 16 queued · deadline ≤ 60s"] E["LanceIndexMetadataLoader<br/>load() (:82) → normalize() (:206)"] end subgraph NAT["native · JNI"] F["Dataset.open(uri, latest)<br/>schema + describeIndices from one snapshot"] end G["LanceLogicalIndex<br/>immutable value object · pure-Java boundary"] A --> B --> C --> D --> E --> F E --> G C -.->|any failure| C2 C2 -.->|redacted exception only| ARequest timeline and resource ownership — namespace resolution stays on the caller thread (it owns the catalog's shared namespace/allocator), the JNI read runs on a bounded worker with a task-owned allocator, and a timed-out caller never cancels the in-flight native call:
Loader internals — one snapshot feeds both the schema mapping and the index descriptions; normalization is fail-closed and produces deterministic, bounded output:
flowchart LR O["Dataset.open(uri, latest)<br/>index cache 0 · metadata cache 64MB<br/>schema and indexes from one snapshot"] O --> S["getLanceSchema().fields()"] O --> L["listIndexes()<br/>physical entries ≤ 16384"] subgraph SB["schema branch"] S --> DFS["buildFieldNamesById() (:148)<br/>DFS depth ≤ 64 · fields ≤ 16384<br/>field id → dotted path, quoted when needed"] end subgraph IB["index branch"] L --> FL["drop system indexes<br/>__lance_frag_reuse · __lance_mem_wal"] FL --> DD["dedupe by logical name ≤ 256"] DD --> CR["describeUserIndexes() (:95)<br/>per-name describeIndices(hasName)<br/>must return exactly 1, name must match"] end DFS --> V CR --> V subgraph NB["normalize() (:206) · fail-closed"] V["known, unique field ids<br/>columns ≤ 64 · names ≤ 16KB total"] --> P["details JSON allowlist<br/>TreeMap-sorted · ≤ 400B"] P --> R["exact duplicate names rejected<br/>sorted by (name, position)"] end R --> OUT["LanceLogicalIndex list<br/>immutable · deterministic"]Fail-closed semantics (intentional)
Three strict behaviors are deliberate and locked by unit tests — the command reports an error rather than showing potentially misleading metadata:
SHOW INDEX(testRejectsUnknownNestedDuplicateNullAndEmptyFieldIds).testRejectsMalformedAndNonObjectJsonWithoutEchoingInput).testRejectsExactDuplicateNameButPreservesCaseOnlyNames).User/developer impact
Users can inspect logical Lance indexes through the standard 13-column
SHOW INDEXinterface for filesystem/Directory catalogs. Lance REST index inspection remains unsupported.Release note
Support
SHOW INDEXfor Lance filesystem/Directory Catalog tables.Check List (For Author)
git diff --checkSHOW INDEXnow returns logical indexes for Lance Directory tables and explicitly rejects Lance REST catalogs