Add support for Base64 Encode/Decode Scalar Functions #9114

SabrinaZhaozyf · 2022-07-27T17:25:49Z

Label = feature

This PR implements toBase64(byte[]) and fromBase64(str) encoding and decoding functionality.

Encoding scheme: encodes input using the Base64 encoding scheme as specified in RFC 4648 and RFC 2045
Decoding will throw an exception if invalid base64 string is passed in.

Use Case Examples

Encode binary data: toBase64(binaryCol)
Encode strings in UTF8: toBase64(toUtf8('hello')) , toBase64(toUtf8(stringCol))
Decode base64-encoded strings to binary data: fromBase64('aGVsbG8h'), fromBase64(stringCol)
Decode base64-encoded strings to UTF8 charset: fromUtf8(fromBase64('aGVsbG8h'))

amrishlal · 2022-07-27T18:11:50Z

In the description, can we add a short overview of the issue being addressed along with a summary of changes.

pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

codecov-commenter · 2022-07-28T00:19:55Z

Codecov Report

Merging #9114 (1d0037e) into master (2676ffa) will decrease coverage by 3.00%.
The diff coverage is 38.76%.

❗ Current head 1d0037e differs from pull request most recent head ef0f3d2. Consider uploading reports for the commit ef0f3d2 to get more accurate results

@@             Coverage Diff              @@
##             master    #9114      +/-   ##
============================================
- Coverage     69.96%   66.96%   -3.01%     
- Complexity     4749     4800      +51     
============================================
  Files          1838     1378     -460     
  Lines         97350    70889   -26461     
  Branches      14672    11211    -3461     
============================================
- Hits          68113    47469   -20644     
+ Misses        24495    20018    -4477     
+ Partials       4742     3402    -1340

Flag	Coverage Δ
integration1	`?`
integration2	`?`
unittests1	`66.96% <38.76%> (+0.06%)`	⬆️
unittests2	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ava/org/apache/pinot/common/config/GrpcConfig.java	`0.00% <0.00%> (ø)`
...ache/pinot/common/metadata/ZKMetadataProvider.java	`15.16% <0.00%> (-52.07%)`	⬇️
...mmon/metadata/controllerjob/ControllerJobType.java	`0.00% <0.00%> (ø)`
...n/java/org/apache/pinot/common/utils/TlsUtils.java	`3.57% <0.00%> (-72.78%)`	⬇️
...pache/pinot/common/utils/grpc/GrpcQueryClient.java	`0.00% <0.00%> (-75.48%)`	⬇️
.../pinot/core/data/manager/BaseTableDataManager.java	`73.22% <ø> (-0.34%)`	⬇️
...ata/manager/realtime/RealtimeTableDataManager.java	`11.32% <0.00%> (-55.48%)`	⬇️
...re/operator/StreamingInstanceResponseOperator.java	`0.00% <0.00%> (-76.93%)`	⬇️
...t/core/plan/StreamingInstanceResponsePlanNode.java	`0.00% <0.00%> (-100.00%)`	⬇️
...che/pinot/core/transport/grpc/GrpcQueryServer.java	`0.00% <0.00%> (-51.32%)`	⬇️
... and 742 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

...roker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java

...roker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java

…gle arg

siddharthteotia · 2022-07-29T07:23:10Z

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

+   */
+  @ScalarFunction
+  public static String binaryToBase64(byte[] input) {
+    return Base64.getEncoder().encodeToString(input);


What is the expectation on charset of input bytes / binary data ? Looks like we assume it is UTF8. What if the input bytes provided here was encoded using UTF16

Resolved - we will not have a fixed charset of input bytes now with the new implementation. For example, if user wants to encode the String "hello!", he/she can choose to do toBase64(toUtf8("hello")) or toBase64(toUtf16("hello")) (Pinot currently only supports toUtf8 and toASCII functions). During decoding, he/she can do fromUtf8(fromBase64("fvduivheui")) to fromUtf16(fromBase64("fvduivheui")) to get "hello!" depending on if they used UTF8 or UTF16 when they encoded.

Do we error if the user passes a string to the toBase64 function, or some implicit type casting will happen? MySQL users might not expect the syntax of toBase64(toUtf8("hello")). Similarly, if the user calls on fromBase64('aGVsbG8h'), they might not expecting us to return hex string. We should at least call this out in the documentation.

Resolved - added tests for calling toBase64 on a string. Will add in documentation that BYTES col will be represented in HEX string(also link:https://docs.pinot.apache.org/users/user-guide-query/querying-pinot#bytes-column).

siddharthteotia · 2022-07-29T07:27:27Z

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

+   * @return decoded string
+   */
+  @ScalarFunction
+  public static String fromBase64(String input) {


If we are providing a toBase64 with a user specified charset encoding and a default utf8 charset flavor then shouldn't we also provide fromBase64 in both flavors as well ?

What if someone calls toBase64("blahblah", "UTF16") to get "something" and then fromBase64("something") - this will convert "something" to bytes and then to String using UTF8 which can be wrong because "something" was created from "blahblah" using UTF16.

Resolved - fromBase64 will decode into byte[] (the layer from byte[] to String is removed now) and the user can decide to encode the byte[] to UTF8 strings or other charsets by calling other functions such as fromUtf8.

siddharthteotia

Thanks for the contribution

siddharthteotia · 2022-08-02T16:14:19Z

@SabrinaZhaozyf - please update the PR description to reflect the latest discussion / approach

jasperjiaguo

Minor questions regarding invalid input / unexpected usage.

...on-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java

jasperjiaguo · 2022-08-02T16:39:55Z

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

+   */
+  @ScalarFunction
+  public static String binaryToBase64(byte[] input) {
+    return Base64.getEncoder().encodeToString(input);


Do we error if the user passes a string to the toBase64 function, or some implicit type casting will happen? MySQL users might not expect the syntax of toBase64(toUtf8("hello")). Similarly, if the user calls on fromBase64('aGVsbG8h'), they might not expecting us to return hex string. We should at least call this out in the documentation.

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java

siddharthteotia · 2022-08-02T21:07:11Z

@SabrinaZhaozyf - let's user docs to gitbook please

npawar · 2022-08-27T22:53:09Z

@SabrinaZhaozyf - let's user docs to gitbook please

Hello, was this added into the docs? I cannot find it in https://docs.pinot.apache.org/configuration-reference/functions

SabrinaZhaozyf · 2022-08-29T16:01:10Z

@npawar Changes to user doc has been merged here (pinot-contrib/pinot-docs#109). Not sure why it hasn't been updated on documentation yet.

npawar · 2022-08-29T18:08:06Z

@npawar Changes to user doc has been merged here (pinot-contrib/pinot-docs#109). Not sure why it hasn't been updated on documentation yet.

Oh i see. lemme check on that

siddharthteotia · 2022-10-14T05:52:14Z

@npawar - User docs were added some time ago by @SabrinaZhaozyf

Just linking them here for reference - https://docs.pinot.apache.org/configuration-reference/functions/base64

SabrinaZhaozyf changed the title ~~Added Base64 Encoding/Decoding Scalar Functions~~ [WIP] Added Base64 Encoding/Decoding Scalar Functions Jul 27, 2022

amrishlal reviewed Jul 27, 2022

View reviewed changes

pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java Outdated Show resolved Hide resolved

amrishlal reviewed Jul 27, 2022

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java Outdated Show resolved Hide resolved

amrishlal reviewed Jul 27, 2022

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java Outdated Show resolved Hide resolved

SabrinaZhaozyf force-pushed the yifzhao/base64 branch from 0f550c9 to 577c35e Compare July 27, 2022 21:12

siddharthteotia marked this pull request as ready for review July 27, 2022 21:15

SabrinaZhaozyf force-pushed the yifzhao/base64 branch from 93eab25 to cb9007b Compare July 27, 2022 23:52

SabrinaZhaozyf changed the title ~~[WIP] Added Base64 Encoding/Decoding Scalar Functions~~ Added Base64 Encoding/Decoding Scalar Functions Jul 27, 2022

Jackie-Jiang added the feature label Jul 28, 2022

Jackie-Jiang reviewed Jul 28, 2022

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java Outdated Show resolved Hide resolved

siddharthteotia reviewed Jul 28, 2022

View reviewed changes

...roker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java Outdated Show resolved Hide resolved

siddharthteotia reviewed Jul 28, 2022

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java Outdated Show resolved Hide resolved

siddharthteotia reviewed Jul 28, 2022

View reviewed changes

pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java Outdated Show resolved Hide resolved

siddharthteotia reviewed Jul 28, 2022

View reviewed changes

...roker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java Show resolved Hide resolved

SabrinaZhaozyf added 9 commits July 28, 2022 16:26

before testing on agg

05c8e99

added null case tests for encode/decode, added invalid check for decode

c23e635

roll back style changes

92e3451

minor changes

10ad711

roll back style changes

6991e74

added base64 with user encoding scheme, added nested tranfunc for sin…

33ec4d5

…gle arg

minor change

03046ff

tested binary to base64

372e13e

code reformat

7a40588

SabrinaZhaozyf force-pushed the yifzhao/base64 branch from ca55cbd to 7a40588 Compare July 29, 2022 00:29

siddharthteotia reviewed Jul 29, 2022

View reviewed changes

siddharthteotia changed the title ~~Added Base64 Encoding/Decoding Scalar Functions~~ Add support for Base64 Encode/Decode Scalar Functions Jul 29, 2022

simplied logic, editted all tests, before reformat

bc8173f

checked style

541aa6a

SabrinaZhaozyf requested review from siddharthteotia, Jackie-Jiang and amrishlal August 1, 2022 21:00

fixed format issue

17410d7

siddharthteotia approved these changes Aug 2, 2022

View reviewed changes

fixed format issue

9774236

jasperjiaguo requested changes Aug 2, 2022

View reviewed changes

added invalid arg test for toBase64

ef0f3d2

jasperjiaguo approved these changes Aug 2, 2022

View reviewed changes

siddharthteotia merged commit 913f0d8 into apache:master Aug 2, 2022

siddharthteotia mentioned this pull request Aug 2, 2022

Add support for regexpReplace scalar function #9123

Merged

siddharthteotia mentioned this pull request Oct 14, 2022

Add Base64 encode / decode functions #9594

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Base64 Encode/Decode Scalar Functions #9114

Add support for Base64 Encode/Decode Scalar Functions #9114

SabrinaZhaozyf commented Jul 27, 2022 •

edited

Loading

amrishlal commented Jul 27, 2022

codecov-commenter commented Jul 28, 2022 •

edited

Loading

siddharthteotia Jul 29, 2022

SabrinaZhaozyf Aug 1, 2022

jasperjiaguo Aug 2, 2022 •

edited

Loading

SabrinaZhaozyf Aug 2, 2022

siddharthteotia Jul 29, 2022 •

edited

Loading

SabrinaZhaozyf Aug 1, 2022

siddharthteotia left a comment

siddharthteotia commented Aug 2, 2022

jasperjiaguo left a comment

jasperjiaguo Aug 2, 2022 •

edited

Loading

siddharthteotia commented Aug 2, 2022

npawar commented Aug 27, 2022

SabrinaZhaozyf commented Aug 29, 2022

npawar commented Aug 29, 2022

siddharthteotia commented Oct 14, 2022

Add support for Base64 Encode/Decode Scalar Functions #9114

Add support for Base64 Encode/Decode Scalar Functions #9114

Conversation

SabrinaZhaozyf commented Jul 27, 2022 • edited Loading

amrishlal commented Jul 27, 2022

codecov-commenter commented Jul 28, 2022 • edited Loading

Codecov Report

siddharthteotia Jul 29, 2022

Choose a reason for hiding this comment

SabrinaZhaozyf Aug 1, 2022

Choose a reason for hiding this comment

jasperjiaguo Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

SabrinaZhaozyf Aug 2, 2022

Choose a reason for hiding this comment

siddharthteotia Jul 29, 2022 • edited Loading

Choose a reason for hiding this comment

SabrinaZhaozyf Aug 1, 2022

Choose a reason for hiding this comment

siddharthteotia left a comment

Choose a reason for hiding this comment

siddharthteotia commented Aug 2, 2022

jasperjiaguo left a comment

Choose a reason for hiding this comment

jasperjiaguo Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

siddharthteotia commented Aug 2, 2022

npawar commented Aug 27, 2022

SabrinaZhaozyf commented Aug 29, 2022

npawar commented Aug 29, 2022

siddharthteotia commented Oct 14, 2022

SabrinaZhaozyf commented Jul 27, 2022 •

edited

Loading

codecov-commenter commented Jul 28, 2022 •

edited

Loading

jasperjiaguo Aug 2, 2022 •

edited

Loading

siddharthteotia Jul 29, 2022 •

edited

Loading

jasperjiaguo Aug 2, 2022 •

edited

Loading