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

remove druid.expressions.useStrictBooleans in favor of always being true #17568

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

clintropolis
Copy link
Member

@clintropolis clintropolis commented Dec 14, 2024

Description

driud.expressions.useStrictBooleans has been deprecated, this PR removes it effectively setting it to always be true. With this it means that Druid always handles boolean types as longs with 1 for true and 0 for false, and expression logical operations behave in sql compatible manner.

Related #17575

I have not updated the docs yet because docs on this are intertwined with discussion about druid.generic.useDefaultValueForNull and druid.generic.useThreeValueLogicForNativeFilters, so I will address them all in one shot in a follow-up after everything is removed.

Release note

Skipping writing the release note for just this PR, i think it makes sense to have a combined release note for when this, druid.generic.useDefaultValueForNull, and druid.generic.useThreeValueLogicForNativeFilters are all removed.

@@ -39,9 +34,6 @@ public class ExpressionProcessingConfig
"druid.expressions.homogenizeNullMultiValueStringArrays";
public static final String ALLOW_VECTORIZE_FALLBACK = "druid.expressions.allowVectorizeFallback";

@JsonProperty("useStrictBooleans")
private final boolean useStrictBooleans;
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, we should keep the config here, and use it for exactly one thing: to log a warning on startup if it's set to false. Just a little hint for people that upgrade without reading the release notes, and end up wondering why behavior changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, that's reasonable, will add it back

@clintropolis clintropolis merged commit a44ab10 into apache:master Dec 18, 2024
77 checks passed
@clintropolis clintropolis deleted the remove-strict-booleans branch December 20, 2024 06:46
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.

2 participants