Make sure that quoted identifiers remain quoted in formatter #11171
Conversation
There was a problem hiding this comment.
SqlFormatter has a dedicated method for formatting QualifiedName:
SetSession's name (and some other QualifiedNamess, e.g. in ResetSession). That should fix the issue.
Please note that neither of the formatting solutions respects the proper identifier semantics. However, the method I suggest is closer and easier to fix, since it delegates the formatting to Identifier.
sergey-melnychuk
force-pushed
the
sm/fix-identifier-formatting
branch
from
01704eb
to
25f136b
Compare
|
Thank you, @kasiafi , the |
sergey-melnychuk
force-pushed
the
sm/fix-identifier-formatting
branch
2 times, most recently
from
78b793f
to
c6e3fb1
Compare
There was a problem hiding this comment.
As a side-effect, this change alters the output of SqlFormatter regarding QualifiedNames in SetSession and ResetSession: before they were lowercased; after -- they are not.
It has no practical meaning, because the affected QualifiedNames are compared and resolved case-insensitive. However, please add this info to the commit message.
Many other Nodes have fields of type QualifiedName, too:
AddColumn
Analyze
Call
Comment
CreateMaterializedView
CreateSchema
CreateTable
CreateTableAsSelect
CreateView
Deny
DropColumn
DropMaterializedView
DropSchema
DropTable
DropView
FunctionCall
Grant
LikeClause
RenameColumn
RenameMaterializedView
RenameSchema
RenameTable
RenameView
ResetSession
Revoke
SetAuthorizationStatement
SetProperties
SetSession
ShowColumns
ShowCreate
Table
TruncateTable
Some of those are formatted with the formatName method, and some use toString. Could you please file an issue to check and fix the other occurrences in SqlFormatter and ExpressionFormatter?
sergey-melnychuk
force-pushed
the
sm/fix-identifier-formatting
branch
2 times, most recently
from
ce7ad9d
to
648a784
Compare
The side-effect of this change is that formatted statements for SetSession and ResetSession are no longer lowercased as before. However QualifiedName instances are resolved and compared case-insensitive, thus such side-effect does not lead to any consequences.
sergey-melnychuk
force-pushed
the
sm/fix-identifier-formatting
branch
from
648a784
to
fa8f2c3
Compare
|
CI hit failure uploading test report to GitHub. nothing actually failed. |
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 <sergey.melnychuk@starburstdata.com>
The
SqlFormattereffectively drops quotes from the quoted identifiers when formatting 'SET SESSION' statement (https://github.com/trinodb/trino/blob/master/core/trino-parser/src/main/java/io/trino/sql/SqlFormatter.java#L1514).This causes valid queries to fail: e.g.
SET SESSION "name-suffix".property = 'value'throwsio.trino.spi.TrinoException: Formatted query does not parseeven when identifier (name-suffix) is quoted.