Skip to content
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

feat(spanner/spansql): add ROW DELETION POLICY parsing #4496

Merged
merged 5 commits into from Jul 27, 2021

Conversation

nktks
Copy link
Contributor

@nktks nktks commented Jul 26, 2021

This enables to parse ROW DELETION POLICY at CREATE TABLE, ALTER TABLE clauses.

https://cloud.google.com/spanner/docs/ttl

(This feature is currently preview.)

takeshi.nakata added 2 commits July 26, 2021 11:19
This enables to parse `ROW DELETION POLICY` at `CREATE TABLE`, `ALTER
TABLE` clauses.
@nktks nktks requested review from hengfengli, skuruppu and a team as code owners July 26, 2021 04:00
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Jul 26, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2021
@hengfengli hengfengli changed the title feat(spanner/spansql): add ROW DELETION POLICY parse feat(spanner/spansql): add ROW DELETION POLICY parsing Jul 26, 2021
@hengfengli
Copy link
Contributor

@olavloite It's worth to note that this feature is currently preview. So it is not open to the public yet.

Copy link
Contributor

@olavloite olavloite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have a question/suggestion on where to start the parsing of the row deletion policy expression, but feel free to leave it as is if you think that is better.

@@ -1236,6 +1245,15 @@ func (p *parser) parseAlterTable() (*AlterTable, *parseError) {
return a, nil
}

if p.eat("ROW") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Would it maybe make more sense to check if you can eat ROW DELETION POLICY here, and then only parse the actual expression. The same would then also apply to the REPLACE ROW DELETION POLICY?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Certainly, this makes easier to switch if another syntax is added after the ROW token.

I fixed at d83fb53

spanner/spansql/sql.go Show resolved Hide resolved
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2021
@nktks nktks requested a review from olavloite July 26, 2021 08:47
@hengfengli hengfengli added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2021
@hengfengli hengfengli added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 27, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 27, 2021
@hengfengli hengfengli merged commit 3d6c6c7 into googleapis:master Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants