-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
sql: fix literal reparse issue with timestamps #68336
Conversation
78bd4cc
to
39c91eb
Compare
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.
change lgtm! but i just had a request to add a comment
@@ -1626,7 +1626,11 @@ func (node *CastExpr) Format(ctx *FmtCtx) { | |||
if _, ok := node.Expr.(*StrVal); ok { | |||
ctx.FormatTypeReference(node.Type) | |||
ctx.WriteByte(' ') | |||
ctx.FormatNode(node.Expr) | |||
if ctx.HasFlags(FmtHideConstants) { |
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 think this might need an explanatory comment.
could you add a comment saying that "we need to replace this with a quoted string constant because the grammar requires a string constant rather than an expression for this form of casting in the typed_literal
rule"
cockroach/pkg/sql/parser/sql.y
Line 11444 in 6eafde9
| const_typename SCONST |
There were reparse issues in multiple test when timestamps were replaced with underscores. Making the underscore characters strings solved the issue. Release note: None
39c91eb
to
86701b2
Compare
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.
lgtm! i re-triggered the teamcity tests to see if they pass
bors r=rafiss |
Build succeeded: |
Related to: #60722
There were reparse issues in multiple test when timestamps
were replaced with underscores. Making the underscore characters
strings solved the issue.
Release note: None