Skip to content

[feature](lance) Add statically validated Lance index DDL surface - #67201

Open
u70b3 wants to merge 6 commits into
apache:branch-4.1from
u70b3:pr3a-lance-index-ddl
Open

[feature](lance) Add statically validated Lance index DDL surface#67201
u70b3 wants to merge 6 commits into
apache:branch-4.1from
u70b3:pr3a-lance-index-ddl

Conversation

@u70b3

@u70b3 u70b3 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: #66497

Related PR: #66637 (merged), #66671 (open — independent; this PR shares no files with it)

Problem Summary:

This is the first sub-PR (PR3A) of delivery slice 3 of the v5.1 design (final 4.2 contract, scope confirmed in this review): the Lance index DDL surface with static validation and target-aware routing, in reject-all mode. Slice 3's remaining pieces — durable jobs, same-name fences, unresolved quotas, replay, job SQL, and dispatch with fake-worker fault tests — land as follow-up PRs; this PR contains no job admission, no index-metadata reads, and no enablement gate.

What this PR adds:

  • Top-level CREATE [OR REPLACE] INDEX ... USING ANN/BTREE/BITMAP and DROP INDEX [IF EXISTS] parsing for Lance Directory catalog tables. OR REPLACE is mutually exclusive with IF NOT EXISTS; BTREE is a new non-reserved keyword. The indexDef rule used by CREATE TABLE and ALTER TABLE ... ADD INDEX is unchanged, and ALTER TABLE ... ADD/DROP INDEX remains unsupported per Section 2.1, so previously parseable internal SQL behaves byte-identically.
  • IndexDefinition carries orReplace and the Lance-only type name without extending the persisted internal IndexDef.IndexType enum (Section 4.4). New validate() guards reject Lance-only syntax on internal tables and fire only for SQL that was a syntax error before this PR.
  • LanceIndexMutationValidator applies the FE static bounds of Section 2.4: the ANN/IVF_PQ property matrix (required index_type=IVF_PQ, metric in l2/cosine/dot, positive required num_partitions/num_sub_vectors, fixed num_bits=8, unknown and case-variant duplicate properties rejected), the BTREE/BITMAP column-type sets (uint64/LARGEINT included as integral for both), exactly one non-null column, and a bounded index name. Arrow-level revalidation (fixed-size-list, dimension, float16-vs-float32, subvector divisibility) is deferred to the isolated worker per Sections 2.4/4.2 because LanceTypeConverter erases those facts.
  • Target-aware routing in AlterTableCommand.validate (after the existing table ALTER privilege check and catalog resolution, before any op validation): top-level CREATE/DROP INDEX on a Lance table is statically validated and then rejected with a typed per-op message before any Env.getNextId() allocation. Lance REST catalogs receive a fixed unsupported error mirroring PR1's SHOW INDEX stance. Internal tables never enter this branch.

Explicitly not in this PR: durable job records, fences, quotas, and replay (PR3B); authoritative IF semantics, admission, and job SQL (Section 2.2, PR3C); dispatch and the isolated worker (PR3D/slice 4); FORCE_RELEASE (PR3E); the mutation enablement configuration (Section 9.7, arrives with admission). Property-value normalization is validation-local; persisting normalized values belongs to admission. SHOW INDEX behavior from PR1 and the PR2 inspection surface are untouched.

Known accepted behavior change: the new BTREE keyword token makes stored-procedure bodies that use btree as a bare identifier (e.g. CLOSE btree) fail PL parsing, the same hazard class as the pre-existing ANN token; all other identifier positions are covered by nonReserved.

Release note

Add the CREATE [OR REPLACE] INDEX ... USING ANN/BTREE/BITMAP and DROP INDEX SQL surface for Lance Directory catalog tables with static validation; statements are currently rejected with a typed not-supported error while the mutation lifecycle is staged.

Check List (For Author)

  • Test

    • Regression test
      • regression-test/suites/external_table_p0/lance/test_lance_index_ddl.groovy added (mirrors test_lance_show_index): typed rejections and static-matrix errors on the filesystem catalog, REST-specific messages, uniform DROP INDEX IF EXISTS rejection, the generic ALTER TABLE ADD INDEX rejection, and ALTER privilege denial preceding the typed rejection. Not executed locally — it requires the shared external docker env (MinIO + preinstalled Lance datasets + Lance REST server); pipeline/CI run requested.
    • Unit Test
      • Focused run passed 187/187 with checkstyle enabled (cd fe && mvn test -pl fe-common,fe-core -am -DfailIfNoTests=false -Dtest='...', no -Dcheckstyle.skip): new CreateIndexParserTest (10), LanceIndexMutationValidatorTest (24), AlterTableCommandLanceIndexTest (9), plus pre-existing NereidsParserTest (79), AlterTableCommandTest (17), IndexDefinitionTest (5), ShowIndexCommandTest (8), LanceIndexMetadataLoaderTest (26), LanceFilesystemCatalogTest (9) — zero regressions.
    • Manual test
      • Parser regenerated from a clean target/generated-sources; fe-common,fe-core -am compile green with and without checkstyle; mvn validate -pl fe-core reports 0 Checkstyle violations (test sources included).
      • Internal-parity reviewed: the new guards are unreachable for previously parseable SQL (all IndexDefinition string-ctor call sites verified); MTMV/OlapTable/Iceberg/JDBC paths never enter the Lance branch; rejected Lance statements allocate no global id.
  • Behavior changed:

    • No.
    • Yes. Top-level CREATE/DROP INDEX on Lance tables now fails with typed per-op messages after static validation (previously a generic external-table rejection after op validation); CREATE INDEX ... USING BTREE/BITMAP and CREATE OR REPLACE INDEX now parse (previously syntax errors). Internal-table index DDL is unchanged.
  • Does this need documentation?

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

u70b3 added a commit to u70b3/doris that referenced this pull request Aug 28, 2026
### What problem does this PR solve?

Issue Number: apache#66497

Related PR: apache#67201

Problem Summary: Lance REST index DDL resolved databases and tables before returning the fixed unsupported-operation error, which could trigger remote metadata requests or expose connection errors first. Reject top-level CREATE, CREATE OR REPLACE, and DROP INDEX immediately after catalog lookup, preserve ALTER TABLE ADD/DROP behavior, and assign stable error codes to Lance index validation and unsupported-operation failures.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - LanceIndexMutationValidatorTest
    - AlterTableCommandLanceIndexTest
    - AlterTableCommandTest
    - IndexDefinitionTest
    - Standard FE build
- Behavior changed: Yes. Lance REST index DDL now fails before database or table metadata resolution and Lance index errors expose stable client-visible codes.
- Does this need documentation: No
@u70b3
u70b3 marked this pull request as ready for review August 29, 2026 07:55
@u70b3
u70b3 requested a review from yiguolei as a code owner August 29, 2026 07:55
@u70b3

u70b3 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@u70b3

u70b3 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/review

u70b3 and others added 5 commits August 29, 2026 15:57
Extend the top-level CREATE INDEX grammar with OR REPLACE and the
BTREE/BITMAP index-type keywords, and carry orReplace plus the
Lance-only type name on IndexDefinition without touching the persisted
internal IndexDef.IndexType enum. The indexDef rule used by CREATE
TABLE and ALTER TABLE ADD INDEX is unchanged, so previously parseable
internal SQL behaves byte-identically; the new validate() guards only
fire for SQL that was a syntax error before.
Add LanceIndexMutationValidator with the FE static bounds of the index
lifecycle design: ANN/IVF_PQ property matrix, BTREE/BITMAP column-type
sets, single non-null column, bounded index name, case-insensitive
property keys with duplicate detection, and a fixed REST-catalog
rejection. AlterTableCommand routes top-level CREATE/DROP INDEX on
Lance tables through the validator and then rejects with a typed
message before any Env id allocation; ALTER TABLE ADD/DROP INDEX keeps
the existing generic rejection and internal tables never enter the
branch. Arrow-level revalidation stays with the isolated worker, and
property normalization for admission stays with the job layer.
Parser tests for the new grammar shapes and the OR REPLACE/IF NOT
EXISTS exclusion, validator tests for the full static matrix, and
command-level tests proving typed rejections after validation, REST
messages, alter-clause fall-through, privilege-before-rejection, and
that no Env id is allocated on any rejected path.
Pipeline-only suite mirroring test_lance_show_index: typed rejections
and static-matrix errors on the filesystem catalog, REST-specific
messages, uniform DROP INDEX IF EXISTS rejection, the generic ALTER
TABLE ADD INDEX rejection, and ALTER privilege denial preceding the
typed rejection. Requires the shared external docker env.
### What problem does this PR solve?

Issue Number: apache#66497

Related PR: apache#67201

Problem Summary: Lance REST index DDL resolved databases and tables before returning the fixed unsupported-operation error, which could trigger remote metadata requests or expose connection errors first. Reject top-level CREATE, CREATE OR REPLACE, and DROP INDEX immediately after catalog lookup, preserve ALTER TABLE ADD/DROP behavior, and assign stable error codes to Lance index validation and unsupported-operation failures.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - LanceIndexMutationValidatorTest
    - AlterTableCommandLanceIndexTest
    - AlterTableCommandTest
    - IndexDefinitionTest
    - Standard FE build
- Behavior changed: Yes. Lance REST index DDL now fails before database or table metadata resolution and Lance index errors expose stable client-visible codes.
- Does this need documentation: No
@u70b3
u70b3 force-pushed the pr3a-lance-index-ddl branch from afec1b8 to 3938775 Compare August 29, 2026 07:57
@Gabriel39

Copy link
Copy Markdown
Contributor

[P2] Please validate blank Lance index names in both the CREATE and DROP paths.

Nereids still accepts empty backquoted identifiers outside the specialized column-path validation (for example, CREATE INDEX `` ... and DROP INDEX `` ...). validateCreateIndex currently checks only whether the UTF-8 name exceeds 64 bytes, so an empty name passes. For Directory DROP, AlterTableCommand returns the typed unsupported error before DropIndexOp.validate() runs, bypassing its existing StringUtils.isEmpty(indexName) check and providing no 64-byte bound.

In the current reject-all stage this masks malformed SQL as an unsupported operation. More importantly, if the follow-up admission path reuses this validator, an empty logical name could reach the durable job and same-name fence.

Please add a shared Lance index-name validator that rejects null/empty names and names over 64 UTF-8 bytes, invoke it for Directory CREATE and DROP, and add command-level coverage for empty quoted names. The REST path can retain its fail-fast unsupported response before database/table metadata resolution.

### What problem does this PR solve?

Issue Number: apache#66497

Related PR: apache#67201

Problem Summary: Nereids accepts an empty backquoted identifier, so CREATE INDEX `` and DROP INDEX `` reach Lance validation with an empty index name. validateCreateIndex only bounded names over 64 UTF-8 bytes, and the Directory DROP path returned the typed unsupported error before DropIndexOp.validate() could run its empty-name check, masking malformed SQL as an unsupported operation; if admission later reuses this validator, an empty logical name could reach the durable job and same-name fence.

Add a shared Lance index-name validator rejecting null/empty names and names over 64 UTF-8 bytes, invoke it for Directory CREATE and DROP, and add command-level coverage for empty quoted names. The REST path retains its fail-fast unsupported response before database/table metadata resolution.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - LanceIndexMutationValidatorTest
    - AlterTableCommandLanceIndexTest
    - CreateIndexParserTest
    - AlterTableCommandTest
    - IndexDefinitionTest
    - Regression suite test_lance_index_ddl (external docker env, runs in CI)
- Behavior changed: Yes. Blank Lance index names now fail with "index name cannot be empty" instead of the unsupported-operation error.
- Does this need documentation: No
@Gabriel39

Copy link
Copy Markdown
Contributor

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot 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.

Approval opinion: no blocking issues found on head 7469e38.

Critical checkpoints:

  • Syntax/model: the top-level CREATE/REPLACE/DROP grammar is target-neutral, the CREATE TABLE/ALTER TABLE indexDef grammar remains unchanged, and Lance-only BTREE/BITMAP state cannot reach internal Index translation.
  • Authorization/routing: table ALTER privilege is checked before catalog access; REST rejects before database/table resolution; Directory Lance statically validates before the typed reject-all response and before ID allocation.
  • Static matrix: every section-2.4 fact retained in Doris Column is checked. Fixed-list shape/dimension and vector element precision/nullability are genuinely erased and remain explicitly assigned to the isolated worker.
  • Compatibility: BTREE is non-reserved for ordinary SQL identifiers. The disclosed PL bare-identifier keyword caveat is the only identified compatibility change; no additional internal/MTMV/other-external regression was found.
  • Tests/release/docs: focused parser, validator, route/error-code, privilege, no-ID, and real-catalog regression coverage matches this reject-all slice. Builds were not run under the review-runner restriction; live compile and FE-UT checks were pending at review time. The release note matches the staged surface, and deferring user documentation until mutation enablement is reasonable.
  • FileScannerV2 checkpoints: not applicable because no FileReader/scan path changes.
  • User focus: no additional focus was provided.

Review completeness: two rounds converged; all candidates were accepted, deduplicated, or dismissed with evidence, and no accepted inline issue remains.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 76.71% (112/146) 🎉
Increment coverage report
Complete coverage report

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.

3 participants