-
Notifications
You must be signed in to change notification settings - Fork 3.8k
SQL compatible Null Handling Part 1 - Expressions and Storage Changes #5278
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
Changes from all commits
226794f
2c78fa5
6e956a6
961d694
aaba00b
c5a5d65
606b980
3da96e4
1054d6b
6ef4d68
5564b18
c0bf599
182f6ee
b685a92
54b91f4
63252ab
c75ff2d
4def13d
03b9757
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| /* | ||
| * Licensed to Metamarkets Group Inc. (Metamarkets) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. Metamarkets licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package io.druid.common.config; | ||
|
|
||
| import com.google.common.base.Strings; | ||
| import com.google.inject.Inject; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * Helper class for NullHandling. This class is used to switch between SQL compatible Null Handling behavior | ||
| * introduced as part of https://github.com/druid-io/druid/issues/4349 and the old druid behavior | ||
| * where null values are replaced with default values e.g Null Strings are replaced with empty values. | ||
| */ | ||
| public class NullHandling | ||
| { | ||
| public static String NULL_HANDLING_CONFIG_STRING = "druid.generic.useDefaultValueForNull"; | ||
|
|
||
| /** | ||
| * use these values to ensure that {@link NullHandling#defaultDoubleValue()}, | ||
| * {@link NullHandling#defaultFloatValue()} , {@link NullHandling#defaultFloatValue()} | ||
| * return the same boxed object when returning a constant zero | ||
| */ | ||
| public static final Double ZERO_DOUBLE = 0.0d; | ||
| public static final Float ZERO_FLOAT = 0.0f; | ||
| public static final Long ZERO_LONG = 0L; | ||
|
|
||
| /** | ||
| * INSTANCE is injected using static injection to avoid adding JacksonInject annotations all over the code. | ||
| * See io.druid.guice.NullHandlingModule for details. | ||
| * It does not take effect in all unit tests since we don't use Guice Injection. | ||
| * For tests default system property is supposed to be used only in tests | ||
| */ | ||
| @Inject | ||
| private static NullValueHandlingConfig INSTANCE = new NullValueHandlingConfig( | ||
| Boolean.valueOf(System.getProperty(NULL_HANDLING_CONFIG_STRING, "true")) | ||
| ); | ||
|
|
||
| public static boolean replaceWithDefault() | ||
| { | ||
| return INSTANCE.isUseDefaultValuesForNull(); | ||
| } | ||
|
|
||
| public static boolean sqlCompatible() | ||
| { | ||
| return !replaceWithDefault(); | ||
| } | ||
|
|
||
| @Nullable | ||
| public static String nullToEmptyIfNeeded(@Nullable String value) | ||
| { | ||
| //CHECKSTYLE.OFF: Regexp | ||
| return replaceWithDefault() ? Strings.nullToEmpty(value) : value; | ||
| //CHECKSTYLE.ON: Regexp | ||
| } | ||
|
|
||
| @Nullable | ||
| public static String emptyToNullIfNeeded(@Nullable String value) | ||
| { | ||
| //CHECKSTYLE.OFF: Regexp | ||
| return replaceWithDefault() ? Strings.emptyToNull(value) : value; | ||
| //CHECKSTYLE.ON: Regexp | ||
| } | ||
|
|
||
| @Nullable | ||
| public static String defaultStringValue() | ||
| { | ||
| return replaceWithDefault() ? "" : null; | ||
| } | ||
|
|
||
| @Nullable | ||
| public static Long defaultLongValue() | ||
| { | ||
| return replaceWithDefault() ? ZERO_LONG : null; | ||
| } | ||
|
|
||
| @Nullable | ||
| public static Float defaultFloatValue() | ||
| { | ||
| return replaceWithDefault() ? ZERO_FLOAT : null; | ||
| } | ||
|
|
||
| @Nullable | ||
| public static Double defaultDoubleValue() | ||
| { | ||
| return replaceWithDefault() ? ZERO_DOUBLE : null; | ||
| } | ||
|
|
||
| public static boolean isNullOrEquivalent(@Nullable String value) | ||
| { | ||
| return replaceWithDefault() ? Strings.isNullOrEmpty(value) : value == null; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * Licensed to Metamarkets Group Inc. (Metamarkets) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. Metamarkets licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package io.druid.common.config; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| public class NullValueHandlingConfig | ||
| { | ||
|
|
||
| @JsonProperty("useDefaultValueForNull") | ||
| private final boolean useDefaultValuesForNull; | ||
|
|
||
| @JsonCreator | ||
| public NullValueHandlingConfig(@JsonProperty("useDefaultValueForNull") Boolean useDefaultValuesForNull) | ||
| { | ||
| this.useDefaultValuesForNull = useDefaultValuesForNull == null | ||
| ? true | ||
| : useDefaultValuesForNull; | ||
| } | ||
|
|
||
| public boolean isUseDefaultValuesForNull() | ||
| { | ||
| return useDefaultValuesForNull; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,12 +20,14 @@ | |
| package io.druid.math.expr; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.base.Strings; | ||
| import com.google.common.primitives.Doubles; | ||
| import com.google.common.primitives.Ints; | ||
| import io.druid.common.config.NullHandling; | ||
| import io.druid.common.guava.GuavaUtils; | ||
| import io.druid.java.util.common.IAE; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| */ | ||
| public abstract class ExprEval<T> | ||
|
|
@@ -50,7 +52,7 @@ public static ExprEval of(double doubleValue) | |
| return new DoubleExprEval(doubleValue); | ||
| } | ||
|
|
||
| public static ExprEval of(String stringValue) | ||
| public static ExprEval of(@Nullable String stringValue) | ||
| { | ||
| return new StringExprEval(stringValue); | ||
| } | ||
|
|
@@ -108,6 +110,7 @@ public boolean isNull() | |
|
|
||
| public abstract double asDouble(); | ||
|
|
||
| @Nullable | ||
| public String asString() | ||
| { | ||
| return value == null ? null : String.valueOf(value); | ||
|
|
@@ -228,9 +231,9 @@ public Expr toExpr() | |
|
|
||
| private static class StringExprEval extends ExprEval<String> | ||
| { | ||
| private StringExprEval(String value) | ||
| private StringExprEval(@Nullable String value) | ||
| { | ||
| super(Strings.emptyToNull(value)); | ||
| super(NullHandling.emptyToNullIfNeeded(value)); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -243,10 +246,12 @@ public final ExprType type() | |
| public final int asInt() | ||
| { | ||
| if (value == null) { | ||
| assert NullHandling.replaceWithDefault(); | ||
| return 0; | ||
| } | ||
|
|
||
| final Integer theInt = Ints.tryParse(value); | ||
| assert NullHandling.replaceWithDefault() || theInt != null; | ||
| return theInt == null ? 0 : theInt; | ||
| } | ||
|
|
||
|
|
@@ -255,17 +260,20 @@ public final long asLong() | |
| { | ||
| // GuavaUtils.tryParseLong handles nulls, no need for special null handling here. | ||
| final Long theLong = GuavaUtils.tryParseLong(value); | ||
| assert NullHandling.replaceWithDefault() || theLong != null; | ||
| return theLong == null ? 0L : theLong; | ||
| } | ||
|
|
||
| @Override | ||
| public final double asDouble() | ||
| { | ||
| if (value == null) { | ||
| assert NullHandling.replaceWithDefault(); | ||
| return 0.0; | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please review the rest of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
| final Double theDouble = Doubles.tryParse(value); | ||
| assert NullHandling.replaceWithDefault() || theDouble != null; | ||
| return theDouble == null ? 0.0 : theDouble; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need check in line 257
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added.