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

add backwards compatibility mode for multi-value string array null value coercion #12210

Conversation

clintropolis
Copy link
Member

Description

Follow up to #12078, adds druid.expressions.homogenizeNullMultiValueStringArrays with a default value of 'false'. If set to true, it opens an escape hatch to a backwards compatibility mode for the 'buggy' behavior of some multi-value string array functions such as MV_APPEND which was fixed in #12078. Prior to that PR, all null, [], and [null] values were treated as [null], which was only intended for implicitly mapped single value expression across multi-valued string rows, and could produce unintuitive results. This option has been added in case anyone happened to be depending on that behavior, but it produces odd behavior that impacts all array expressions, so it is not recommended to set this flag to true if possible, and instead add a workaround by coercing the values explicitly with additional expressions if needed.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • 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
Contributor

@cryptoe cryptoe left a comment

Choose a reason for hiding this comment

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

Failures look unrelated.
LGTM
[ERROR] ITIndexerTest.testGetLockedIntervals:364->AbstractIndexerTest.lambda$unloader$0:71->AbstractIndexerTest.unloadAndKillData:91 » Runtime

// expression has array output
&& plan.is(ExpressionPlan.Trait.NON_SCALAR_OUTPUT);

final boolean homogenizeNullMultiValueStringArrays =
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe move this inside the if statement at 281?

@clintropolis clintropolis merged commit f9b406c into apache:master Feb 1, 2022
@clintropolis clintropolis deleted the multi-value-array-null-coercion-compat branch February 1, 2022 06:38
@abhishekagarwal87 abhishekagarwal87 added this to the 0.23.0 milestone May 11, 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.

4 participants