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): fix (date|timestamp) comparison parse. #4471

Closed
wants to merge 2 commits into from

Conversation

nktks
Copy link
Contributor

@nktks nktks commented Jul 21, 2021

This PR fixes two nit things.

  • fix (date|timestamp) comparison parse.
  • fix unstable SelectFromTable SQL

Second was made unstable tests and SQL output by #4457 (sorry.)

takeshi.nakata added 2 commits July 21, 2021 10:57
This fixes case when query has `date = @date` or `timestamp =
@timestamp` case.
Since date and timestamp are not reserved keyword, they can be used as
ID
googleapis#4457 made unstable SQL string, and fail test sometimes like this.
```
--- FAIL: TestSQL (0.00s)
    sql_test.go:410: {{false [A] [{Table  map[FORCE_INDEX:Idx GROUPBY_SCAN_OPTIMIZATION:TRUE]}] {4 B b <nil>} [] [] []} [] <nil> <nil>}.SQL() wrong.
         got SELECT A FROM Table@{GROUPBY_SCAN_OPTIMIZATION=TRUE,FORCE_INDEX=Idx} WHERE B = @b
        want SELECT A FROM Table@{FORCE_INDEX=Idx,GROUPBY_SCAN_OPTIMIZATION=TRUE} WHERE B = @b
```
Because map range result is unstable.
This PR makes the SQL output stable by sorting the Hints keys.
@nktks nktks requested review from hengfengli, skuruppu and a team as code owners July 21, 2021 02:19
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 21, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Jul 21, 2021
@nktks
Copy link
Contributor Author

nktks commented Jul 21, 2021

The first one is imcomplete, so I'll modify this PR only include failed test.

fix (date|timestamp) comparison parse.

So, I close this and made new PR #4473 only include fix test.

@nktks nktks closed this Jul 21, 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

1 participant