-
Notifications
You must be signed in to change notification settings - Fork 586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(flink): quote SQL identifiers indiscriminately #7605
fix(flink): quote SQL identifiers indiscriminately #7605
Conversation
bb10e78
to
969b9d7
Compare
pyproject.toml
Outdated
@@ -376,7 +376,7 @@ show_deps = true | |||
[tool.codespell] | |||
# local codespell matches `./docs`, pre-commit codespell matches `docs` | |||
skip = "*.lock,.direnv,.git,./docs/_freeze,docs/_freeze/**,*.svg,*.css,*.html,*.js" | |||
ignore-regex = '\b(i[if]f|I[IF]F|AFE)\b' | |||
ignore-regex = '\b(i[if]f|I[IF]F|AFE|inout)\b' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if inline ignores were supported since "inout" seems like a very possible typo in most cases), but they're not.
@@ -1215,7 +1215,7 @@ def test_first_last(backend): | |||
raises=com.OperationNotDefinedError, | |||
reason="not support by the backend", | |||
) | |||
@pytest.mark.notyet(["flink"], raises=Py4JJavaError, reason="not supported by Flink") | |||
@pytest.mark.broken(["flink"], raises=Py4JJavaError, reason="bug in Flink") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were two issues here:
time
is a reserved keyword- Even then, there seems to be an issue: https://apache-flink.slack.com/archives/C03G7LJTS2G/p1700675938430139
The stack trace on the second results in:
Caused by: org.apache.flink.util.FlinkRuntimeException: org.apache.flink.api.common.InvalidProgramException: Table program cannot be compiled. This is a bug. Please file an issue.
I'm still discussing with somebody on the Flink side; may update later with a Jira ref.
@@ -14,9 +14,11 @@ def format_call(translator, func, *args): | |||
return "{}({})".format(func, ", ".join(formatted_args)) | |||
|
|||
|
|||
def quote_identifier(name, quotechar="`", force=False): | |||
def quote_identifier( | |||
name, quotechar="`", force=False, base_identifiers=identifiers.base_identifiers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to name it reserved_keywords
, but I think Flink's set is actually reserved + unreserved keywords. So I left it as base_identifiers
.
e8e344b
to
29c20d1
Compare
29c20d1
to
5f1ab00
Compare
(I intentionally haven't squashed, to leave the reference for customizing identifiers, but just let me know if would like me to before merging.) |
No need to squash! |
Needed to update the list for Flink (ran into a couple of issues today, with
TIME
andVALUE
).