korbit-ai[bot] commented on code in PR #32350:
URL: https://github.com/apache/superset/pull/32350#discussion_r1966676296
##########
superset/sql/dialects/firebolt.py:
##########
@@ -84,6 +84,9 @@ class FireboltOld(Firebolt):
function.
"""
+ class Tokenizer(tokens.Tokenizer):
+ STRING_ESCAPES = ["'", "\\"]
Review Comment:
### Tokenizer Added to Wrong Dialect Class <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The STRING_ESCAPES list is defined but the Tokenizer class is added to
FireboltOld instead of the main Firebolt dialect class, which doesn't align
with the developer's intent to add escaping support for the Firebolt dialect.
###### Why this matters
Users of the main Firebolt dialect won't have access to the backslash
escaping functionality for single quotes, as it's only available in the legacy
FireboltOld dialect.
###### Suggested change ∙ *Feature Preview*
Move the Tokenizer class from FireboltOld to the main Firebolt dialect class:
```python
class Firebolt(Dialect):
class Tokenizer(tokens.Tokenizer):
STRING_ESCAPES = ["'", "\\"]
class Parser(parser.Parser):
# ... rest of the code
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eecb4261-89b6-4a4e-b3e5-638c39b72d01?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:132c2677-1a93-4c0e-a614-21a2aa9e5e1a -->
--
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]
