From 62b4de5b5d3dd8390b59720ccd798d12f4deb724 Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Wed, 6 May 2020 00:14:32 -0700 Subject: [PATCH 1/7] Fail incorrectly constructed join queries --- .../join/HashJoinSegmentStorageAdapter.java | 9 +++ .../HashJoinSegmentStorageAdapterTest.java | 37 +++++++++ .../druid/sql/calcite/CalciteQueryTest.java | 78 +++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java index c854ac89c610..65c25d498b56 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java @@ -21,6 +21,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.java.util.common.guava.Sequences; @@ -46,6 +47,7 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -207,6 +209,13 @@ public Sequence makeCursors( @Nullable final QueryMetrics queryMetrics ) { + if (!Objects.equals(joinFilterPreAnalysis.getOriginalFilter(), filter)) { + throw new ISE( + "Filter provided to cursor [%s] does not match join pre-analysis filter [%s]", + filter, + joinFilterPreAnalysis.getOriginalFilter() + ); + } final List preJoinVirtualColumns = new ArrayList<>(); final List postJoinVirtualColumns = new ArrayList<>(); diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index a9b02e24a12d..04df55417070 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.Lists; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; @@ -1598,4 +1599,40 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRows() ImmutableList.of() ); } + + @Test + public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowISE() + { + List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); + + JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( + joinableClauses, + VirtualColumns.EMPTY, + null, + true, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + ); + Filter filter = new SelectorFilter("page", "this matches nothing"); + + try { + new HashJoinSegmentStorageAdapter( + factSegment.asStorageAdapter(), + joinableClauses, + preAnalysis + ).makeCursors( + filter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ); + Assert.fail(); + } + catch (ISE e) { + Assert.assertTrue(e.getMessage().startsWith("Filter provided to cursor [")); + } + } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 5a16eb6bb11e..e15424e14666 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -27,6 +27,7 @@ import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.JodaUtils; import org.apache.druid.java.util.common.StringUtils; @@ -10966,6 +10967,83 @@ public void testTimeExtractWithTooFewArguments() throws Exception testQuery("SELECT TIME_EXTRACT(__time) FROM druid.foo", ImmutableList.of(), ImmutableList.of()); } + @Test + public void testNestedGroupByOnInlineDataSourceWithFilterIsNotSupported() throws Exception + { + try { + testQuery( + "with abc as" + + "(" + + " SELECT dim1, m2 from druid.foo where \"__time\" >= '2001-01-02'" + + ")" + + ", def as" + + "(" + + " SELECT t1.dim1, SUM(t2.m2) as \"metricSum\" " + + " from abc as t1 inner join abc as t2 on t1.dim1 = t2.dim1" + + " where t1.dim1='def'" + + " group by 1" + + ")" + + "SELECT count(*) from def", + ImmutableList.of( + GroupByQuery + .builder() + .setDataSource( + GroupByQuery + .builder() + .setDataSource( + join( + new QueryDataSource( + newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Intervals.of("2001-01-02T00:00:00.000Z/146140482-04-24T15:36:27.903Z"))) + .columns("dim1", "m2") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + new QueryDataSource( + newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Intervals.of("2001-01-02T00:00:00.000Z/146140482-04-24T15:36:27.903Z"))) + .columns("dim1", "m2") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + "j0", + equalsCondition( + DruidExpression.fromColumn("dim1"), + DruidExpression.fromColumn("j0.dim1") + ), + JoinType.INNER + ) + ) + .setGranularity(Granularities.ALL) + .setInterval(querySegmentSpec(Filtration.eternity())) + .build() + ) + .setGranularity(Granularities.ALL) + .setInterval(querySegmentSpec(Filtration.eternity())) + .build() + ), + ImmutableList.of(new Object[] {1}) + ); + Assert.fail("Expected an ISE to be thrown"); + } + catch (RuntimeException e) { + Throwable cause = e.getCause(); + boolean foundISE = false; + while (cause != null) { + if (cause instanceof ISE) { + foundISE = true; + break; + } + cause = cause.getCause(); + } + Assert.assertTrue(foundISE); + } + } + @Test public void testUsingSubqueryAsFilterOnTwoColumns() throws Exception { From cb8b2b265fd6e5fd0ee68e9f413d8c5e0b8f289e Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Wed, 6 May 2020 15:11:54 -0700 Subject: [PATCH 2/7] wip annotation for equals implementations --- .../SubclassesMustOverrideEquals.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java diff --git a/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java b/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java new file mode 100644 index 000000000000..c37a8c954814 --- /dev/null +++ b/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.annotations; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * An annotation that tells all subclasses of the annotated class to override equals. + */ +@Documented +@Inherited +@Retention(RetentionPolicy.SOURCE) +@Target(ElementType.TYPE) +public @interface SubclassesMustOverrideEquals +{ +} From aa223780f067778a5fdf106adeab0c7e72d7059e Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Fri, 8 May 2020 18:00:16 -0700 Subject: [PATCH 3/7] Add equals tests --- ...classesMustOverrideEqualsAndHashCode.java} | 9 +- .../java/org/apache/druid/math/expr/Expr.java | 249 ++++++++++++++++++ .../druid/math/expr/ExprMacroTable.java | 41 +++ .../org/apache/druid/math/expr/ExprTest.java | 190 +++++++++++++ processing/pom.xml | 6 + .../collections/spatial/search/Bound.java | 2 + .../spatial/search/PolygonBound.java | 27 ++ .../spatial/search/RectangularBound.java | 27 ++ .../druid/query/dimension/DimensionSpec.java | 2 + .../expression/TimestampCeilExprMacro.java | 30 ++- .../expression/TimestampFloorExprMacro.java | 23 ++ .../druid/query/expression/TrimExprMacro.java | 56 +++- .../query/extraction/CascadeExtractionFn.java | 51 ++-- .../druid/query/extraction/ExtractionFn.java | 2 + .../query/extraction/LowerExtractionFn.java | 21 ++ .../query/extraction/StrlenExtractionFn.java | 12 + .../query/extraction/UpperExtractionFn.java | 21 ++ .../query/filter/DruidPredicateFactory.java | 2 + .../org/apache/druid/query/filter/Filter.java | 2 + .../druid/query/filter/FilterTuning.java | 2 + .../druid/query/filter/LikeDimFilter.java | 121 ++++++--- .../filter/SelectorPredicateFactory.java | 20 ++ .../druid/query/search/SearchQuerySpec.java | 2 + .../druid/segment/filter/BoundFilter.java | 161 ++++++----- .../filter/ColumnComparisonFilter.java | 20 ++ .../filter/DimensionPredicateFilter.java | 191 +++++++++----- .../segment/filter/ExpressionFilter.java | 22 ++ .../druid/segment/filter/FalseFilter.java | 12 + .../apache/druid/segment/filter/InFilter.java | 154 +++++++---- .../segment/filter/JavaScriptFilter.java | 22 ++ .../druid/segment/filter/LikeFilter.java | 23 ++ .../druid/segment/filter/RegexFilter.java | 124 ++++++--- .../segment/filter/SearchQueryFilter.java | 84 ++++-- .../druid/segment/filter/SpatialFilter.java | 123 ++++++--- .../druid/segment/filter/TrueFilter.java | 12 + ...ssesMustOverrideEqualsAndHashCodeTest.java | 66 +++++ .../spatial/search/PolygonBoundTest.java | 9 + .../spatial/search/RectangularBoundTest.java | 9 + .../TimestampCeilExprMacroTest.java | 44 ++++ .../TimestampFloorExprMacroTest.java | 44 ++++ .../query/expression/TrimExprMacroTest.java | 43 +++ .../extraction/CascadeExtractionFnTest.java | 38 ++- .../extraction/LowerExtractionFnTest.java | 9 + .../extraction/StrlenExtractionFnTest.java | 7 + .../extraction/UpperExtractionFnTest.java | 9 + .../druid/query/filter/LikeDimFilterTest.java | 12 +- .../filter/SelectorPredicateFactoryTest.java | 37 +++ .../druid/segment/filter/BoundFilterTest.java | 10 +- .../filter/ColumnComparisonFilterTest.java | 9 + .../filter/DimensionPredicateFilterTest.java | 43 +++ .../segment/filter/ExpressionFilterTest.java | 9 + .../druid/segment/filter/FalseFilterTest.java | 33 +++ .../druid/segment/filter/InFilterTest.java | 10 + .../segment/filter/JavaScriptFilterTest.java | 8 + .../druid/segment/filter/RegexFilterTest.java | 17 ++ .../segment/filter/SearchQueryFilterTest.java | 15 ++ .../segment/filter/SpatialFilterTest.java | 16 +- .../druid/segment/filter/TrueFilterTest.java | 33 +++ .../IncrementalIndexStorageAdapterTest.java | 7 + .../HashJoinSegmentStorageAdapterTest.java | 2 +- 60 files changed, 2046 insertions(+), 359 deletions(-) rename core/src/main/java/org/apache/druid/annotations/{SubclassesMustOverrideEquals.java => SubclassesMustOverrideEqualsAndHashCode.java} (79%) create mode 100644 core/src/test/java/org/apache/druid/math/expr/ExprTest.java create mode 100644 processing/src/test/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCodeTest.java create mode 100644 processing/src/test/java/org/apache/druid/query/expression/TimestampCeilExprMacroTest.java create mode 100644 processing/src/test/java/org/apache/druid/query/expression/TimestampFloorExprMacroTest.java create mode 100644 processing/src/test/java/org/apache/druid/query/expression/TrimExprMacroTest.java create mode 100644 processing/src/test/java/org/apache/druid/query/filter/SelectorPredicateFactoryTest.java create mode 100644 processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java create mode 100644 processing/src/test/java/org/apache/druid/segment/filter/FalseFilterTest.java create mode 100644 processing/src/test/java/org/apache/druid/segment/filter/TrueFilterTest.java diff --git a/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java b/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCode.java similarity index 79% rename from core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java rename to core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCode.java index c37a8c954814..a53e81662d80 100644 --- a/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEquals.java +++ b/core/src/main/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCode.java @@ -27,12 +27,15 @@ import java.lang.annotation.Target; /** - * An annotation that tells all subclasses of the annotated class to override equals. + * An annotation that tells all subclasses of the annotated class to not use the default implementation of hashCode + * and equals that is provided by {@link Object}. + * + * This annotation is useful on classes that you expect will be used in equals checks in other parts of the codebase. */ @Documented @Inherited -@Retention(RetentionPolicy.SOURCE) +@Retention(RetentionPolicy.CLASS) @Target(ElementType.TYPE) -public @interface SubclassesMustOverrideEquals +public @interface SubclassesMustOverrideEqualsAndHashCode { } diff --git a/core/src/main/java/org/apache/druid/math/expr/Expr.java b/core/src/main/java/org/apache/druid/math/expr/Expr.java index 2911c7f95342..28544e239c08 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Expr.java +++ b/core/src/main/java/org/apache/druid/math/expr/Expr.java @@ -27,6 +27,7 @@ import com.google.common.math.LongMath; import com.google.common.primitives.Ints; import org.apache.commons.lang.StringEscapeUtils; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; @@ -46,6 +47,7 @@ * Base interface of Druid expression language abstract syntax tree nodes. All {@link Expr} implementations are * immutable. */ +@SubclassesMustOverrideEqualsAndHashCode public interface Expr { String NULL_LITERAL = "null"; @@ -494,6 +496,18 @@ public String toString() { return NULL_LITERAL; } + + @Override + public int hashCode() + { + return getClass().hashCode(); + } + + @Override + public boolean equals(Object obj) + { + return obj != null && getClass().equals(obj.getClass()); + } } class LongExpr extends ConstantExpr @@ -522,6 +536,25 @@ public ExprEval eval(ObjectBinding bindings) { return ExprEval.ofLong(value); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LongExpr longExpr = (LongExpr) o; + return Objects.equals(value, longExpr.value); + } + + @Override + public int hashCode() + { + return Objects.hash(value); + } } class NullLongExpr extends NullNumericConstantExpr @@ -569,6 +602,25 @@ public String stringify() } return StringUtils.format("%s", toString()); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LongArrayExpr that = (LongArrayExpr) o; + return Arrays.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Arrays.hashCode(value); + } } class StringExpr extends ConstantExpr @@ -606,6 +658,25 @@ public String stringify() // escape as javascript string since string literals are wrapped in single quotes return value == null ? NULL_LITERAL : StringUtils.format("'%s'", StringEscapeUtils.escapeJavaScript(value)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + StringExpr that = (StringExpr) o; + return Objects.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Objects.hash(value); + } } class StringArrayExpr extends ConstantExpr @@ -655,6 +726,25 @@ public String stringify() ) ); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + StringArrayExpr that = (StringArrayExpr) o; + return Arrays.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Arrays.hashCode(value); + } } class DoubleExpr extends ConstantExpr @@ -683,6 +773,25 @@ public ExprEval eval(ObjectBinding bindings) { return ExprEval.ofDouble(value); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + DoubleExpr that = (DoubleExpr) o; + return Objects.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Objects.hash(value); + } } class NullDoubleExpr extends NullNumericConstantExpr @@ -729,6 +838,25 @@ public String stringify() } return StringUtils.format("%s", toString()); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + DoubleArrayExpr that = (DoubleArrayExpr) o; + return Arrays.equals(value, that.value); + } + + @Override + public int hashCode() + { + return Arrays.hashCode(value); + } } /** @@ -837,6 +965,25 @@ public Expr visit(Shuttle shuttle) { return shuttle.visit(this); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + IdentifierExpr that = (IdentifierExpr) o; + return Objects.equals(binding, that.binding); + } + + @Override + public int hashCode() + { + return Objects.hash(binding); + } } class LambdaExpr implements Expr @@ -921,6 +1068,26 @@ public BindingDetails analyzeInputs() BindingDetails bodyDetails = expr.analyzeInputs(); return bodyDetails.removeLambdaArguments(lambdaArgs); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LambdaExpr that = (LambdaExpr) o; + return Objects.equals(args, that.args) && + Objects.equals(expr, that.expr); + } + + @Override + public int hashCode() + { + return Objects.hash(args, expr); + } } /** @@ -989,6 +1156,26 @@ public BindingDetails analyzeInputs() .withArrayInputs(function.hasArrayInputs()) .withArrayOutput(function.hasArrayOutput()); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + FunctionExpr that = (FunctionExpr) o; + return args.equals(that.args) && + name.equals(that.name); + } + + @Override + public int hashCode() + { + return Objects.hash(args, name); + } } /** @@ -1080,6 +1267,27 @@ public BindingDetails analyzeInputs() { return bindingDetails; } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ApplyFunctionExpr that = (ApplyFunctionExpr) o; + return name.equals(that.name) && + lambdaExpr.equals(that.lambdaExpr) && + argsExpr.equals(that.argsExpr); + } + + @Override + public int hashCode() + { + return Objects.hash(name, lambdaExpr, argsExpr); + } } /** @@ -1120,6 +1328,25 @@ public BindingDetails analyzeInputs() // currently all unary operators only operate on scalar inputs return expr.analyzeInputs().withScalarArguments(ImmutableSet.of(expr)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + UnaryExpr unaryExpr = (UnaryExpr) o; + return Objects.equals(expr, unaryExpr.expr); + } + + @Override + public int hashCode() + { + return Objects.hash(expr); + } } class UnaryMinusExpr extends UnaryExpr @@ -1262,6 +1489,27 @@ public BindingDetails analyzeInputs() // currently all binary operators operate on scalar inputs return left.analyzeInputs().with(right).withScalarArguments(ImmutableSet.of(left, right)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + BinaryOpExprBase that = (BinaryOpExprBase) o; + return Objects.equals(op, that.op) && + Objects.equals(left, that.left) && + Objects.equals(right, that.right); + } + + @Override + public int hashCode() + { + return Objects.hash(op, left, right); + } } /** @@ -1711,5 +1959,6 @@ public ExprEval eval(ObjectBinding bindings) ExprEval leftVal = left.eval(bindings); return leftVal.asBoolean() ? leftVal : right.eval(bindings); } + } diff --git a/core/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java b/core/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java index a6fa4846cb12..f7cf1d0f6489 100644 --- a/core/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java +++ b/core/src/main/java/org/apache/druid/math/expr/ExprMacroTable.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -129,6 +130,26 @@ public String stringify() return StringUtils.format("%s(%s)", name, arg.stringify()); } + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + BaseScalarUnivariateMacroFunctionExpr that = (BaseScalarUnivariateMacroFunctionExpr) o; + return Objects.equals(name, that.name) && + Objects.equals(arg, that.arg); + } + + @Override + public int hashCode() + { + return Objects.hash(name, arg); + } + private BindingDetails supplyAnalyzeInputs() { return arg.analyzeInputs().withScalarArguments(ImmutableSet.of(arg)); @@ -178,6 +199,26 @@ public BindingDetails analyzeInputs() return analyzeInputsSupplier.get(); } + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + BaseScalarMacroFunctionExpr that = (BaseScalarMacroFunctionExpr) o; + return Objects.equals(name, that.name) && + Objects.equals(args, that.args); + } + + @Override + public int hashCode() + { + return Objects.hash(name, args); + } + private BindingDetails supplyAnalyzeInputs() { final Set argSet = Sets.newHashSetWithExpectedSize(args.size()); diff --git a/core/src/test/java/org/apache/druid/math/expr/ExprTest.java b/core/src/test/java/org/apache/druid/math/expr/ExprTest.java new file mode 100644 index 000000000000..b1eb4705a8bf --- /dev/null +++ b/core/src/test/java/org/apache/druid/math/expr/ExprTest.java @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.math.expr; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class ExprTest +{ + @Test + public void testEqualsContractForBinOrExpr() + { + EqualsVerifier.forClass(BinOrExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinGtExpr() + { + EqualsVerifier.forClass(BinGtExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinMinusExpr() + { + EqualsVerifier.forClass(BinMinusExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinPowExpr() + { + EqualsVerifier.forClass(BinPowExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinMulExpr() + { + EqualsVerifier.forClass(BinMulExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinDivExpr() + { + EqualsVerifier.forClass(BinDivExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinModuloExpr() + { + EqualsVerifier.forClass(BinModuloExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinPlusExpr() + { + EqualsVerifier.forClass(BinPlusExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinLtExpr() + { + EqualsVerifier.forClass(BinLtExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinGeqExpr() + { + EqualsVerifier.forClass(BinGeqExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinEqExpr() + { + EqualsVerifier.forClass(BinEqExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinNeqExpr() + { + EqualsVerifier.forClass(BinNeqExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBinAndExpr() + { + EqualsVerifier.forClass(BinAndExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForFunctionExpr() + { + EqualsVerifier.forClass(FunctionExpr.class).usingGetClass().withIgnoredFields("function").verify(); + } + + @Test + public void testEqualsContractForApplyFunctionExpr() + { + EqualsVerifier.forClass(ApplyFunctionExpr.class) + .usingGetClass() + .withIgnoredFields("function", "bindingDetails", "lambdaBindingDetails", "argsBindingDetails") + .verify(); + } + + @Test + public void testEqualsContractForUnaryNotExpr() + { + EqualsVerifier.forClass(UnaryNotExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForUnaryMinusExpr() + { + EqualsVerifier.forClass(UnaryMinusExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForStringExpr() + { + EqualsVerifier.forClass(StringExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForDoubleExpr() + { + EqualsVerifier.forClass(DoubleExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForLongExpr() + { + EqualsVerifier.forClass(LongExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForStringArrayExpr() + { + EqualsVerifier.forClass(StringArrayExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForLongArrayExpr() + { + EqualsVerifier.forClass(LongArrayExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForDoubleArrayExpr() + { + EqualsVerifier.forClass(DoubleArrayExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForIdentifierExpr() + { + EqualsVerifier.forClass(IdentifierExpr.class).usingGetClass().withIgnoredFields("identifier").verify(); + } + + @Test + public void testEqualsContractForLambdaExpr() + { + EqualsVerifier.forClass(LambdaExpr.class).usingGetClass().verify(); + } + @Test + public void testEqualsContractForNullLongExpr() + { + EqualsVerifier.forClass(NullLongExpr.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForNullDoubleExpr() + { + EqualsVerifier.forClass(NullDoubleExpr.class).usingGetClass().verify(); + } +} diff --git a/processing/pom.xml b/processing/pom.xml index 883104152011..d3b244f3c2b1 100644 --- a/processing/pom.xml +++ b/processing/pom.xml @@ -204,6 +204,12 @@ equalsverifier test + + org.reflections + reflections + 0.9.12 + test + pl.pragmatists JUnitParams diff --git a/processing/src/main/java/org/apache/druid/collections/spatial/search/Bound.java b/processing/src/main/java/org/apache/druid/collections/spatial/search/Bound.java index 1b8a97e9713c..4ba0a3863478 100644 --- a/processing/src/main/java/org/apache/druid/collections/spatial/search/Bound.java +++ b/processing/src/main/java/org/apache/druid/collections/spatial/search/Bound.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import org.apache.druid.collections.spatial.ImmutableNode; import org.apache.druid.collections.spatial.ImmutablePoint; @@ -32,6 +33,7 @@ @JsonSubTypes.Type(name = "radius", value = RadiusBound.class), @JsonSubTypes.Type(name = "polygon", value = PolygonBound.class) }) +@SubclassesMustOverrideEqualsAndHashCode public interface Bound { int getLimit(); diff --git a/processing/src/main/java/org/apache/druid/collections/spatial/search/PolygonBound.java b/processing/src/main/java/org/apache/druid/collections/spatial/search/PolygonBound.java index 2b77bc23a294..d3b0b47f6e49 100644 --- a/processing/src/main/java/org/apache/druid/collections/spatial/search/PolygonBound.java +++ b/processing/src/main/java/org/apache/druid/collections/spatial/search/PolygonBound.java @@ -27,6 +27,7 @@ import org.apache.druid.collections.spatial.ImmutablePoint; import java.nio.ByteBuffer; +import java.util.Arrays; /** */ @@ -195,4 +196,30 @@ public byte[] getCacheKey() return cacheKey.array(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + PolygonBound that = (PolygonBound) o; + return Arrays.equals(abscissa, that.abscissa) && + Arrays.equals(ordinate, that.ordinate); + } + + @Override + public int hashCode() + { + int result = super.hashCode(); + result = 31 * result + Arrays.hashCode(abscissa); + result = 31 * result + Arrays.hashCode(ordinate); + return result; + } } diff --git a/processing/src/main/java/org/apache/druid/collections/spatial/search/RectangularBound.java b/processing/src/main/java/org/apache/druid/collections/spatial/search/RectangularBound.java index 8f5dd04bae80..79f2f688d608 100644 --- a/processing/src/main/java/org/apache/druid/collections/spatial/search/RectangularBound.java +++ b/processing/src/main/java/org/apache/druid/collections/spatial/search/RectangularBound.java @@ -28,6 +28,8 @@ import org.apache.druid.collections.spatial.ImmutablePoint; import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Objects; /** */ @@ -151,4 +153,29 @@ public byte[] getCacheKey() .put(CACHE_TYPE_ID); return cacheKey.array(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + RectangularBound that = (RectangularBound) o; + return limit == that.limit && + numDims == that.numDims && + Arrays.equals(minCoords, that.minCoords) && + Arrays.equals(maxCoords, that.maxCoords); + } + + @Override + public int hashCode() + { + int result = Objects.hash(limit, numDims); + result = 31 * result + Arrays.hashCode(minCoords); + result = 31 * result + Arrays.hashCode(maxCoords); + return result; + } } diff --git a/processing/src/main/java/org/apache/druid/query/dimension/DimensionSpec.java b/processing/src/main/java/org/apache/druid/query/dimension/DimensionSpec.java index 31ba365ad20d..738c65df9143 100644 --- a/processing/src/main/java/org/apache/druid/query/dimension/DimensionSpec.java +++ b/processing/src/main/java/org/apache/druid/query/dimension/DimensionSpec.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import org.apache.druid.java.util.common.Cacheable; import org.apache.druid.java.util.common.UOE; import org.apache.druid.query.extraction.ExtractionFn; @@ -43,6 +44,7 @@ @JsonSubTypes.Type(name = "listFiltered", value = ListFilteredDimensionSpec.class), @JsonSubTypes.Type(name = "prefixFiltered", value = PrefixFilteredDimensionSpec.class) }) +@SubclassesMustOverrideEqualsAndHashCode public interface DimensionSpec extends Cacheable { String getDimension(); diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java index d50c5005fbfc..8d6a628d97a3 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java @@ -19,6 +19,7 @@ package org.apache.druid.query.expression; +import com.google.common.annotations.VisibleForTesting; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.granularity.Granularity; @@ -30,6 +31,7 @@ import javax.annotation.Nonnull; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; public class TimestampCeilExprMacro implements ExprMacroTable.ExprMacro @@ -56,7 +58,8 @@ public Expr apply(final List args) } } - private static class TimestampCeilExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr + @VisibleForTesting + static class TimestampCeilExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr { private final Granularity granularity; @@ -89,6 +92,28 @@ public Expr visit(Shuttle shuttle) List newArgs = args.stream().map(x -> x.visit(shuttle)).collect(Collectors.toList()); return shuttle.visit(new TimestampCeilExpr(newArgs)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + TimestampCeilExpr that = (TimestampCeilExpr) o; + return Objects.equals(granularity, that.granularity); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), granularity); + } } private static PeriodGranularity getGranularity(final List args, final Expr.ObjectBinding bindings) @@ -101,7 +126,8 @@ private static PeriodGranularity getGranularity(final List args, final Exp ); } - private static class TimestampCeilDynamicExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr + @VisibleForTesting + static class TimestampCeilDynamicExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr { TimestampCeilDynamicExpr(final List args) { diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java index 85603ce019a2..aef159ae7cea 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampFloorExprMacro.java @@ -28,6 +28,7 @@ import javax.annotation.Nonnull; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; public class TimestampFloorExprMacro implements ExprMacroTable.ExprMacro @@ -109,6 +110,28 @@ public Expr visit(Shuttle shuttle) return shuttle.visit(new TimestampFloorExpr(newArgs)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + TimestampFloorExpr that = (TimestampFloorExpr) o; + return Objects.equals(granularity, that.granularity); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), granularity); + } } public static class TimestampFloorDynamicExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr diff --git a/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java index 90cde1c91d77..c7ce44f8fe91 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/TrimExprMacro.java @@ -19,6 +19,7 @@ package org.apache.druid.query.expression; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; @@ -27,7 +28,9 @@ import org.apache.druid.math.expr.ExprMacroTable; import javax.annotation.Nonnull; +import java.util.Arrays; import java.util.List; +import java.util.Objects; public abstract class TrimExprMacro implements ExprMacroTable.ExprMacro { @@ -101,7 +104,8 @@ public Expr apply(final List args) } } - private static class TrimStaticCharsExpr extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr + @VisibleForTesting + static class TrimStaticCharsExpr extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr { private final TrimMode mode; private final char[] chars; @@ -172,9 +176,36 @@ public String stringify() } return super.stringify(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + TrimStaticCharsExpr that = (TrimStaticCharsExpr) o; + return mode == that.mode && + Arrays.equals(chars, that.chars) && + Objects.equals(charsExpr, that.charsExpr); + } + + @Override + public int hashCode() + { + int result = Objects.hash(super.hashCode(), mode, charsExpr); + result = 31 * result + Arrays.hashCode(chars); + return result; + } } - private static class TrimDynamicCharsExpr implements Expr + @VisibleForTesting + static class TrimDynamicCharsExpr implements Expr { private final TrimMode mode; private final Expr stringExpr; @@ -265,6 +296,27 @@ public BindingDetails analyzeInputs() .with(charsExpr) .withScalarArguments(ImmutableSet.of(stringExpr, charsExpr)); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + TrimDynamicCharsExpr that = (TrimDynamicCharsExpr) o; + return mode == that.mode && + Objects.equals(stringExpr, that.stringExpr) && + Objects.equals(charsExpr, that.charsExpr); + } + + @Override + public int hashCode() + { + return Objects.hash(mode, stringExpr, charsExpr); + } } private static boolean arrayContains(char[] array, char c) diff --git a/processing/src/main/java/org/apache/druid/query/extraction/CascadeExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/CascadeExtractionFn.java index e3c618f254c4..a005afcb127a 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/CascadeExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/CascadeExtractionFn.java @@ -21,20 +21,22 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.primitives.Bytes; import javax.annotation.Nullable; import java.util.Arrays; +import java.util.Objects; public class CascadeExtractionFn implements ExtractionFn { - private final ExtractionFn[] extractionFns; - private final ChainedExtractionFn chainedExtractionFn; - private final ChainedExtractionFn DEFAULT_CHAINED_EXTRACTION_FN = new ChainedExtractionFn( + @VisibleForTesting + static final ChainedExtractionFn DEFAULT_CHAINED_EXTRACTION_FN = new ChainedExtractionFn( new ExtractionFn() { + private static final String NAME = "nullExtractionFn{}"; @Override public byte[] getCacheKey() { @@ -76,12 +78,28 @@ public ExtractionType getExtractionType() @Override public String toString() { - return "nullExtractionFn{}"; + return NAME; + } + + @Override + public int hashCode() + { + return NAME.hashCode(); + } + + @Override + public boolean equals(Object obj) + { + return obj != null + && getClass().equals(obj.getClass()); } }, null ); + private final ExtractionFn[] extractionFns; + private final ChainedExtractionFn chainedExtractionFn; + @JsonCreator public CascadeExtractionFn( @JsonProperty("extractionFns") ExtractionFn[] extractionFn @@ -172,7 +190,9 @@ public boolean equals(Object o) @Override public int hashCode() { - return chainedExtractionFn.hashCode(); + int result = Objects.hash(chainedExtractionFn); + result = 31 * result + Arrays.hashCode(extractionFns); + return result; } @Override @@ -181,7 +201,8 @@ public String toString() return "CascadeExtractionFn{extractionFns=[" + chainedExtractionFn + "]}"; } - private static class ChainedExtractionFn + @VisibleForTesting + static class ChainedExtractionFn { private final ExtractionFn fn; private final ChainedExtractionFn child; @@ -240,27 +261,15 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - ChainedExtractionFn that = (ChainedExtractionFn) o; - - if (!fn.equals(that.fn)) { - return false; - } - if (child != null && !child.equals(that.child)) { - return false; - } - - return true; + return Objects.equals(fn, that.fn) && + Objects.equals(child, that.child); } @Override public int hashCode() { - int result = fn.hashCode(); - if (child != null) { - result = 31 * result + child.hashCode(); - } - return result; + return Objects.hash(fn, child); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/extraction/ExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/ExtractionFn.java index 5122460864c8..3312945ddeb7 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/ExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/ExtractionFn.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import org.apache.druid.guice.annotations.ExtensionPoint; import org.apache.druid.java.util.common.Cacheable; import org.apache.druid.query.lookup.LookupExtractionFn; @@ -54,6 +55,7 @@ @JsonSubTypes.Type(name = "bucket", value = BucketExtractionFn.class), @JsonSubTypes.Type(name = "strlen", value = StrlenExtractionFn.class) }) +@SubclassesMustOverrideEqualsAndHashCode public interface ExtractionFn extends Cacheable { /** diff --git a/processing/src/main/java/org/apache/druid/query/extraction/LowerExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/LowerExtractionFn.java index 9ccf9c3209d8..87789091ed71 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/LowerExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/LowerExtractionFn.java @@ -27,6 +27,7 @@ import javax.annotation.Nullable; import java.nio.ByteBuffer; import java.util.Locale; +import java.util.Objects; @JsonTypeName("lower") public class LowerExtractionFn extends DimExtractionFn @@ -80,4 +81,24 @@ public byte[] getCacheKey() .put(localeBytes) .array(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LowerExtractionFn that = (LowerExtractionFn) o; + return Objects.equals(locale, that.locale) && + Objects.equals(localeString, that.localeString); + } + + @Override + public int hashCode() + { + return Objects.hash(locale, localeString); + } } diff --git a/processing/src/main/java/org/apache/druid/query/extraction/StrlenExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/StrlenExtractionFn.java index fb665433bc3f..9f98f92355d5 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/StrlenExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/StrlenExtractionFn.java @@ -65,4 +65,16 @@ public byte[] getCacheKey() { return new byte[]{ExtractionCacheHelper.CACHE_TYPE_ID_STRLEN}; } + + @Override + public final int hashCode() + { + return StrlenExtractionFn.class.hashCode(); + } + + @Override + public final boolean equals(Object obj) + { + return obj instanceof StrlenExtractionFn; + } } diff --git a/processing/src/main/java/org/apache/druid/query/extraction/UpperExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/UpperExtractionFn.java index c92c876abf06..40f22b1756e8 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/UpperExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/UpperExtractionFn.java @@ -27,6 +27,7 @@ import javax.annotation.Nullable; import java.nio.ByteBuffer; import java.util.Locale; +import java.util.Objects; @JsonTypeName("upper") public class UpperExtractionFn extends DimExtractionFn @@ -79,4 +80,24 @@ public byte[] getCacheKey() .put(localeBytes) .array(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + UpperExtractionFn that = (UpperExtractionFn) o; + return Objects.equals(locale, that.locale) && + Objects.equals(localeString, that.localeString); + } + + @Override + public int hashCode() + { + return Objects.hash(locale, localeString); + } } diff --git a/processing/src/main/java/org/apache/druid/query/filter/DruidPredicateFactory.java b/processing/src/main/java/org/apache/druid/query/filter/DruidPredicateFactory.java index eade09a79fa9..62d32f90610d 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/DruidPredicateFactory.java +++ b/processing/src/main/java/org/apache/druid/query/filter/DruidPredicateFactory.java @@ -20,7 +20,9 @@ package org.apache.druid.query.filter; import com.google.common.base.Predicate; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; +@SubclassesMustOverrideEqualsAndHashCode public interface DruidPredicateFactory { Predicate makeStringPredicate(); diff --git a/processing/src/main/java/org/apache/druid/query/filter/Filter.java b/processing/src/main/java/org/apache/druid/query/filter/Filter.java index 27a504709dd2..19338e73c1b5 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/Filter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/Filter.java @@ -19,6 +19,7 @@ package org.apache.druid.query.filter; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.java.util.common.UOE; import org.apache.druid.query.BitmapResultFactory; @@ -30,6 +31,7 @@ import java.util.Set; +@SubclassesMustOverrideEqualsAndHashCode public interface Filter { /** diff --git a/processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java b/processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java index 6f9e1254eab5..10d39f9f3b53 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java +++ b/processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import javax.annotation.Nullable; import java.util.Objects; @@ -45,6 +46,7 @@ * As such, it is currently undocumented in user facing documentation on purpose, but whatever this turns into once more * automatic usage of this is in place, should be documented in a future release. */ +@SubclassesMustOverrideEqualsAndHashCode public class FilterTuning { public static FilterTuning createDefault(Filter filter, BitmapIndexSelector selector) diff --git a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java index 29fac03b80de..42058f6cde6b 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java @@ -284,6 +284,11 @@ private static void addPatternCharacter(final StringBuilder patternBuilder, fina } public boolean matches(@Nullable final String s) + { + return matches(s, pattern); + } + + private static boolean matches(@Nullable final String s, Pattern pattern) { String val = NullHandling.nullToEmptyIfNeeded(s); return val != null && pattern.matcher(val).matches(); @@ -310,48 +315,7 @@ public boolean matchesSuffixOnly(final Indexed strings, final int i) public DruidPredicateFactory predicateFactory(final ExtractionFn extractionFn) { - return new DruidPredicateFactory() - { - @Override - public Predicate makeStringPredicate() - { - if (extractionFn != null) { - return input -> matches(extractionFn.apply(input)); - } else { - return input -> matches(input); - } - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - if (extractionFn != null) { - return input -> matches(extractionFn.apply(input)); - } else { - return input -> matches(String.valueOf(input)); - } - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - if (extractionFn != null) { - return input -> matches(extractionFn.apply(input)); - } else { - return input -> matches(String.valueOf(input)); - } - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - if (extractionFn != null) { - return input -> matches(extractionFn.apply(input)); - } else { - return input -> matches(String.valueOf(input)); - } - } - }; + return new PatternDruidPredicateFactory(extractionFn, pattern); } public String getPrefix() @@ -363,5 +327,78 @@ public SuffixMatch getSuffixMatch() { return suffixMatch; } + + @VisibleForTesting + static class PatternDruidPredicateFactory implements DruidPredicateFactory + { + private final ExtractionFn extractionFn; + private final Pattern pattern; + + PatternDruidPredicateFactory(ExtractionFn extractionFn, Pattern pattern) + { + this.extractionFn = extractionFn; + this.pattern = pattern; + } + + @Override + public Predicate makeStringPredicate() + { + if (extractionFn != null) { + return input -> matches(extractionFn.apply(input), pattern); + } else { + return input -> matches(input, pattern); + } + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + if (extractionFn != null) { + return input -> matches(extractionFn.apply(input), pattern); + } else { + return input -> matches(String.valueOf(input), pattern); + } + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + if (extractionFn != null) { + return input -> matches(extractionFn.apply(input), pattern); + } else { + return input -> matches(String.valueOf(input), pattern); + } + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + if (extractionFn != null) { + return input -> matches(extractionFn.apply(input), pattern); + } else { + return input -> matches(String.valueOf(input), pattern); + } + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + PatternDruidPredicateFactory that = (PatternDruidPredicateFactory) o; + return Objects.equals(extractionFn, that.extractionFn) && + Objects.equals(pattern, that.pattern); + } + + @Override + public int hashCode() + { + return Objects.hash(extractionFn, pattern); + } + } } } diff --git a/processing/src/main/java/org/apache/druid/query/filter/SelectorPredicateFactory.java b/processing/src/main/java/org/apache/druid/query/filter/SelectorPredicateFactory.java index b4d12fdf7022..803a1906f524 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/SelectorPredicateFactory.java +++ b/processing/src/main/java/org/apache/druid/query/filter/SelectorPredicateFactory.java @@ -24,6 +24,7 @@ import org.apache.druid.segment.DimensionHandlerUtils; import javax.annotation.Nullable; +import java.util.Objects; /** * A {@link DruidPredicateFactory} that checks if input values equal a specific, provided value. Initialization work @@ -147,4 +148,23 @@ private void initDoublePredicate() } } } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + SelectorPredicateFactory that = (SelectorPredicateFactory) o; + return Objects.equals(initLock, that.initLock); + } + + @Override + public int hashCode() + { + return Objects.hash(initLock); + } } diff --git a/processing/src/main/java/org/apache/druid/query/search/SearchQuerySpec.java b/processing/src/main/java/org/apache/druid/query/search/SearchQuerySpec.java index c0daf775eb5a..ae2be73c2717 100644 --- a/processing/src/main/java/org/apache/druid/query/search/SearchQuerySpec.java +++ b/processing/src/main/java/org/apache/druid/query/search/SearchQuerySpec.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; import javax.annotation.Nullable; @@ -34,6 +35,7 @@ @JsonSubTypes.Type(name = "regex", value = RegexSearchQuerySpec.class), @JsonSubTypes.Type(name = "all", value = AllSearchQuerySpec.class) }) +@SubclassesMustOverrideEqualsAndHashCode public interface SearchQuerySpec { boolean accept(@Nullable String dimVal); diff --git a/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java index 664740ac334b..2c8d3b98c51f 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.filter; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.base.Supplier; import it.unimi.dsi.fastutil.ints.IntList; @@ -46,29 +47,19 @@ import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; -import java.util.Comparator; import java.util.Objects; import java.util.Set; public class BoundFilter implements Filter { private final BoundDimFilter boundDimFilter; - private final Comparator comparator; private final ExtractionFn extractionFn; private final FilterTuning filterTuning; - private final Supplier longPredicateSupplier; - private final Supplier floatPredicateSupplier; - private final Supplier doublePredicateSupplier; - public BoundFilter(final BoundDimFilter boundDimFilter) { this.boundDimFilter = boundDimFilter; - this.comparator = boundDimFilter.getOrdering(); this.extractionFn = boundDimFilter.getExtractionFn(); - this.longPredicateSupplier = boundDimFilter.getLongPredicateSupplier(); - this.floatPredicateSupplier = boundDimFilter.getFloatPredicateSupplier(); - this.doublePredicateSupplier = boundDimFilter.getDoublePredicateSupplier(); this.filterTuning = boundDimFilter.getFilterTuning(); } @@ -79,7 +70,7 @@ public T getBitmapResult(BitmapIndexSelector selector, BitmapResultFactory makeStringPredicate() - { - if (extractionFn != null) { - return input -> doesMatch(extractionFn.apply(input)); - } - return input -> doesMatch(input); - - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - if (extractionFn != null) { - return input -> doesMatch(extractionFn.apply(input)); - } - if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { - return longPredicateSupplier.get(); - } - return input -> doesMatch(String.valueOf(input)); - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - if (extractionFn != null) { - return input -> doesMatch(extractionFn.apply(input)); - } - if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { - return floatPredicateSupplier.get(); - } - return input -> doesMatch(String.valueOf(input)); - } + return new BoundDimFilterDruidPredicateFactory(extractionFn, boundDimFilter); + } - @Override - public DruidDoublePredicate makeDoublePredicate() - { - if (extractionFn != null) { - return input -> doesMatch(extractionFn.apply(input)); - } - if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { - return doublePredicateSupplier.get(); - } - return input -> doesMatch(String.valueOf(input)); - } - }; + private boolean doesMatchNull() + { + return doesMatch(null, boundDimFilter); } - private boolean doesMatch(String input) + private static boolean doesMatch(String input, BoundDimFilter boundDimFilter) { if (input == null) { return (!boundDimFilter.hasLowerBound() @@ -293,10 +242,10 @@ private boolean doesMatch(String input) int lowerComparing = 1; int upperComparing = 1; if (boundDimFilter.hasLowerBound()) { - lowerComparing = comparator.compare(input, boundDimFilter.getLower()); + lowerComparing = boundDimFilter.getOrdering().compare(input, boundDimFilter.getLower()); } if (boundDimFilter.hasUpperBound()) { - upperComparing = comparator.compare(boundDimFilter.getUpper(), input); + upperComparing = boundDimFilter.getOrdering().compare(boundDimFilter.getUpper(), input); } if (boundDimFilter.isLowerStrict() && boundDimFilter.isUpperStrict()) { return ((lowerComparing > 0)) && (upperComparing > 0); @@ -319,7 +268,6 @@ public boolean equals(Object o) } BoundFilter that = (BoundFilter) o; return Objects.equals(boundDimFilter, that.boundDimFilter) && - Objects.equals(comparator, that.comparator) && Objects.equals(extractionFn, that.extractionFn) && Objects.equals(filterTuning, that.filterTuning); } @@ -327,6 +275,91 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(boundDimFilter, comparator, extractionFn, filterTuning); + return Objects.hash(boundDimFilter, extractionFn, filterTuning); + } + + @VisibleForTesting + static class BoundDimFilterDruidPredicateFactory implements DruidPredicateFactory + { + private final ExtractionFn extractionFn; + private final BoundDimFilter boundDimFilter; + private final Supplier longPredicateSupplier; + private final Supplier floatPredicateSupplier; + private final Supplier doublePredicateSupplier; + + BoundDimFilterDruidPredicateFactory(ExtractionFn extractionFn, BoundDimFilter boundDimFilter) + { + this.extractionFn = extractionFn; + this.boundDimFilter = boundDimFilter; + this.longPredicateSupplier = boundDimFilter.getLongPredicateSupplier(); + this.floatPredicateSupplier = boundDimFilter.getFloatPredicateSupplier(); + this.doublePredicateSupplier = boundDimFilter.getDoublePredicateSupplier(); + } + + @Override + public Predicate makeStringPredicate() + { + if (extractionFn != null) { + return input -> doesMatch(extractionFn.apply(input), boundDimFilter); + } + return input -> doesMatch(input, boundDimFilter); + + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + if (extractionFn != null) { + return input -> doesMatch(extractionFn.apply(input), boundDimFilter); + } + if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { + return longPredicateSupplier.get(); + } + return input -> doesMatch(String.valueOf(input), boundDimFilter); + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + if (extractionFn != null) { + return input -> doesMatch(extractionFn.apply(input), boundDimFilter); + } + if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { + return floatPredicateSupplier.get(); + } + return input -> doesMatch(String.valueOf(input), boundDimFilter); + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + if (extractionFn != null) { + return input -> doesMatch(extractionFn.apply(input), boundDimFilter); + } + if (boundDimFilter.getOrdering().equals(StringComparators.NUMERIC)) { + return doublePredicateSupplier.get(); + } + return input -> doesMatch(String.valueOf(input), boundDimFilter); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + BoundDimFilterDruidPredicateFactory that = (BoundDimFilterDruidPredicateFactory) o; + return Objects.equals(extractionFn, that.extractionFn) && + Objects.equals(boundDimFilter, that.boundDimFilter); + } + + @Override + public int hashCode() + { + return Objects.hash(extractionFn, boundDimFilter); + } } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java index 07ded8e287f7..e636572e399a 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java @@ -163,6 +163,26 @@ public double estimateSelectivity(BitmapIndexSelector indexSelector) throw new UnsupportedOperationException(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ColumnComparisonFilter that = (ColumnComparisonFilter) o; + return Objects.equals(dimensions, that.dimensions); + } + + @Override + public int hashCode() + { + return Objects.hash(dimensions); + } + private static class ColumnComparisonReaderFactory implements ColumnProcessorFactory> { private static final ColumnComparisonReaderFactory INSTANCE = new ColumnComparisonReaderFactory(); diff --git a/processing/src/main/java/org/apache/druid/segment/filter/DimensionPredicateFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/DimensionPredicateFilter.java index 3bc3918778c6..817f584a9938 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/DimensionPredicateFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/DimensionPredicateFilter.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.filter; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; @@ -40,6 +41,7 @@ import org.apache.druid.segment.DimensionHandlerUtils; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; +import java.util.Objects; import java.util.Set; /** @@ -77,73 +79,7 @@ public DimensionPredicateFilter( if (extractionFn == null) { this.predicateFactory = predicateFactory; } else { - this.predicateFactory = new DruidPredicateFactory() - { - final Predicate baseStringPredicate = predicateFactory.makeStringPredicate(); - - @Override - public Predicate makeStringPredicate() - { - return input -> baseStringPredicate.apply(extractionFn.apply(input)); - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - return new DruidLongPredicate() - { - @Override - public boolean applyLong(long input) - { - return baseStringPredicate.apply(extractionFn.apply(input)); - } - - @Override - public boolean applyNull() - { - return baseStringPredicate.apply(extractionFn.apply(null)); - } - }; - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - return new DruidFloatPredicate() - { - @Override - public boolean applyFloat(float input) - { - return baseStringPredicate.apply(extractionFn.apply(input)); - } - - @Override - public boolean applyNull() - { - return baseStringPredicate.apply(extractionFn.apply(null)); - } - }; - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - return new DruidDoublePredicate() - { - @Override - public boolean applyDouble(double input) - { - return baseStringPredicate.apply(extractionFn.apply(input)); - } - - @Override - public boolean applyNull() - { - return baseStringPredicate.apply(extractionFn.apply(null)); - } - }; - } - }; + this.predicateFactory = new DelegatingStringPredicateFactory(predicateFactory, extractionFn); } } @@ -209,6 +145,29 @@ public double estimateSelectivity(BitmapIndexSelector indexSelector) ); } + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + DimensionPredicateFilter that = (DimensionPredicateFilter) o; + return Objects.equals(dimension, that.dimension) && + Objects.equals(predicateFactory, that.predicateFactory) && + Objects.equals(basePredicateString, that.basePredicateString) && + Objects.equals(extractionFn, that.extractionFn) && + Objects.equals(filterTuning, that.filterTuning); + } + + @Override + public int hashCode() + { + return Objects.hash(dimension, predicateFactory, basePredicateString, extractionFn, filterTuning); + } + @Override public String toString() { @@ -218,4 +177,102 @@ public String toString() return StringUtils.format("%s = %s", dimension, basePredicateString); } } + + @VisibleForTesting + static class DelegatingStringPredicateFactory implements DruidPredicateFactory + { + private final Predicate baseStringPredicate; + private final DruidPredicateFactory predicateFactory; + private final ExtractionFn extractionFn; + + DelegatingStringPredicateFactory(DruidPredicateFactory predicateFactory, ExtractionFn extractionFn) + { + this.predicateFactory = predicateFactory; + this.baseStringPredicate = predicateFactory.makeStringPredicate(); + this.extractionFn = extractionFn; + } + + @Override + public Predicate makeStringPredicate() + { + return input -> baseStringPredicate.apply(extractionFn.apply(input)); + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + return new DruidLongPredicate() + { + @Override + public boolean applyLong(long input) + { + return baseStringPredicate.apply(extractionFn.apply(input)); + } + + @Override + public boolean applyNull() + { + return baseStringPredicate.apply(extractionFn.apply(null)); + } + }; + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + return new DruidFloatPredicate() + { + @Override + public boolean applyFloat(float input) + { + return baseStringPredicate.apply(extractionFn.apply(input)); + } + + @Override + public boolean applyNull() + { + return baseStringPredicate.apply(extractionFn.apply(null)); + } + }; + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + return new DruidDoublePredicate() + { + @Override + public boolean applyDouble(double input) + { + return baseStringPredicate.apply(extractionFn.apply(input)); + } + + @Override + public boolean applyNull() + { + return baseStringPredicate.apply(extractionFn.apply(null)); + } + }; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + DelegatingStringPredicateFactory that = (DelegatingStringPredicateFactory) o; + return Objects.equals(predicateFactory, that.predicateFactory) && + Objects.equals(extractionFn, that.extractionFn); + } + + @Override + public int hashCode() + { + return Objects.hash(predicateFactory, extractionFn); + } + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java index 8283fd728e72..8b79456f2acc 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java @@ -39,6 +39,7 @@ import org.apache.druid.segment.virtual.ExpressionSelectors; import java.util.Arrays; +import java.util.Objects; import java.util.Set; public class ExpressionFilter implements Filter @@ -176,4 +177,25 @@ public Set getRequiredColumns() { return requiredBindings.get(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ExpressionFilter that = (ExpressionFilter) o; + return Objects.equals(expr, that.expr) && + Objects.equals(requiredBindings, that.requiredBindings) && + Objects.equals(filterTuning, that.filterTuning); + } + + @Override + public int hashCode() + { + return Objects.hash(expr, requiredBindings, filterTuning); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/FalseFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/FalseFilter.java index 902dae48ab11..e20ce5f3154d 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/FalseFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/FalseFilter.java @@ -104,4 +104,16 @@ public String toString() { return "false"; } + + @Override + public final int hashCode() + { + return FalseFilter.class.hashCode(); + } + + @Override + public final boolean equals(Object obj) + { + return obj instanceof FalseFilter; + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index d609a5418ebe..f2ff83eb1d57 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.filter; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableSet; @@ -131,27 +132,20 @@ private Iterable getBitmapIterable(final BitmapIndex bitmapInde private IntIterable getBitmapIndexIterable(final BitmapIndex bitmapIndex) { - return new IntIterable() + return () -> new IntIterator() { + final Iterator iterator = values.iterator(); + @Override - public IntIterator iterator() + public boolean hasNext() { - return new IntIterator() - { - Iterator iterator = values.iterator(); - - @Override - public boolean hasNext() - { - return iterator.hasNext(); - } + return iterator.hasNext(); + } - @Override - public int nextInt() - { - return bitmapIndex.getIndex(iterator.next()); - } - }; + @Override + public int nextInt() + { + return bitmapIndex.getIndex(iterator.next()); } }; } @@ -204,47 +198,7 @@ public boolean supportsSelectivityEstimation(ColumnSelector columnSelector, Bitm private DruidPredicateFactory getPredicateFactory() { - return new DruidPredicateFactory() - { - @Override - public Predicate makeStringPredicate() - { - if (extractionFn != null) { - return input -> values.contains(extractionFn.apply(input)); - } else { - return input -> values.contains(input); - } - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - if (extractionFn != null) { - return input -> values.contains(extractionFn.apply(input)); - } else { - return longPredicateSupplier.get(); - } - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - if (extractionFn != null) { - return input -> values.contains(extractionFn.apply(input)); - } else { - return floatPredicateSupplier.get(); - } - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - if (extractionFn != null) { - return input -> values.contains(extractionFn.apply(input)); - } - return input -> doublePredicateSupplier.get().applyDouble(input); - } - }; + return new InFilterDruidPredicateFactory(extractionFn, values, longPredicateSupplier, floatPredicateSupplier, doublePredicateSupplier); } @Override @@ -268,4 +222,88 @@ public int hashCode() { return Objects.hash(dimension, values, extractionFn, filterTuning); } + + @VisibleForTesting + static class InFilterDruidPredicateFactory implements DruidPredicateFactory + { + private final ExtractionFn extractionFn; + private final Set values; + private final Supplier longPredicateSupplier; + private final Supplier floatPredicateSupplier; + private final Supplier doublePredicateSupplier; + + InFilterDruidPredicateFactory( + ExtractionFn extractionFn, + Set values, + Supplier longPredicateSupplier, + Supplier floatPredicateSupplier, + Supplier doublePredicateSupplier + ) + { + this.extractionFn = extractionFn; + this.values = values; + this.longPredicateSupplier = longPredicateSupplier; + this.floatPredicateSupplier = floatPredicateSupplier; + this.doublePredicateSupplier = doublePredicateSupplier; + } + + @Override + public Predicate makeStringPredicate() + { + if (extractionFn != null) { + return input -> values.contains(extractionFn.apply(input)); + } else { + return input -> values.contains(input); + } + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + if (extractionFn != null) { + return input -> values.contains(extractionFn.apply(input)); + } else { + return longPredicateSupplier.get(); + } + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + if (extractionFn != null) { + return input -> values.contains(extractionFn.apply(input)); + } else { + return floatPredicateSupplier.get(); + } + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + if (extractionFn != null) { + return input -> values.contains(extractionFn.apply(input)); + } + return input -> doublePredicateSupplier.get().applyDouble(input); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + InFilterDruidPredicateFactory that = (InFilterDruidPredicateFactory) o; + return Objects.equals(extractionFn, that.extractionFn) && + Objects.equals(values, that.values); + } + + @Override + public int hashCode() + { + return Objects.hash(extractionFn, values); + } + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/JavaScriptFilter.java index 952f269f4724..d0e50cc47414 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/JavaScriptFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/JavaScriptFilter.java @@ -31,6 +31,7 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.mozilla.javascript.Context; +import java.util.Objects; import java.util.Set; public class JavaScriptFilter implements Filter @@ -116,4 +117,25 @@ public Set getRequiredColumns() { return ImmutableSet.of(dimension); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + JavaScriptFilter that = (JavaScriptFilter) o; + return Objects.equals(dimension, that.dimension) && + Objects.equals(predicateFactory, that.predicateFactory) && + Objects.equals(filterTuning, that.filterTuning); + } + + @Override + public int hashCode() + { + return Objects.hash(dimension, predicateFactory, filterTuning); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java index aa7f952fccec..3a881144bc2b 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java @@ -45,6 +45,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.NoSuchElementException; +import java.util.Objects; import java.util.Set; public class LikeFilter implements Filter @@ -253,4 +254,26 @@ public int nextInt() } }; } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LikeFilter that = (LikeFilter) o; + return Objects.equals(dimension, that.dimension) && + Objects.equals(extractionFn, that.extractionFn) && + Objects.equals(likeMatcher, that.likeMatcher) && + Objects.equals(filterTuning, that.filterTuning); + } + + @Override + public int hashCode() + { + return Objects.hash(dimension, extractionFn, likeMatcher, filterTuning); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java index c71841ad08d2..6bbbbfcc75be 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.filter; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import org.apache.druid.query.extraction.ExtractionFn; import org.apache.druid.query.filter.DruidDoublePredicate; @@ -27,12 +28,15 @@ import org.apache.druid.query.filter.DruidPredicateFactory; import org.apache.druid.query.filter.FilterTuning; +import java.util.Objects; import java.util.regex.Pattern; /** */ public class RegexFilter extends DimensionPredicateFilter { + private final Pattern pattern; + public RegexFilter( final String dimension, final Pattern pattern, @@ -42,42 +46,94 @@ public RegexFilter( { super( dimension, - new DruidPredicateFactory() - { - @Override - public Predicate makeStringPredicate() - { - return input -> (input != null) && pattern.matcher(input).find(); - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - return input -> pattern.matcher(String.valueOf(input)).find(); - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - return input -> pattern.matcher(String.valueOf(input)).find(); - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - return input -> pattern.matcher(String.valueOf(input)).find(); - } - - @Override - public String toString() - { - return "RegexFilter{" + - "pattern='" + pattern + '\'' + - '}'; - } - }, + new PatternDruidPredicateFactory(pattern), extractionFn, filterTuning ); + this.pattern = pattern; + } + + @VisibleForTesting + static class PatternDruidPredicateFactory implements DruidPredicateFactory + { + private final Pattern pattern; + + PatternDruidPredicateFactory(Pattern pattern) + { + this.pattern = pattern; + } + + @Override + public Predicate makeStringPredicate() + { + return input -> (input != null) && pattern.matcher(input).find(); + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + return input -> pattern.matcher(String.valueOf(input)).find(); + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + return input -> pattern.matcher(String.valueOf(input)).find(); + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + return input -> pattern.matcher(String.valueOf(input)).find(); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + PatternDruidPredicateFactory that = (PatternDruidPredicateFactory) o; + return Objects.equals(pattern, that.pattern); + } + + @Override + public int hashCode() + { + return Objects.hash(pattern); + } + } + + @Override + public String toString() + { + return "RegexFilter{" + + "pattern='" + pattern + '\'' + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + RegexFilter that = (RegexFilter) o; + return Objects.equals(pattern, that.pattern); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), pattern); } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/SearchQueryFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/SearchQueryFilter.java index 81424fe76ce8..67843e918f05 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/SearchQueryFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/SearchQueryFilter.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import org.apache.druid.query.extraction.ExtractionFn; import org.apache.druid.query.filter.DruidDoublePredicate; @@ -30,6 +31,8 @@ import org.apache.druid.query.filter.FilterTuning; import org.apache.druid.query.search.SearchQuerySpec; +import java.util.Objects; + /** */ public class SearchQueryFilter extends DimensionPredicateFilter @@ -44,34 +47,63 @@ public SearchQueryFilter( { super( dimension, - new DruidPredicateFactory() - { - @Override - public Predicate makeStringPredicate() - { - return input -> query.accept(input); - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - return input -> query.accept(String.valueOf(input)); - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - return input -> query.accept(String.valueOf(input)); - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - return input -> query.accept(String.valueOf(input)); - } - }, + new SearchQueryDruidPredicateFactory(query), extractionFn, filterTuning ); } + + @VisibleForTesting + static class SearchQueryDruidPredicateFactory implements DruidPredicateFactory + { + private final SearchQuerySpec query; + + SearchQueryDruidPredicateFactory(SearchQuerySpec query) + { + this.query = query; + } + + @Override + public Predicate makeStringPredicate() + { + return input -> query.accept(input); + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + return input -> query.accept(String.valueOf(input)); + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + return input -> query.accept(String.valueOf(input)); + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + return input -> query.accept(String.valueOf(input)); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + SearchQueryDruidPredicateFactory that = (SearchQueryDruidPredicateFactory) o; + return Objects.equals(query, that.query); + } + + @Override + public int hashCode() + { + return Objects.hash(query); + } + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/SpatialFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/SpatialFilter.java index e07c56d20d66..749f4e803694 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/SpatialFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/SpatialFilter.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.filter; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; @@ -37,6 +38,7 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.incremental.SpatialDimensionRowTransformer; +import java.util.Objects; import java.util.Set; /** @@ -71,41 +73,8 @@ public ValueMatcher makeMatcher(ColumnSelectorFactory factory) return Filters.makeValueMatcher( factory, dimension, - new DruidPredicateFactory() - { - @Override - public Predicate makeStringPredicate() - { - return input -> { - if (input == null) { - return false; - } - final float[] coordinate = SpatialDimensionRowTransformer.decode(input); - return bound.contains(coordinate); - }; - } - - @Override - public DruidLongPredicate makeLongPredicate() - { - // SpatialFilter does not currently support longs - return DruidLongPredicate.ALWAYS_FALSE; - } - - @Override - public DruidFloatPredicate makeFloatPredicate() - { - // SpatialFilter does not currently support floats - return DruidFloatPredicate.ALWAYS_FALSE; - } - - @Override - public DruidDoublePredicate makeDoublePredicate() - { - // SpatialFilter does not currently support doubles - return DruidDoublePredicate.ALWAYS_FALSE; - } - } + new BoundDruidPredicateFactory(bound) + ); } @@ -139,4 +108,88 @@ public double estimateSelectivity(BitmapIndexSelector indexSelector) // selectivity estimation for multi-value columns is not implemented yet. throw new UnsupportedOperationException(); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + SpatialFilter that = (SpatialFilter) o; + return Objects.equals(dimension, that.dimension) && + Objects.equals(bound, that.bound) && + Objects.equals(filterTuning, that.filterTuning); + } + + @Override + public int hashCode() + { + return Objects.hash(dimension, bound, filterTuning); + } + + @VisibleForTesting + static class BoundDruidPredicateFactory implements DruidPredicateFactory + { + private final Bound bound; + + BoundDruidPredicateFactory(Bound bound) + { + this.bound = bound; + } + + @Override + public Predicate makeStringPredicate() + { + return input -> { + if (input == null) { + return false; + } + final float[] coordinate = SpatialDimensionRowTransformer.decode(input); + return bound.contains(coordinate); + }; + } + + @Override + public DruidLongPredicate makeLongPredicate() + { + // SpatialFilter does not currently support longs + return DruidLongPredicate.ALWAYS_FALSE; + } + + @Override + public DruidFloatPredicate makeFloatPredicate() + { + // SpatialFilter does not currently support floats + return DruidFloatPredicate.ALWAYS_FALSE; + } + + @Override + public DruidDoublePredicate makeDoublePredicate() + { + // SpatialFilter does not currently support doubles + return DruidDoublePredicate.ALWAYS_FALSE; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + BoundDruidPredicateFactory that = (BoundDruidPredicateFactory) o; + return Objects.equals(bound, that.bound); + } + + @Override + public int hashCode() + { + return Objects.hash(bound); + } + } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/TrueFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/TrueFilter.java index bb35b4f5a314..dae980908f48 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/TrueFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/TrueFilter.java @@ -106,4 +106,16 @@ public String toString() { return "true"; } + + @Override + public final int hashCode() + { + return TrueFilter.class.hashCode(); + } + + @Override + public final boolean equals(Object obj) + { + return obj instanceof TrueFilter; + } } diff --git a/processing/src/test/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCodeTest.java b/processing/src/test/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCodeTest.java new file mode 100644 index 000000000000..99db65ed9f32 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/annotations/SubclassesMustOverrideEqualsAndHashCodeTest.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.annotations; + +import org.junit.Assert; +import org.junit.Test; +import org.reflections.Reflections; +import org.reflections.util.ClasspathHelper; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.net.URL; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +public class SubclassesMustOverrideEqualsAndHashCodeTest +{ + @Test + public void testEqualsAndHashCode() throws NoSuchMethodException + { + // Exclude test classes + Set urls = ClasspathHelper.forPackage("org.apache.druid") + .stream() + .filter(url -> !url.toString().contains("/target/test-classes")) + .collect(Collectors.toSet()); + Reflections reflections = new Reflections(urls); + Set> classes = reflections.getTypesAnnotatedWith(SubclassesMustOverrideEqualsAndHashCode.class); + Set failed = new HashSet<>(); + for (Class clazz : classes) { + if (clazz.isInterface() || Modifier.isAbstract(clazz.getModifiers())) { + continue; + } + Method m = clazz.getMethod("hashCode"); + String className = clazz.getName(); + try { + Assert.assertNotSame(className + " does not implment hashCode", Object.class, m.getDeclaringClass()); + } + catch (AssertionError e) { + failed.add(className); + } + } + if (!failed.isEmpty()) { + System.err.println("failed classes [" + failed.size() + "] : "); + failed.forEach(c -> System.err.println("\t" + c)); + Assert.fail(); + } + } +} diff --git a/processing/src/test/java/org/apache/druid/collections/spatial/search/PolygonBoundTest.java b/processing/src/test/java/org/apache/druid/collections/spatial/search/PolygonBoundTest.java index 5172bafa08d0..3a5d89b73f19 100644 --- a/processing/src/test/java/org/apache/druid/collections/spatial/search/PolygonBoundTest.java +++ b/processing/src/test/java/org/apache/druid/collections/spatial/search/PolygonBoundTest.java @@ -19,6 +19,7 @@ package org.apache.druid.collections.spatial.search; +import nl.jqno.equalsverifier.EqualsVerifier; import org.junit.Assert; import org.junit.Test; @@ -77,4 +78,12 @@ public void testContains() Assert.assertTrue(rightTriangle.contains(new float[]{3f, 3f - delta})); Assert.assertFalse(rightTriangle.contains(new float[]{3f, 3f + delta})); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(PolygonBound.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/collections/spatial/search/RectangularBoundTest.java b/processing/src/test/java/org/apache/druid/collections/spatial/search/RectangularBoundTest.java index b7ea2e589d9b..c50b70be6ec6 100644 --- a/processing/src/test/java/org/apache/druid/collections/spatial/search/RectangularBoundTest.java +++ b/processing/src/test/java/org/apache/druid/collections/spatial/search/RectangularBoundTest.java @@ -19,6 +19,7 @@ package org.apache.druid.collections.spatial.search; +import nl.jqno.equalsverifier.EqualsVerifier; import org.junit.Assert; import org.junit.Test; @@ -46,4 +47,12 @@ public void testCacheKey() new RectangularBound(new float[]{1F, 1F}, new float[]{2F, 2F}, 2).getCacheKey() )); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(RectangularBound.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/expression/TimestampCeilExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/TimestampCeilExprMacroTest.java new file mode 100644 index 000000000000..d7a8d52adf00 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/expression/TimestampCeilExprMacroTest.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.query.expression; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class TimestampCeilExprMacroTest +{ + @Test + public void testEqualsContractForTimestampCeilExpr() + { + EqualsVerifier.forClass(TimestampCeilExprMacro.TimestampCeilExpr.class) + .withIgnoredFields("analyzeInputsSupplier") + .usingGetClass() + .verify(); + } + + @Test + public void testEqualsContractForTimestampCeilDynamicExpr() + { + EqualsVerifier.forClass(TimestampCeilExprMacro.TimestampCeilDynamicExpr.class) + .withIgnoredFields("analyzeInputsSupplier") + .usingGetClass() + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/query/expression/TimestampFloorExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/TimestampFloorExprMacroTest.java new file mode 100644 index 000000000000..bea0ee74a935 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/expression/TimestampFloorExprMacroTest.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.query.expression; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class TimestampFloorExprMacroTest +{ + @Test + public void testEqualsContractForTimestampFloorExpr() + { + EqualsVerifier.forClass(TimestampFloorExprMacro.TimestampFloorExpr.class) + .usingGetClass() + .withIgnoredFields("analyzeInputsSupplier") + .verify(); + } + + @Test + public void testEqualsContractForTimestampFloorDynamicExpr() + { + EqualsVerifier.forClass(TimestampFloorExprMacro.TimestampFloorDynamicExpr.class) + .withIgnoredFields("analyzeInputsSupplier") + .usingGetClass() + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/query/expression/TrimExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/TrimExprMacroTest.java new file mode 100644 index 000000000000..28ea9a0f307a --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/expression/TrimExprMacroTest.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.query.expression; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class TrimExprMacroTest +{ + @Test + public void testEqualsContractForTrimStaticCharsExpr() + { + EqualsVerifier.forClass(TrimExprMacro.TrimStaticCharsExpr.class) + .withIgnoredFields("analyzeInputsSupplier") + .usingGetClass() + .verify(); + } + + @Test + public void testEqualsContractForTrimDynamicCharsExpr() + { + EqualsVerifier.forClass(TrimExprMacro.TrimDynamicCharsExpr.class) + .usingGetClass() + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/query/extraction/CascadeExtractionFnTest.java b/processing/src/test/java/org/apache/druid/query/extraction/CascadeExtractionFnTest.java index 15a1d15ac048..fcbf6f0e0f88 100644 --- a/processing/src/test/java/org/apache/druid/query/extraction/CascadeExtractionFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/extraction/CascadeExtractionFnTest.java @@ -23,8 +23,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.js.JavaScriptConfig; +import org.apache.druid.testing.InitializedNullHandlingTest; import org.junit.Assert; import org.junit.Test; @@ -32,7 +34,7 @@ import java.util.LinkedHashSet; import java.util.Set; -public class CascadeExtractionFnTest +public class CascadeExtractionFnTest extends InitializedNullHandlingTest { private static final String[] PATHS = { "/druid/prod/historical", @@ -205,4 +207,38 @@ public void testSerde() throws Exception ) ); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(CascadeExtractionFn.class) + .withPrefabValues( + CascadeExtractionFn.ChainedExtractionFn.class, + CascadeExtractionFn.DEFAULT_CHAINED_EXTRACTION_FN, + new CascadeExtractionFn.ChainedExtractionFn( + StrlenExtractionFn.instance(), + CascadeExtractionFn.DEFAULT_CHAINED_EXTRACTION_FN + ) + ) + .withNonnullFields("chainedExtractionFn") + .usingGetClass() + .verify(); + } + + @Test + public void testEqualsContractForChainedExtractionFn() + { + EqualsVerifier.forClass(CascadeExtractionFn.ChainedExtractionFn.class) + .withPrefabValues( + CascadeExtractionFn.ChainedExtractionFn.class, + CascadeExtractionFn.DEFAULT_CHAINED_EXTRACTION_FN, + new CascadeExtractionFn.ChainedExtractionFn( + StrlenExtractionFn.instance(), + CascadeExtractionFn.DEFAULT_CHAINED_EXTRACTION_FN + ) + ) + .withNonnullFields("fn") + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/extraction/LowerExtractionFnTest.java b/processing/src/test/java/org/apache/druid/query/extraction/LowerExtractionFnTest.java index d33c1ee6622d..31cc33834964 100644 --- a/processing/src/test/java/org/apache/druid/query/extraction/LowerExtractionFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/extraction/LowerExtractionFnTest.java @@ -19,6 +19,7 @@ package org.apache.druid.query.extraction; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.junit.Assert; import org.junit.Test; @@ -45,4 +46,12 @@ public void testGetCacheKey() Assert.assertArrayEquals(extractionFn.getCacheKey(), extractionFn.getCacheKey()); Assert.assertFalse(Arrays.equals(extractionFn.getCacheKey(), new UpperExtractionFn(null).getCacheKey())); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(LowerExtractionFn.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/extraction/StrlenExtractionFnTest.java b/processing/src/test/java/org/apache/druid/query/extraction/StrlenExtractionFnTest.java index baf629d40bb6..1997a0f9bb2d 100644 --- a/processing/src/test/java/org/apache/druid/query/extraction/StrlenExtractionFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/extraction/StrlenExtractionFnTest.java @@ -20,6 +20,7 @@ package org.apache.druid.query.extraction; import com.fasterxml.jackson.databind.ObjectMapper; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.jackson.DefaultObjectMapper; import org.junit.Assert; @@ -63,4 +64,10 @@ public void testSerde() throws Exception Assert.assertTrue(extractionFn == extractionFnRoundTrip); Assert.assertTrue(extractionFn == StrlenExtractionFn.instance()); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(StrlenExtractionFn.class).verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/extraction/UpperExtractionFnTest.java b/processing/src/test/java/org/apache/druid/query/extraction/UpperExtractionFnTest.java index e9d6e2339b5f..12c0bdc41035 100644 --- a/processing/src/test/java/org/apache/druid/query/extraction/UpperExtractionFnTest.java +++ b/processing/src/test/java/org/apache/druid/query/extraction/UpperExtractionFnTest.java @@ -19,6 +19,7 @@ package org.apache.druid.query.extraction; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.junit.Assert; import org.junit.Test; @@ -45,4 +46,12 @@ public void testGetCacheKey() Assert.assertArrayEquals(extractionFn.getCacheKey(), extractionFn.getCacheKey()); Assert.assertFalse(Arrays.equals(extractionFn.getCacheKey(), new LowerExtractionFn(null).getCacheKey())); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(UpperExtractionFn.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/filter/LikeDimFilterTest.java b/processing/src/test/java/org/apache/druid/query/filter/LikeDimFilterTest.java index e918a66f7aa3..a2529615e18e 100644 --- a/processing/src/test/java/org/apache/druid/query/filter/LikeDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/query/filter/LikeDimFilterTest.java @@ -21,15 +21,17 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.Sets; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.query.extraction.SubstringDimExtractionFn; +import org.apache.druid.testing.InitializedNullHandlingTest; import org.junit.Assert; import org.junit.Test; import java.io.IOException; import java.util.Arrays; -public class LikeDimFilterTest +public class LikeDimFilterTest extends InitializedNullHandlingTest { @Test public void testSerde() throws IOException @@ -68,4 +70,12 @@ public void testGetRequiredColumns() final DimFilter filter = new LikeDimFilter("foo", "bar%", "@", new SubstringDimExtractionFn(1, 2)); Assert.assertEquals(filter.getRequiredColumns(), Sets.newHashSet("foo")); } + + @Test + public void testEqualsContractForExtractionFnDruidPredicateFactory() + { + EqualsVerifier.forClass(LikeDimFilter.LikeMatcher.PatternDruidPredicateFactory.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/query/filter/SelectorPredicateFactoryTest.java b/processing/src/test/java/org/apache/druid/query/filter/SelectorPredicateFactoryTest.java new file mode 100644 index 000000000000..03770efffcb9 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/filter/SelectorPredicateFactoryTest.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.query.filter; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class SelectorPredicateFactoryTest +{ + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(SelectorPredicateFactory.class) + // initLock is initialized when the object is constructed and never changed, + // so the lock can be used in equals comparison. + .withOnlyTheseFields("initLock") + .usingGetClass() + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java index cda4eba50f4a..565be2a1a232 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java @@ -728,7 +728,15 @@ public void test_equals() { EqualsVerifier.forClass(BoundFilter.class) .usingGetClass() - .withNonnullFields("boundDimFilter", "comparator") + .withNonnullFields("boundDimFilter") + .verify(); + } + + @Test + public void test_equals_boundDimFilterDruidPredicateFactory() + { + EqualsVerifier.forClass(BoundFilter.BoundDimFilterDruidPredicateFactory.class) + .usingGetClass() .withIgnoredFields("longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") .verify(); } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/ColumnComparisonFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/ColumnComparisonFilterTest.java index 676fa4d4743a..e9f26d68a97a 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/ColumnComparisonFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/ColumnComparisonFilterTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.impl.DimensionsSpec; @@ -192,4 +193,12 @@ public void testSelectorWithLookupExtractionFn() new ExtractionDimensionSpec("dim1", "dim1", lookupFn) )), ImmutableList.of("2", "5", "7", "8")); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(ColumnComparisonFilter.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java new file mode 100644 index 000000000000..e3d6246309f3 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.segment.filter; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class DimensionPredicateFilterTest +{ + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(DimensionPredicateFilter.class) + .usingGetClass() + .verify(); + } + + @Test + public void testEqualsContractForDelegatingStringPredicateFactory() + { + EqualsVerifier.forClass(DimensionPredicateFilter.DelegatingStringPredicateFactory.class) + .withIgnoredFields("baseStringPredicate") + .usingGetClass() + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java index 2687be96f94f..0f2933768b8b 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.impl.DimensionsSpec; @@ -273,6 +274,14 @@ public void testGetRequiredColumn() Assert.assertEquals(edf("missing == ''").getRequiredColumns(), Sets.newHashSet("missing")); } + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(ExpressionFilter.class) + .usingGetClass() + .verify(); + } + private static ExpressionDimFilter edf(final String expression) { return new ExpressionDimFilter(expression, null, TestExprMacroTable.INSTANCE); diff --git a/processing/src/test/java/org/apache/druid/segment/filter/FalseFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/FalseFilterTest.java new file mode 100644 index 000000000000..1b211566d133 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/filter/FalseFilterTest.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.segment.filter; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class FalseFilterTest +{ + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(FalseFilter.class) + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java index 4ca919279beb..e4ab25994a3f 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java @@ -363,6 +363,16 @@ public void test_equals() .verify(); } + @Test + public void test_equals_forInFilterDruidPredicateFactory() + { + EqualsVerifier.forClass(InFilter.InFilterDruidPredicateFactory.class) + .usingGetClass() + .withNonnullFields("values") + .withIgnoredFields("longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") + .verify(); + } + private DimFilter toInFilter(String dim) { List emptyList = new ArrayList<>(); diff --git a/processing/src/test/java/org/apache/druid/segment/filter/JavaScriptFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/JavaScriptFilterTest.java index 510908d4692c..8b3af07856a1 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/JavaScriptFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/JavaScriptFilterTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.Pair; import org.apache.druid.js.JavaScriptConfig; @@ -221,6 +222,13 @@ public void testNumericNull() assertFilterMatchesSkipVectorize(newJavaScriptDimFilter("l0", jsNumericValueFilter("9001"), null), ImmutableList.of("4")); } + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(JavaScriptFilter.class) + .usingGetClass() + .verify(); + } private JavaScriptDimFilter newJavaScriptDimFilter( final String dimension, final String function, diff --git a/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java index 7a4b0d85ed4b..5492efa8ebc2 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java @@ -21,6 +21,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.Pair; import org.apache.druid.js.JavaScriptConfig; @@ -135,4 +136,20 @@ public void testRegexWithExtractionFn() assertFilterMatches(new RegexDimFilter("dim4", ".*ANYMORE", changeNullFn), ImmutableList.of("0", "1", "2", "3", "4", "5")); assertFilterMatches(new RegexDimFilter("dim4", "a.*", changeNullFn), ImmutableList.of()); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(RegexFilter.class) + .usingGetClass() + .verify(); + } + + @Test + public void testEqualsContractForPatternDruidPredicateFactory() + { + EqualsVerifier.forClass(RegexFilter.PatternDruidPredicateFactory.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java index 31ae1c46598e..9948841ff568 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java @@ -21,6 +21,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.Pair; import org.apache.druid.js.JavaScriptConfig; @@ -172,4 +173,18 @@ public void testSearchQueryWithExtractionFn() assertFilterMatches(new SearchQueryDimFilter("dim4", specForValue("ANYMORE"), changeNullFn), ImmutableList.of("0", "1", "2", "3", "4", "5")); assertFilterMatches(new SearchQueryDimFilter("dim4", specForValue("a"), changeNullFn), ImmutableList.of()); } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(SearchQueryFilter.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForSearchQueryDruidPredicateFactory() + { + EqualsVerifier.forClass(SearchQueryFilter.SearchQueryDruidPredicateFactory.class) + .usingGetClass() + .verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/SpatialFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/SpatialFilterTest.java index 3ab78fc986ac..9d13c312d05b 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/SpatialFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/SpatialFilterTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.collections.spatial.search.RadiusBound; import org.apache.druid.collections.spatial.search.RectangularBound; import org.apache.druid.data.input.MapBasedInputRow; @@ -55,6 +56,7 @@ import org.apache.druid.segment.incremental.IncrementalIndex; import org.apache.druid.segment.incremental.IncrementalIndexSchema; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; +import org.apache.druid.testing.InitializedNullHandlingTest; import org.joda.time.Interval; import org.junit.Test; import org.junit.runner.RunWith; @@ -72,7 +74,7 @@ /** */ @RunWith(Parameterized.class) -public class SpatialFilterTest +public class SpatialFilterTest extends InitializedNullHandlingTest { private static IndexMerger INDEX_MERGER = TestHelper.getTestIndexMergerV9(OffHeapMemorySegmentWriteOutMediumFactory.instance()); private static IndexIO INDEX_IO = TestHelper.getTestIndexIO(); @@ -716,4 +718,16 @@ public void testSpatialQueryMorePoints() throw new RuntimeException(e); } } + + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(SpatialFilter.class).usingGetClass().verify(); + } + + @Test + public void testEqualsContractForBoundDruidPredicateFactory() + { + EqualsVerifier.forClass(SpatialFilter.BoundDruidPredicateFactory.class).usingGetClass().verify(); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/TrueFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/TrueFilterTest.java new file mode 100644 index 000000000000..ab98f3e8fe9e --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/filter/TrueFilterTest.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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 org.apache.druid.segment.filter; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.Test; + +public class TrueFilterTest +{ + @Test + public void testEqualsContract() + { + EqualsVerifier.forClass(TrueFilter.class) + .verify(); + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java index 1eda58305486..ec03a9e3d71d 100644 --- a/processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java @@ -713,6 +713,13 @@ public Set getRequiredColumns() return Collections.emptySet(); } + @Override + public int hashCode() + { + // Test code, hashcode and equals isn't important + return super.hashCode(); + } + private class DictionaryRaceTestFilterDruidPredicateFactory implements DruidPredicateFactory { @Override diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index bdb9a0bfcd58..cfa757972efa 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -2169,7 +2169,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo ImmutableList.of() ); } - + @Test public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowISE() { From 5a7697794301cd1c278cd294f4c6e6317f0ac450 Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Mon, 11 May 2020 18:14:22 -0700 Subject: [PATCH 4/7] fix tests --- core/src/main/java/org/apache/druid/math/expr/Expr.java | 4 ++-- core/src/test/java/org/apache/druid/math/expr/ExprTest.java | 2 +- .../druid/segment/join/HashJoinSegmentStorageAdapterTest.java | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/druid/math/expr/Expr.java b/core/src/main/java/org/apache/druid/math/expr/Expr.java index 28544e239c08..d8ddbb49ca9a 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Expr.java +++ b/core/src/main/java/org/apache/druid/math/expr/Expr.java @@ -976,13 +976,13 @@ public boolean equals(Object o) return false; } IdentifierExpr that = (IdentifierExpr) o; - return Objects.equals(binding, that.binding); + return Objects.equals(identifier, that.identifier); } @Override public int hashCode() { - return Objects.hash(binding); + return Objects.hash(identifier); } } diff --git a/core/src/test/java/org/apache/druid/math/expr/ExprTest.java b/core/src/test/java/org/apache/druid/math/expr/ExprTest.java index b1eb4705a8bf..90c3d9a28394 100644 --- a/core/src/test/java/org/apache/druid/math/expr/ExprTest.java +++ b/core/src/test/java/org/apache/druid/math/expr/ExprTest.java @@ -168,7 +168,7 @@ public void testEqualsContractForDoubleArrayExpr() @Test public void testEqualsContractForIdentifierExpr() { - EqualsVerifier.forClass(IdentifierExpr.class).usingGetClass().withIgnoredFields("identifier").verify(); + EqualsVerifier.forClass(IdentifierExpr.class).usingGetClass().withIgnoredFields("binding").verify(); } @Test diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index cfa757972efa..c58291301d06 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -2133,6 +2133,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRows() ); } + @Test public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLookup() { Filter originalFilter = new SelectorFilter("page", "this matches nothing"); From 077710eadad760b334586f93ee1fb929911af78e Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Tue, 12 May 2020 08:47:54 -0700 Subject: [PATCH 5/7] Actually fix the tests --- .../druid/segment/filter/ExpressionFilter.java | 13 +++++++++++-- .../apache/druid/segment/filter/RegexFilter.java | 8 ++++++++ .../filter/DimensionPredicateFilterTest.java | 1 + .../druid/segment/filter/ExpressionFilterTest.java | 1 + .../druid/segment/filter/RegexFilterTest.java | 2 ++ .../druid/segment/filter/SearchQueryFilterTest.java | 5 ++++- 6 files changed, 27 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java index e70d8f22ec16..ef7613c6137c 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java @@ -196,13 +196,22 @@ public boolean equals(Object o) } ExpressionFilter that = (ExpressionFilter) o; return Objects.equals(expr, that.expr) && - Objects.equals(requiredBindings, that.requiredBindings) && Objects.equals(filterTuning, that.filterTuning); } @Override public int hashCode() { - return Objects.hash(expr, requiredBindings, filterTuning); + return Objects.hash(expr, filterTuning); + } + + @Override + public String toString() + { + return "ExpressionFilter{" + + "expr=" + expr + + ", requiredBindings=" + requiredBindings + + ", filterTuning=" + filterTuning + + '}'; } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java index dedc869daebd..2e24ab3ceaef 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java @@ -108,6 +108,14 @@ public int hashCode() { return Objects.hash(pattern); } + + @Override + public String toString() + { + return "RegexFilter$PatternDruidPredicateFactory{" + + "pattern='" + pattern + '\'' + + '}'; + } } @Override diff --git a/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java index e3d6246309f3..07707a5d8872 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/DimensionPredicateFilterTest.java @@ -28,6 +28,7 @@ public class DimensionPredicateFilterTest public void testEqualsContract() { EqualsVerifier.forClass(DimensionPredicateFilter.class) + .withIgnoredFields("predicateFactory") .usingGetClass() .verify(); } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java index 9406774bce28..b44d5541cad6 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/ExpressionFilterTest.java @@ -284,6 +284,7 @@ public void testGetRequiredColumn() public void testEqualsContract() { EqualsVerifier.forClass(ExpressionFilter.class) + .withIgnoredFields("requiredBindings") .usingGetClass() .verify(); } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java index 676bab68e5ab..617212dd4977 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/RegexFilterTest.java @@ -150,6 +150,8 @@ public void testRegexWithExtractionFn() public void testEqualsContract() { EqualsVerifier.forClass(RegexFilter.class) + .withNonnullFields("pattern") + .withIgnoredFields("predicateFactory") .usingGetClass() .verify(); } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java index d70e10a46c0f..a7246642925f 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/SearchQueryFilterTest.java @@ -186,7 +186,10 @@ public void testSearchQueryWithExtractionFn() @Test public void testEqualsContract() { - EqualsVerifier.forClass(SearchQueryFilter.class).usingGetClass().verify(); + EqualsVerifier.forClass(SearchQueryFilter.class) + .withIgnoredFields("predicateFactory") + .usingGetClass() + .verify(); } @Test From 6fa929deb3cb66d46e7b4e8996c326f26878f3ef Mon Sep 17 00:00:00 2001 From: Suneet Saldanha Date: Tue, 12 May 2020 21:43:23 -0700 Subject: [PATCH 6/7] Address review comments --- .idea/inspectionProfiles/Druid.xml | 35 +++++++++++------- .../java/org/apache/druid/math/expr/Expr.java | 36 ++++++++++++------- .../org/apache/druid/math/expr/ExprTest.java | 4 +-- .../dimension/RegexFilteredDimensionSpec.java | 1 - .../druid/query/filter/LikeDimFilter.java | 2 +- .../filter/SelectorPredicateFactory.java | 4 +-- .../druid/segment/filter/RegexFilter.java | 2 +- .../RegexFilteredDimensionSpecTest.java | 3 +- .../druid/query/filter/LikeDimFilterTest.java | 1 + .../filter/SelectorPredicateFactoryTest.java | 4 +-- .../druid/segment/filter/RegexFilterTest.java | 1 + 11 files changed, 57 insertions(+), 36 deletions(-) diff --git a/.idea/inspectionProfiles/Druid.xml b/.idea/inspectionProfiles/Druid.xml index 4025da7f1ef7..c6f2cbc39d2d 100644 --- a/.idea/inspectionProfiles/Druid.xml +++ b/.idea/inspectionProfiles/Druid.xml @@ -69,7 +69,6 @@