sha174n commented on PR #44496:
URL: https://github.com/apache/superset/pull/44496#issuecomment-5834829410
@rusackas conflicts are cleared, the branch carries master and GitHub shows
it mergeable again.
On `_strip_comments` leaning on sqlglot's tokenizer instead: I checked
against the pinned 30.18.0 and it does not model the two things the strip
exists for.
- `tokenize("SELECT 1--2", read="mysql")` returns `NUMBER '1'` carrying
`comments: ['2']`, so sqlglot reads `--2` as a comment on MySQL. That is the
case b88e209 fixed, and deferring to the tokenizer would reintroduce it.
- A literal is a single token: `tokenize("EXECUTE IMMEDIATE
'PUT/**/file:///a @s'")` yields one `STRING` holding the whole literal, so the
tokenizer never sees the comment inside it. Getting at that still needs
per-literal re-tokenisation, which is the recursion already there.
- Comment offsets are not retained either (the block comment arrives as
`comments: ['']`, no span), so comments cannot be blanked by position without
inferring spans from gaps between token offsets.
Where sqlglot does carry the knowledge is the dialect family, and taking it
from there found a real gap, so 841fbda3 changes behavior again, flagging it
like the earlier ones:
- The MySQL-family list was hand-maintained and had drifted. `pinot` and
`odelasticsearch` subclass `MySQL` and inherit its tokenizer but were not on
it, so an unspaced `--` in their nested bodies was stripped as a comment and
dropped the rest of the body from this gate and from the two pre-existing ones
that scan the same text. It is `isinstance(dialect, MySQL)` now, which is how
sqlglot models the family, so it cannot drift again. Both directions pinned for
`pinot`.
- `_strip_comments` no longer descends into a literal that carries neither
`--` nor `/*`, since that cannot change it. A body of nothing but quotes was
costing one recursion per `''` pair: 60x on a 32 KB body, byte-identical output.
- `get_client_file_transfer_command` now defaults to `None` on the base
statement, as `changes_search_path` does, so the Kusto override goes away.
@aminghadersohi the `_strip_comments` lexing is the part worth your eyes,
per Evan's note above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
