-
Notifications
You must be signed in to change notification settings - Fork 20
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
Support SELECT AS VALUE and SELECT AS typename #122
Support SELECT AS VALUE and SELECT AS typename #122
Conversation
60d020c
to
b9e8f3c
Compare
Rebase to reduce dependency to other branch. |
ast/ast.go
Outdated
@@ -443,7 +453,8 @@ type Select struct { | |||
Select token.Pos // position of "select" keyword | |||
|
|||
Distinct bool | |||
AsStruct bool | |||
AsStruct bool // deprecated, use As |
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'm not sure this deprecation is necessary. Could we remove it simply?
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.
Removed ff14fe6
It is because I didn't sure the deprecation policy of memefish.
Current policy: We can change any AST node without breaking change notification because there are no release version.
Is it correct?
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 have no time to maintain codes with some deprecations. Also, we have no plan to publish a major version, so we can easily introduce breaking changes IMO.
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.
Thank you for answering!
We can make breaking changes to v0.x.y at any time, but the release process itself is a significant effort, so I think the current policy is fine.
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!
This PR add support of
SELECT AS VALUE
#97 andSELECT AS typename
#89.SELECT AS VALUE
https://cloud.google.com/spanner/docs/reference/standard-sql/query-syntax#select_as_value
SELECT AS typename
https://cloud.google.com/spanner/docs/reference/standard-sql/query-syntax#select_as_typename
They and
SELECT AS STRUCT
are generalized asast.SelectAs
.Note: This PR depends onrebasedast.NamedType
in #121, so it is needed to rebase after #121 is merged.fix #97