Repository navigation
Conversation
Lets a dialect parse the name of a table in a table factor, falling back to parse_object_name when it returns None. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hb9YMy3Ay2oNWagfTz2KzZ
|
No dialect label was applied because the title does not name a SQL dialect. If this change targets one or more dialects, edit the title to |
Yeah in my opinion the clean solution will be at some point to refactor the tokenizer dialect-generic, so tokenization can be optimally implemented for each dialect. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2610 +/- ##
=======================================
Coverage 81.14% 81.15%
=======================================
Files 42 42
Lines 33736 33748 +12
Branches 33736 33748 +12
=======================================
+ Hits 27376 27388 +12
Misses 2797 2797
Partials 3563 3563 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hb9YMy3Ay2oNWagfTz2KzZ
|
Agreed that a dialect-specific tokenizer is the right long-term fix for lexing. For Presto, I'm open to working on the tokenizer side too. If useful, I can open an issue sketching it, for example a |
|
No dialect needs this hook. Presto, the one engine named with The hook also cannot represent what it is for. It returns an Today I will now close this PR. |
|
I'd like to correct two claims in the closing comment: that this syntax is already handled, and that no dialect needs this hook. I know the second claim is not correct, because I have written — in a real prototype — exactly the workaround that #2604 describes as the only option available today. The workaroundThe dialect I am targeting writes a qualified table name in
Why this is not a solutionBoth of its defects come from doing parser work at token level:
Nor is this the Presto situation in #2611: there the colon is a character inside a single identifier, so a lexer rule settles it; here it separates two parts that must be split and reordered, which no lexer rule can do. If this syntax were already supported, none of this code would exist. The hook is what would let a dialect delete it: the parser already knows it is in table position; the dialect would supply only the name. Connecting back to #2604The special cases in the core parser are not an alternative to this extension point: every dialect-specific table syntax the parser supports today lives in the core as an ad-hoc case, because there is nowhere in a dialect to put it:
That is the gap #2604 is about. #2604 also lists the other motivations — On the processI do not think closing this PR while leaving #2604 open is a coherent place to land. The limitations raised in the closing comment were flagged by me from the start: the round-trip note is in this PR's own description, and the table-factor-only scope is the scope #2604 proposed. I still believe the hook, limits included, is a base to iterate on in review — and a better one than the token-rewriting workaround every downstream dialect has to write today. Closing it in favour of a refactor that so far exists as a sketch, to "look into in a month or two", leaves #2604 in limbo: the issue open and incremental improvement have been rejected in favor of a bigger improvement that is not currently prioritized |
|
Please do not be a meat proxy as I did spend my own time reading and trying to make sense of the code. On the As I wrote in my previous reply, the long-term answer is a tokenizer generic over the dialect. I will work on it if and when I have the time, as I am interested in improving its performance. |
|
@LucaCappelletti94 -- let me know if you need help. I would enjoy helping make sqlparser more efficient as well |
We will certainly have to chat it over for a while, as I expect it would be a rather large refactoring with a non trivial amount of implications downstream. For the immediate future I prefer sticking to bug squashing so when we finally get to refactoring we will be able to tell whether what we see is a regression or not. |
Closes #2604.
Dialectcan override expression, statement and column-option parsing, but not table names:parse_table_factoralways callsparse_object_name(true). A downstream dialect with syntax likeFROM events:analyticshas to rewrite tokens before parsing.This adds
parse_table_factor_name, which works likeparse_prefix/parse_statement: returnNoneto fall back to the default parser. Only the name is replaced; alias, joins, sampling and hints still go through the core parser. Note thatObjectNameprints its parts joined with., so custom separators don't round-trip.derive_dialect's generated imports gainObjectNameso derived dialects compile.Dialects: none
Tests:
custom_table_factor_name_parserintests/sqlparser_custom_dialect.rs.Implemented with AI assistance (Claude Code).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Hb9YMy3Ay2oNWagfTz2KzZ