Skip to content

fix(query): support CTE WITH queries, leading comments, and trailing semicolons in tabular query detection - #69

Merged
ZhuchkaTriplesix merged 1 commit into
devfrom
issue/58-cte-comments-trailing-semicolon
Sep 28, 2026
Merged

ZhuchkaTriplesix merged 1 commit into
devfrom
issue/58-cte-comments-trailing-semicolon

Conversation

@ZhuchkaTriplesix

Copy link
Copy Markdown
Member

Summary

  • is_tabular_query classified queries by uppercasing the raw SQL and checking its literal prefix, which misclassified:
    • queries with a leading -- comment or /* comment */ as non-tabular (silently returning 0 columns/0 rows)
    • CTE WITH ... SELECT queries as non-tabular (WITH wasn't in the prefix list)
    • queries with a trailing ; — FORMAT was appended after the semicolon, causing a ClickHouse syntax error
  • Now classifies on strip_sql_comments_and_trim(trimmed_sql) (the same comment/string-aware normalizer already used for Safe Mode), added WITH to the tabular prefix list, and strips trailing ;/whitespace before appending FORMAT JSONCompactEachRowWithNamesAndTypes.
  • Added regression tests covering all three edge cases plus a placement check for the FORMAT clause.

Closes #58

…semicolons in tabular query detection

is_tabular_query in handle_query classified queries by uppercasing the
raw SQL and checking its literal prefix, which broke on:
- a leading `-- comment` or `/* comment */` (misclassified as
  non-tabular, silently returning 0 columns/0 rows)
- CTE `WITH ... SELECT` queries (`WITH` wasn't in the prefix list)
- a trailing `;` (FORMAT was appended after it, producing a ClickHouse
  syntax error, since FORMAT must precede the statement terminator)

Classify on strip_sql_comments_and_trim(trimmed_sql) instead of a raw
uppercase, add WITH to the tabular prefix list, and strip trailing
`;`/whitespace before appending the FORMAT clause. Closes #58
@ZhuchkaTriplesix
ZhuchkaTriplesix merged commit 75ddfbb into dev Sep 28, 2026
2 checks passed
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.

1 participant