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): fill in missing hash functions #4808

Merged

Conversation

dfinkel
Copy link
Contributor

@dfinkel dfinkel commented Sep 24, 2021

The spansql package is quite useful for parsing and managing schemas,
however, with the addition of CHECK constraints to the spanner DDL,
the full set of SQL functions can now appear in schemas.

Fill in the missing Hash functions to support their use.

List of hash functions pulled from: https://cloud.google.com/spanner/docs/hash_functions

@dfinkel dfinkel requested review from hengfengli, skuruppu and a team as code owners Sep 24, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Cloud Spanner API. label Sep 24, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 24, 2021
@dfinkel dfinkel force-pushed the spansql_populate_hash_funcs branch 2 times, most recently from 5207d23 to d6c8d8f Compare Sep 24, 2021
The `spansql` package is quite useful for parsing and managing schemas,
however, with the addition of `CHECK` constraints to the spanner DDL,
the full set of SQL functions can now appear in schemas.

Fill in the missing Hash functions to support their use.
@hengfengli hengfengli requested a review from olavloite Sep 25, 2021
@hengfengli hengfengli changed the title spanner/spansql: Fill in missing hash functions feat(spanner/spansql): fill in missing hash functions Sep 25, 2021
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 26, 2021
Copy link
Contributor

@olavloite olavloite left a comment

LGTM.

Note that this will only add these functions to the list of functions that are recognized as being a function. They will still not return any values, so they cannot be used in queries.

@hengfengli hengfengli added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 26, 2021
@dfinkel
Copy link
Contributor Author

dfinkel commented Sep 27, 2021

Thanks @olavloite!

Yeah, when I glanced at which functions spannertest had implemented, I only saw one string-function (and it had to do some type-resolution that would have required a bit more work, so I didn't bother trying to implement any of these functions (looks like another's been added since then).

For my use-case, I want to include SHA256 in a CHECK constraint's expression. Adding the function to the list of recognized functions unblocks me adding that constraint, even if it's mostly ignored by spannertest. (I'll run most of my tests against the C++ emulator (and a real Cloud Spanner instance) anyway, because of other deficiencies in spannertest)

@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 27, 2021
@olavloite olavloite added the automerge Merge the pull request once unit tests and other checks pass. label Sep 27, 2021
@gcf-merge-on-green
Copy link
Contributor

gcf-merge-on-green bot commented Sep 27, 2021

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Sep 27, 2021
@hengfengli hengfengli added kokoro:force-run Add this label to force Kokoro to re-run the tests. automerge Merge the pull request once unit tests and other checks pass. labels Sep 28, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 28, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 37ee2d9 into googleapis:master Sep 28, 2021
4 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Sep 28, 2021
@dfinkel dfinkel deleted the spansql_populate_hash_funcs branch Sep 28, 2021
@dfinkel
Copy link
Contributor Author

dfinkel commented Sep 28, 2021

Thanks @hengfengli !

@hengfengli
Copy link
Contributor

hengfengli commented Sep 28, 2021

@dfinkel No worries. Thank you for your contribution!

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 Cloud 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