Repository navigation
fix(index): reject WITH params combined with sibling options - #259
Open
jackylee-ch wants to merge 1 commit into
Open
jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
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.
There was a problem hiding this comment.
✅ 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A raw
paramsJSON passed toCREATE INDEX ... WITH (...)is treated as thecomplete index parameter set, so sibling options in the same clause were
silently dropped — e.g.
WITH (params='{...}', metric_type='cosine')ignoredmetric_typeand 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/retraincontrol flags are unaffected. Tests in
test/sql/index_ddl.testcover both apath-literal target and an attached-namespace table name.