remove druid.expressions.useStrictBooleans in favor of always being t…

…rue (#17568)

indexing-service/src/test/java/org/apache/druid/indexing/overlord/sampler/InputSourceSamplerDiscoveryTest.java

@@ -31,7 +31,6 @@
 import org.apache.druid.data.input.impl.StringDimensionSchema;
 import org.apache.druid.data.input.impl.TimestampSpec;
 import org.apache.druid.jackson.DefaultObjectMapper;
-import org.apache.druid.math.expr.ExpressionProcessing;
 import org.apache.druid.segment.AutoTypeColumnSchema;
 import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.column.RowSignature;
@@ -56,71 +55,6 @@ public class InputSourceSamplerDiscoveryTest extends InitializedNullHandlingTest
   );
   private InputSourceSampler inputSourceSampler = new InputSourceSampler(OBJECT_MAPPER);
 
-  @Test
-  public void testDiscoveredTypesNonStrictBooleans()
-  {
-
-    try {
-      ExpressionProcessing.initializeForStrictBooleansTests(false);
-      final InputSource inputSource = new InlineInputSource(Strings.join(STR_JSON_ROWS, '\n'));
-      final SamplerResponse response = inputSourceSampler.sample(
-          inputSource,
-          new JsonInputFormat(null, null, null, null, null),
-          DataSchema.builder()
-                    .withDataSource("test")
-                    .withTimestamp(new TimestampSpec("t", null, null))
-                    .withDimensions(DimensionsSpec.builder().useSchemaDiscovery(true).build())
-                    .build(),
-          null
-      );
-
-      Assert.assertEquals(6, response.getNumRowsRead());
-      Assert.assertEquals(5, response.getNumRowsIndexed());
-      Assert.assertEquals(6, response.getData().size());
-      Assert.assertEquals(
-          ImmutableList.of(
-              new StringDimensionSchema("string"),
-              new LongDimensionSchema("long"),
-              new DoubleDimensionSchema("double"),
-              new StringDimensionSchema("bool"),
-              new StringDimensionSchema("variant"),
-              new AutoTypeColumnSchema("array", null),
-              new AutoTypeColumnSchema("nested", null)
-          ),
-          response.getLogicalDimensions()
-      );
-
-      Assert.assertEquals(
-          ImmutableList.of(
-              new AutoTypeColumnSchema("string", null),
-              new AutoTypeColumnSchema("long", null),
-              new AutoTypeColumnSchema("double", null),
-              new AutoTypeColumnSchema("bool", null),
-              new AutoTypeColumnSchema("variant", null),
-              new AutoTypeColumnSchema("array", null),
-              new AutoTypeColumnSchema("nested", null)
-          ),
-          response.getPhysicalDimensions()
-      );
-      Assert.assertEquals(
-          RowSignature.builder()
-                      .addTimeColumn()
-                      .add("string", ColumnType.STRING)
-                      .add("long", ColumnType.LONG)
-                      .add("double", ColumnType.DOUBLE)
-                      .add("bool", ColumnType.STRING)
-                      .add("variant", ColumnType.STRING)
-                      .add("array", ColumnType.LONG_ARRAY)
-                      .add("nested", ColumnType.NESTED_DATA)
-                      .build(),
-          response.getLogicalSegmentSchema()
-      );
-    }
-    finally {
-      ExpressionProcessing.initializeForTests();
-    }
-  }
-
   @Test
   public void testDiscoveredTypesStrictBooleans()
   {

processing/src/main/java/org/apache/druid/common/config/NullHandling.java

@@ -22,7 +22,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.inject.Inject;
-import org.apache.druid.math.expr.ExpressionProcessing;
 import org.apache.druid.query.BitmapResultFactory;
 import org.apache.druid.query.filter.DimFilter;
 import org.apache.druid.query.filter.ValueMatcher;
@@ -130,8 +129,7 @@ public static boolean sqlCompatible()
   public static boolean useThreeValueLogic()
   {
     return sqlCompatible() &&
-           INSTANCE.isUseThreeValueLogicForNativeFilters() &&
-           ExpressionProcessing.useStrictBooleans();
+           INSTANCE.isUseThreeValueLogicForNativeFilters();
   }
 
   @Nullable

processing/src/main/java/org/apache/druid/math/expr/BinaryEvalOpExprBase.java

@@ -23,7 +23,6 @@
 import org.apache.druid.common.config.NullHandling;
 import org.apache.druid.java.util.common.IAE;
 import org.apache.druid.java.util.common.StringUtils;
-import org.apache.druid.segment.column.Types;
 
 import javax.annotation.Nullable;
 import java.util.Objects;
@@ -206,9 +205,6 @@ public ExprEval eval(ObjectBinding bindings)
         result = evalDouble(leftVal.asDouble(), rightVal.asDouble());
         break;
     }
-    if (!ExpressionProcessing.useStrictBooleans() && !type.is(ExprType.STRING) && !type.isArray()) {
-      return ExprEval.ofBoolean(result, type);
-    }
     return ExprEval.ofLongBoolean(result);
   }
 
@@ -224,11 +220,7 @@ public ExprEval eval(ObjectBinding bindings)
   @Override
   public ExpressionType getOutputType(InputBindingInspector inspector)
   {
-    ExpressionType implicitCast = super.getOutputType(inspector);
-    if (ExpressionProcessing.useStrictBooleans() || Types.isNullOr(implicitCast, ExprType.STRING)) {
-      return ExpressionType.LONG;
-    }
-    return implicitCast;
+    return ExpressionType.LONG;
   }
 
   @Override

processing/src/main/java/org/apache/druid/math/expr/BinaryLogicalOperatorExpr.java

@@ -340,9 +340,6 @@ protected BinaryOpExprBase copy(Expr left, Expr right)
   public ExprEval eval(ObjectBinding bindings)
   {
     ExprEval leftVal = left.eval(bindings);
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      return leftVal.asBoolean() ? right.eval(bindings) : leftVal;
-    }
 
     // if left is false, always false
     if (leftVal.value() != null && !leftVal.asBoolean()) {
@@ -376,9 +373,7 @@ public ExprEval eval(ObjectBinding bindings)
   @Override
   public boolean canVectorize(InputBindingInspector inspector)
   {
-    return ExpressionProcessing.useStrictBooleans() &&
-           inspector.areSameTypes(left, right) &&
-           inspector.canVectorize(left, right);
+    return inspector.areSameTypes(left, right) && inspector.canVectorize(left, right);
   }
 
   @Override
@@ -391,9 +386,6 @@ public <T> ExprVectorProcessor<T> asVectorProcessor(VectorInputBindingInspector
   @Override
   public ExpressionType getOutputType(InputBindingInspector inspector)
   {
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      return super.getOutputType(inspector);
-    }
     return ExpressionType.LONG;
   }
 }
@@ -415,9 +407,6 @@ protected BinaryOpExprBase copy(Expr left, Expr right)
   public ExprEval eval(ObjectBinding bindings)
   {
     ExprEval leftVal = left.eval(bindings);
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      return leftVal.asBoolean() ? leftVal : right.eval(bindings);
-    }
 
     // if left is true, always true
     if (leftVal.value() != null && leftVal.asBoolean()) {
@@ -454,9 +443,7 @@ public ExprEval eval(ObjectBinding bindings)
   public boolean canVectorize(InputBindingInspector inspector)
   {
 
-    return ExpressionProcessing.useStrictBooleans() &&
-           inspector.areSameTypes(left, right) &&
-           inspector.canVectorize(left, right);
+    return inspector.areSameTypes(left, right) && inspector.canVectorize(left, right);
   }
 
   @Override
@@ -469,9 +456,6 @@ public <T> ExprVectorProcessor<T> asVectorProcessor(VectorInputBindingInspector
   @Override
   public ExpressionType getOutputType(InputBindingInspector inspector)
   {
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      return super.getOutputType(inspector);
-    }
     return ExpressionType.LONG;
   }
 }

processing/src/main/java/org/apache/druid/math/expr/ExprEval.java

@@ -242,11 +242,7 @@ private static Class convertType(@Nullable Class existing, Class next)
     if (Number.class.isAssignableFrom(next) || next == String.class || next == Boolean.class) {
       // coerce booleans
       if (next == Boolean.class) {
-        if (ExpressionProcessing.useStrictBooleans()) {
-          next = Long.class;
-        } else {
-          next = String.class;
-        }
+        next = Long.class;
       }
       if (existing == null) {
         return next;
@@ -350,28 +346,6 @@ public static ExprEval ofArray(ExpressionType outputType, @Nullable Object[] val
     return new ArrayExprEval(outputType, value);
   }
 
-  /**
-   * Convert a boolean back into native expression type
-   *
-   * Do not use this method unless {@link ExpressionProcessing#useStrictBooleans()} is set to false.
-   * {@link ExpressionType#LONG} is the Druid boolean unless this mode is enabled, so use {@link #ofLongBoolean}
-   * instead.
-   */
-  @Deprecated
-  public static ExprEval ofBoolean(boolean value, ExpressionType type)
-  {
-    switch (type.getType()) {
-      case DOUBLE:
-        return of(Evals.asDouble(value));
-      case LONG:
-        return ofLongBoolean(value);
-      case STRING:
-        return of(String.valueOf(value));
-      default:
-        throw new Types.InvalidCastBooleanException(type);
-    }
-  }
-
   /**
    * Convert a boolean into a long expression type
    */
@@ -421,10 +395,7 @@ public static ExprEval bestEffortOf(@Nullable Object val)
       return new LongExprEval((Number) val);
     }
     if (val instanceof Boolean) {
-      if (ExpressionProcessing.useStrictBooleans()) {
-        return ofLongBoolean((Boolean) val);
-      }
-      return new StringExprEval(String.valueOf(val));
+      return ofLongBoolean((Boolean) val);
     }
     if (val instanceof Long[]) {
       final Long[] inputArray = (Long[]) val;

processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessing.java

@@ -48,12 +48,6 @@ public static void initializeForTests()
     INSTANCE = new ExpressionProcessingConfig(null, null, null, null);
   }
 
-  @VisibleForTesting
-  public static void initializeForStrictBooleansTests(boolean useStrict)
-  {
-    INSTANCE = new ExpressionProcessingConfig(useStrict, null, null, null);
-  }
-
   @VisibleForTesting
   public static void initializeForHomogenizeNullMultiValueStrings()
   {
@@ -66,15 +60,6 @@ public static void initializeForFallback()
     INSTANCE = new ExpressionProcessingConfig(null, null, null, true);
   }
 
-  /**
-   * All boolean expressions are {@link ExpressionType#LONG}
-   */
-  public static boolean useStrictBooleans()
-  {
-    checkInitialized();
-    return INSTANCE.isUseStrictBooleans();
-  }
-
   /**
    * All {@link ExprType#ARRAY} values will be converted to {@link ExpressionType#STRING} by their column selectors
    * (not within expression processing) to be treated as multi-value strings instead of native arrays.

processing/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java

@@ -30,6 +30,7 @@ public class ExpressionProcessingConfig
 {
   private static final Logger LOG = new Logger(ExpressionProcessingConfig.class);
 
+  @Deprecated
   public static final String NULL_HANDLING_LEGACY_LOGICAL_OPS_STRING = "druid.expressions.useStrictBooleans";
   // Coerce arrays to multi value strings
   public static final String PROCESS_ARRAYS_AS_MULTIVALUE_STRINGS_CONFIG_STRING =
@@ -39,9 +40,6 @@ public class ExpressionProcessingConfig
       "druid.expressions.homogenizeNullMultiValueStringArrays";
   public static final String ALLOW_VECTORIZE_FALLBACK = "druid.expressions.allowVectorizeFallback";
 
-  @JsonProperty("useStrictBooleans")
-  private final boolean useStrictBooleans;
-
   @JsonProperty("processArraysAsMultiValueStrings")
   private final boolean processArraysAsMultiValueStrings;
 
@@ -51,9 +49,13 @@ public class ExpressionProcessingConfig
   @JsonProperty("allowVectorizeFallback")
   private final boolean allowVectorizeFallback;
 
+  @Deprecated
+  @JsonProperty("useStrictBooleans")
+  private final boolean useStrictBooleans;
+
   @JsonCreator
   public ExpressionProcessingConfig(
-      @JsonProperty("useStrictBooleans") @Nullable Boolean useStrictBooleans,
+      @Deprecated @JsonProperty("useStrictBooleans") @Nullable Boolean useStrictBooleans,
       @JsonProperty("processArraysAsMultiValueStrings") @Nullable Boolean processArraysAsMultiValueStrings,
       @JsonProperty("homogenizeNullMultiValueStringArrays") @Nullable Boolean homogenizeNullMultiValueStringArrays,
       @JsonProperty("allowVectorizeFallback") @Nullable Boolean allowVectorizeFallback
@@ -83,17 +85,12 @@ public ExpressionProcessingConfig(
     final String docsBaseFormat = "https://druid.apache.org/docs/%s/querying/sql-data-types#%s";
     if (!this.useStrictBooleans) {
       LOG.warn(
-          "druid.expressions.useStrictBooleans set to 'false', we recommend using 'true' if using SQL to query Druid for the most SQL compliant behavior, see %s for details",
+          "druid.expressions.useStrictBooleans set to 'false', but has been removed from Druid and is always 'true' now for the most SQL compliant behavior, see %s for details",
           StringUtils.format(docsBaseFormat, version, "boolean-logic")
       );
     }
   }
 
-  public boolean isUseStrictBooleans()
-  {
-    return useStrictBooleans;
-  }
-
   public boolean processArraysAsMultiValueStrings()
   {
     return processArraysAsMultiValueStrings;

processing/src/main/java/org/apache/druid/math/expr/UnaryOperatorExpr.java

@@ -26,7 +26,6 @@
 import org.apache.druid.math.expr.vector.ExprVectorProcessor;
 import org.apache.druid.math.expr.vector.VectorMathProcessors;
 import org.apache.druid.math.expr.vector.VectorProcessors;
-import org.apache.druid.segment.column.Types;
 
 import javax.annotation.Nullable;
 import java.math.BigInteger;
@@ -181,25 +180,13 @@ public ExprEval eval(ObjectBinding bindings)
     if (NullHandling.sqlCompatible() && (ret.value() == null)) {
       return ExprEval.of(null);
     }
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      // conforming to other boolean-returning binary operators
-      ExpressionType retType = ret.type().is(ExprType.DOUBLE) ? ExpressionType.DOUBLE : ExpressionType.LONG;
-      return ExprEval.ofBoolean(!ret.asBoolean(), retType);
-    }
     return ExprEval.ofLongBoolean(!ret.asBoolean());
   }
 
   @Nullable
   @Override
   public ExpressionType getOutputType(InputBindingInspector inspector)
   {
-    if (!ExpressionProcessing.useStrictBooleans()) {
-      ExpressionType implicitCast = super.getOutputType(inspector);
-      if (Types.is(implicitCast, ExprType.STRING)) {
-        return ExpressionType.LONG;
-      }
-      return implicitCast;
-    }
     return ExpressionType.LONG;
   }