[Feature][Transform-V2] Add Base64 SQL functions#11114
Conversation
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I reviewed the latest head from the actual SQL transform execution path, not just from the function registration diff.
What this PR fixes
- User pain: SQL transform already has functions like
RAWTOHEX/HEXTORAW, but it still lacks a direct Base64 encode/decode pair for normal SQL expressions. - Fix approach: the PR registers
TO_BASE64/FROM_BASE64, wires their return type throughZetaSQLType, implements the runtime logic inStringFunction, and adds unit tests, SQL-level tests, E2E coverage, and docs. - One-line summary: this is a clean SQL string-function expansion that fills a real usability gap in the current function matrix.
Actual runtime path
SQLTransform execution
-> ZetaSQLFunction dispatch [ZetaSQLFunction.java:475-480]
-> TO_BASE64 / FROM_BASE64
-> StringFunction.toBase64 / fromBase64 [StringFunction.java:192-233]
-> string input
-> EncodingUtils.tryParseCharset(...)
-> charset-aware Base64 encode/decode
-> byte[] input
-> TO_BASE64 encodes raw bytes directly
-> bytes + charset is rejected explicitly
-> ZetaSQLType marks both functions as STRING [ZetaSQLType.java:327-334]
Review result
- I did not find a source-level blocker on the current head.
- The implementation is consistent with the existing SQL function architecture and stays nicely scoped.
- The important edge cases are covered: byte-array input, optional charset, invalid charset, invalid Base64 content, SQL-level invocation, and E2E output assertions.
- Compatibility looks good because this only adds new callable functions; existing queries do not change behavior.
CI note
Buildis still red, but the failing jobs currently shown areunit-test (8, windows-latest),jdbc-connectors-it-part-1, andconnector-file-sftp-it.- That failure set is much broader than the SQL transform files touched here. I could confirm the job names from the run metadata, but GitHub's log stream kept getting cancelled in this environment, so I cannot fully prove from logs yet that all three are unrelated. From the code path alone, I do not see a reason for this PR to break JDBC connector IT or SFTP connector IT.
Conclusion: can merge
Blocking items (must fix)
- No source-level blocking item from Daniel on the current head.
- The remaining gate is CI only: the contributor/maintainer still needs to clear the red
Buildchecks before GitHub can merge.
Suggested follow-up (non-blocking)
- No additional non-blocking suggestion from my side.
Overall assessment
- This is a well-scoped feature PR with solid test layering and matching docs.
- The runtime path is straightforward, and the new functions should be reached normally in SQL transform execution.
|
+1 if CI pass |
|
Thanks for checking. I agree with that read: from my side the source path on the current head looks good, so the remaining gate is just the CI result. |
davidzollo
left a comment
There was a problem hiding this comment.
+1
LGTM
Thanks for your contribution! This is a solid addition to the project, and your attention to detail really shows. Contributors like you are what keep this community moving forward. Feel free to keep the PRs coming — we're always glad to have you here!
Purpose of this pull request
Add Base64 conversion functions to SQLTransform.
TO_BASE64(value)TO_BASE64(value, charset)FROM_BASE64(value)FROM_BASE64(value, charset)The default charset is
UTF-8.TO_BASE64(value)also supportsbyte[]input.Closes #11107
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
incompatible-changes.mdto describe the incompatibility caused by this PR.