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

spanner/spansql: support for Update in parseDMLStmt #3162

Closed
lazeratops opened this issue Nov 9, 2020 · 2 comments · Fixed by #3201
Closed

spanner/spansql: support for Update in parseDMLStmt #3162

lazeratops opened this issue Nov 9, 2020 · 2 comments · Fixed by #3201
Assignees
Labels
api: spanner Issues related to the Spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lazeratops
Copy link

lazeratops commented Nov 9, 2020

Is your feature request related to a problem? Please describe.

Currently spannertest seems to be unusable with UPDATE SQL queries. parseDMLStmt() in spansql/parser.go seems to only support DELETE. There is a TODO: Insert, Update comment, but I was unable to find an associated feature request issue for this and not sure if it is planned, hence this request to put it on the roadmap. Relevant line: https://github.com/googleapis/google-cloud-go/blob/master/spanner/spansql/parser.go#L1284

Describe the solution you'd like

I would like for UPDATE DML to be parsed via spansql , which would enable us to properly test these statements via spannertest. Ideally INSERT would be parsed as well, but as UPDATE is more relevant for our immediate purpose I am focusing on that one here.

Describe alternatives you've considered

An alternative would be to update via an Update mutation instead. We will likely go with this solution for the time being, but being forced to change the implementation to a mutation solely to account for missing test functionality does not seem ideal.

Thanks!

@lazeratops lazeratops added the triage me I really want to be triaged. label Nov 9, 2020
@codyoss codyoss changed the title spansql: support for Update in parseDMLStmt spanner/spansql: support for Update in parseDMLStmt Nov 9, 2020
@codyoss codyoss added api: spanner Issues related to the Spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Nov 9, 2020
@dsymonds dsymonds self-assigned this Nov 10, 2020
@dsymonds
Copy link
Contributor

Syntax types and parsing for UPDATE DML is now done in the spansql package. I'll be working on adding its implementation in spannertest next.

@dsymonds
Copy link
Contributor

Implementation is out for review.

You could test it locally real quick by running commands like these in your repo:

go mod edit -replace cloud.google.com/go/spanner=github.com/dsymonds/google-cloud-go/spanner@spannertest-update-dml
go mod download
go mod tidy
go test ./...

and chime in on #3201 if there's something significantly wrong for your use case.

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. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants