Preserve quotedness for Identifier when formatting Query #17186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swapsmagic
requested changes
Jan 13, 2022
Comment on lines
462
to
465
| Identifier tableName = parts.get(0); | ||
| Identifier schemaName = (parts.size() > 1) ? parts.get(1) : new Identifier(objectName.getSchemaName()); | ||
| Identifier catalogName = (parts.size() > 2) ? parts.get(2) : new Identifier(objectName.getCatalogName()); | ||
| String sql = formatSql(new CreateView(QualifiedName.of(ImmutableList.of(catalogName, schemaName, tableName)), query, false, Optional.empty()), Optional.of(parameters)).trim(); |
There was a problem hiding this comment.
This logic can be moved into some methods and reused.
i.e. getTableName, getSchemaName, getCatalogName
v-jizhang
force-pushed
the
maintain_quotedness
branch
from
574c820 to
f80abb1
Compare
|
The failed test is unrelated. |
swapsmagic
approved these changes
Jan 26, 2022
| @@ -161,7 +161,7 @@ public void testPreparedStatementsSpecialCharacters() | |||
| .put(PRESTO_LANGUAGE, "zh-TW") | |||
| .put(PRESTO_TIME_ZONE, "Asia/Taipei") | |||
| .put(PRESTO_CLIENT_INFO, "null") | |||
| .put(PRESTO_PREPARED_STATEMENT, "query1=select * from tbl:ns") | |||
| .put(PRESTO_PREPARED_STATEMENT, "query1=select * from \"tbl:ns\"") | |||
There was a problem hiding this comment.
What is the reason for having this query with delimiter? If it's needed, we should have it generated using the parser so if in future we change the delimiter, this test won't break.
There was a problem hiding this comment.
Please the commit message here
|
Also please get the failed test pass by re-running the tests. |
|
Is there any chance of being merged in the next version? Very important fix for us. Thank you! |
|
Looks like this is missing one followup from @v-jizhang |
Cherry-pick of trinodb/trino#80 Presto doesn't maintain the quotedness of an identifier when using SqlQueryFormatter so it results in throwing parsing error when we run prepare query of the syntax [prestodb#10739]. This PR solves that above issue. Co-authored-by: praveenkrishna <[email protected]>
v-jizhang
force-pushed
the
maintain_quotedness
branch
from
cecfba5 to
c620eb8
Compare
Cherry-pick of trinodb/trino#6380: Identifiers with characters such as @ or : are not being treated as delimited identifiers, which causes issues when the planner creates synthetic identifiers as part of the plan IR expressions. When the IR expressions are serialized, they are not being properly quoted, which causes failures when parsing the plan on the work side. For example, for a query such as: SELECT try("a@b") FROM t The planner creates an expression of the form: "$internal$try"("$INTERNAL$BIND"("a@b", (a@b_0) -> "a@b_0")) The argument to the lambda expression is not properly quoted. Co-authored-by: Martin Traverso <[email protected]>
v-jizhang
force-pushed
the
maintain_quotedness
branch
from
c620eb8 to
1fda46d
Compare
|
@jbapple All tests passed. |
v-jizhang
added a commit
to v-jizhang/presto
that referenced
this pull request
Mar 4, 2022
Cherry-pick of trinodb/trino#11171 The SqlFormatter effectively drops quotes from the quoted identifiers when formatting 'SET SESSION' statement. This causes valid queries to fail: e.g. SET SESSION "name-suffix".property = 'value' PrestoException: Formatted query does not parse even when identifier (name-suffix) is quoted. Follow up prestodb#17186 Co-authored-by: Sergey Melnychuk <[email protected]>
9 tasks
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick of trinodb/trino#80
Presto doesn't maintain the quotedness of an identifier when
using SqlQueryFormatter so it results in throwing parsing error
when we run prepare query of the syntax
[https://github.com//issues/10739]. This PR
solves that above issue.
Co-authored-by: praveenkrishna [email protected]
Test plan - Added tests