Skip to content

fix(index): reject WITH params combined with sibling options - #259

Open
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/index-params-sibling-options
Open

jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/index-params-sibling-options

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

A raw params JSON passed to CREATE INDEX ... WITH (...) is treated as the
complete index parameter set, so sibling options in the same clause were
silently dropped — e.g. WITH (params='{...}', metric_type='cosine') ignored
metric_type and built the index with the default metric, no error raised.

Reject the combination in both WITH-clause parsers (the parser-extension
string path and the DuckDB options path); the replace / train / retrain
control flags are unaffected. Tests in test/sql/index_ddl.test cover both a
path-literal target and an attached-namespace table name.

A raw `params` JSON is treated as the complete index parameter set, so
sibling options in the same WITH clause (e.g. metric_type) were silently
dropped — building a different index than requested, with no error.

Reject the combination in both WITH-clause parsers (the SQL-string
parser-extension path and the DuckDB options path). The replace / train
/ retrain control flags are unaffected.

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@lance-gatekeeper lance-gatekeeper 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.

✅ Gate recommendation: approve.

Rejecting mixed parameter sources prevents silently building an index with different settings than requested, while preserving raw params with replace and train. Focused checks passed for both conversion paths; the full extension test suite was not run.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant