Skip to content

fix complex_decode_base64 function, add SQL bindings#13332

Merged
clintropolis merged 2 commits intoapache:masterfrom
clintropolis:decode_complex_base64
Nov 10, 2022
Merged

fix complex_decode_base64 function, add SQL bindings#13332
clintropolis merged 2 commits intoapache:masterfrom
clintropolis:decode_complex_base64

Conversation

@clintropolis
Copy link
Copy Markdown
Member

@clintropolis clintropolis commented Nov 9, 2022

Description

Fixes some bugs with the complex_decode_base64 function, which was basically broken before this PR for a few reasons, and migrates it to being implemented as an ExprMacro instead of a Function, so that the literal arguments can be computed externally to the function instead of every evaluation. This involves a slight adjustment to ExprMacroTable to add the concept of built-ins to make it a bit easier to add macros.

This PR also wires complex_decode_base64 up to SQL, previously it was only a native expression. I have purposefully still not documented this function yet to save room for future adjustments and determining the best way to position this functionality, since its use cases are currently a bit limited.

The improvements do allow some pretty funny/absurd things to work now, such as round-tripping complex types through JSON serialization and back into complex types all in the same expression as shown in one of the tests added:

SELECT APPROX_COUNT_DISTINCT_BUILTIN(COMPLEX_DECODE_BASE64('hyperUnique',PARSE_JSON(TO_JSON_STRING(unique_dim1)))) from druid.foo

😜


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

Copy link
Copy Markdown
Member

@rohangarg rohangarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the changes @clintropolis ! LGTM % comments

@clintropolis clintropolis merged commit 27215d1 into apache:master Nov 10, 2022
@clintropolis clintropolis deleted the decode_complex_base64 branch November 10, 2022 07:40
@kfaraz kfaraz added this to the 25.0 milestone Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants