Skip to content

fix(drizzle-kit): handle commas in SQLite expression index columns - #6065

Open
mixelburg wants to merge 1 commit into
drizzle-team:mainfrom
mixelburg:fix/sqlite-expression-index-comma
Open

fix(drizzle-kit): handle commas in SQLite expression index columns#6065
mixelburg wants to merge 1 commit into
drizzle-team:mainfrom
mixelburg:fix/sqlite-expression-index-comma

Conversation

@mixelburg

Copy link
Copy Markdown

squashIdx and squashUnique used comma-joined column strings which broke when an expression column contained its own comma (e.g., json_extract(payload, '$.ref')). This caused to emit invalid SQL with backtick-quoted expression fragments.\n\nUses JSON encoding for the columns array with a split(',') fallback for backward compatibility with existing snapshot files.\n\nFixes #6062

squashIdx/squashUnique used comma-joined column strings which
broke when an expression column contained its own comma
(e.g., json_extract(payload, '$.ref')). Use JSON encoding
for the columns array with a split(',') fallback for
backward compatibility with existing snapshot files.

Fixes drizzle-team#6062

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

Nice approach — fixing it at the serialization boundary rather than trying to re-join fragments downstream is the right call, since the isExpression guard in the convertor can never match once the string has already been split.

One edge case in the backward-compat path, though. The fallback only fires on a thrown parse error:

try {
    columns = JSON.parse(columnsString);
} catch {
    columns = columnsString.split(',');
}

For an old-format snapshot, columnsString is the raw comma-joined names. Most of the time that isn't valid JSON so it throws and the fallback is correct. But if the index has a single column whose name happens to be valid JSON, JSON.parse succeeds and returns a non-array:

"123"   -> 123      (number)
"true"  -> true     (boolean)
"null"  -> null
"1e5"   -> 100000

So an old snapshot with an index on a column named 123 (legal as a quoted identifier) silently yields columns: 123 instead of ["123"], skipping the fallback entirely. It'll then blow up in index.parse as a zod error rather than falling back gracefully.

Guarding on the shape rather than on throwing handles it:

let columns: string[];
try {
    const parsed = JSON.parse(columnsString);
    columns = Array.isArray(parsed) ? parsed : columnsString.split(',');
} catch {
    columns = columnsString.split(',');
}

Same applies to the unsquashUnique / unsquashPK copies of this pattern if they take the same shape.

Might also be worth a test with an old-format squashed string as input, since the current tests presumably all round-trip through the new squashIdx and so never exercise the fallback branch at all.

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.

2 participants