feat: add bigframes.bigquery.aead.* scalar functions#17168
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration file, aead.yaml, which defines several GoogleSQL scalar functions for AEAD encryption and decryption, including aead.decrypt_bytes, aead.decrypt_string, aead.encrypt, and their envelope counterparts. I have no feedback to provide.
|
Tested manually using the example at https://docs.cloud.google.com/bigquery/docs/reference/standard-sql/aead_encryption_functions#aeaddecrypt_bytes 
|
|
|
||
|
|
||
| def decrypt_bytes( | ||
| keyset: Union[T, bigframes.core.col.Expression, Union[bytes, dict]], |
There was a problem hiding this comment.
Is this "bigframes.core.col.Expression" necessary, given that it's already included in "T"?
There was a problem hiding this comment.
Yes. My intention is to allow a mix of types. Basically, a mix of Series and Expression should give Series. Just Expression should give Expression.
There was a problem hiding this comment.
I see your point. How about ditching the generic type entirely and spell out all possible types in the type hints? I don't think "T" brings us any significant advantages here. Plus, it makes mypy unhappy.
There was a problem hiding this comment.
It does bring a big advantage: mypy can determine when all are series, that it should return a series. The alternative is to return a Union type, but then we'd have to cast any time we use one of these functions.
| cast(bpd.Series, scalar_types_df["{{ arg.col_name }}"]), | ||
| {% endfor %} | ||
| ).to_frame() | ||
| snapshot.assert_match(result.sql.rstrip() + "\n", "out.sql") |
There was a problem hiding this comment.
nit: an empty line before the assert_match to demarcate the "act" and "assert" block
2 participants
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕