This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 0d285f701bebeff31da5c2b408191862d88fb24f Author: Martin Desruisseaux <[email protected]> AuthorDate: Tue Mar 24 17:44:54 2026 +0100 Fix a ClassCastException in the result of optimizing an `equal` filter when the operands are two Boolean values. --- .../org/apache/sis/filter/ComparisonFilter.java | 79 +++++++++++++++-- .../sis/filter/base/BinaryFunctionWidening.java | 3 +- .../apache/sis/filter/ComparisonFilterTest.java | 98 ++++++++++++++-------- .../apache/sis/filter/DynamicOptimizationTest.java | 19 +++++ .../main/org/apache/sis/math/NumberType.java | 2 +- 5 files changed, 153 insertions(+), 48 deletions(-) diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java index e1ea67677f..07a72eba9e 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/ComparisonFilter.java @@ -165,6 +165,14 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj return true; } + /** + * Creates a new filter of the same type and parameters than this filter, except for the expressions. + * The given array shall be of length 2 and shall contain the replacement for {@link #expression1} + * and {@link #expression2}, in that order. + */ + @Override + public abstract ComparisonFilter<R> recreate(Expression<R,?>[] effective); + /** * Tries to optimize this filter. Fist, this method applies the optimization documented * in the {@linkplain Optimization.OnFilter#optimize default method impmementation}. @@ -191,6 +199,11 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj if (temporal.evaluator != null) { return temporal; } + if (Comparable.class.isAssignableFrom(t1) && t1.isAssignableFrom(t2)) { + if (isMatchingCase || !CharSequence.class.isAssignableFrom(t1)) { + return new Comparables(); + } + } } } return result; @@ -208,7 +221,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj /** * An optimized versions of this filter for the case where the operands are numeric. */ - private final class Numeric extends Node implements Filter<R> { + private final class Numeric extends Node implements Optimization.OnFilter<R> { /** For cross-version compatibility during (de)serialization. */ private static final long serialVersionUID = 4969425622445580192L; @@ -228,12 +241,17 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj @Override public boolean test(final R candidate) { return ((Integer) evaluator.apply(candidate)) != 0; } + + /** Creates a new filter of the same type but different parameters. */ + @Override public Filter<R> recreate(Expression<R,?>[] effective) { + return ComparisonFilter.this.recreate(effective).new Numeric(); + } } /** * An optimized versions of this filter for the case where the operands are temporal. */ - private final class Time<T,S> extends Node implements Filter<R> { + private final class Time<T,S> extends Node implements Optimization.OnFilter<R> { /** For cross-version compatibility during (de)serialization. */ private static final long serialVersionUID = -5132906457258846016L; @@ -245,6 +263,11 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj evaluator = (methods != null) ? methods.predicate(temporalTest(), otherType) : null; } + /** Creates a new filter which is reusing an already determined evaluator. */ + Time(final BiPredicate<T,S> evaluator) { + this.evaluator = evaluator; + } + /** Delegates to the enclosing class.*/ @Override public CodeList<?> getOperatorType() {return ComparisonFilter.this.getOperatorType();} @Override public Class<? super R> getResourceClass() {return ComparisonFilter.this.getResourceClass();} @@ -264,6 +287,46 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } return false; } + + /** Creates a new filter of the same type but different parameters. */ + @Override public Filter<R> recreate(Expression<R,?>[] effective) { + return ComparisonFilter.this.recreate(effective).new Time<>(evaluator); + } + } + + /** + * An optimized versions of this filter for the case where the operands are comparable. + */ + private final class Comparables extends Node implements Optimization.OnFilter<R> { + /** For cross-version compatibility during (de)serialization. */ + private static final long serialVersionUID = -8311546273569455269L; + + /** Delegates to the enclosing class.*/ + @Override public CodeList<?> getOperatorType() {return ComparisonFilter.this.getOperatorType();} + @Override public Class<? super R> getResourceClass() {return ComparisonFilter.this.getResourceClass();} + @Override public List<Expression<R,?>> getExpressions() {return ComparisonFilter.this.getExpressions();} + @Override protected Collection<?> getChildren() {return ComparisonFilter.this.getChildren();} + + /** Determines if the test represented by this filter passes with the given operands. */ + @Override public boolean test(final R candidate) { + final Object left = expression1.apply(candidate); + if (left != null) { + final Object right = expression2.apply(candidate); + if (right != null) { + if (left.getClass() == right.getClass()) { + @SuppressWarnings("unchecked") + final int result = ((Comparable) left).compareTo(right); + return fromCompareTo(result); + } + } + } + return false; + } + + /** Creates a new filter of the same type but different parameters. */ + @Override public Filter<R> recreate(Expression<R,?>[] effective) { + return ComparisonFilter.this.recreate(effective).new Comparables(); + } } /** @@ -437,7 +500,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new LessThan<>(effective[0], effective[1], isMatchingCase, matchAction); } @@ -477,7 +540,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new LessThanOrEqualTo<>(effective[0], effective[1], isMatchingCase, matchAction); } @@ -517,7 +580,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new GreaterThan<>(effective[0], effective[1], isMatchingCase, matchAction); } @@ -557,7 +620,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new GreaterThanOrEqualTo<>(effective[0], effective[1], isMatchingCase, matchAction); } @@ -597,7 +660,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new EqualTo<>(effective[0], effective[1], isMatchingCase, matchAction); } @@ -637,7 +700,7 @@ abstract class ComparisonFilter<R> extends BinaryFunctionWidening<R, Object, Obj } /** Creates a new filter of the same type but different parameters. */ - @Override public Filter<R> recreate(final Expression<R,?>[] effective) { + @Override public ComparisonFilter<R> recreate(final Expression<R,?>[] effective) { return new NotEqualTo<>(effective[0], effective[1], isMatchingCase, matchAction); } diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/BinaryFunctionWidening.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/BinaryFunctionWidening.java index 09150331b8..3303df898a 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/BinaryFunctionWidening.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/BinaryFunctionWidening.java @@ -162,7 +162,6 @@ public abstract class BinaryFunctionWidening<R, A1, A2> extends BinaryFunction<R */ protected static NumberType effective(final NumberType type) { switch (type) { - case NULL: // Case of expressions without `FeatureExpression.getResultType()`. case NUMBER: // Case of expressions that declare only the generic `Number` class. case FRACTION: case BIG_INTEGER: @@ -171,7 +170,7 @@ public abstract class BinaryFunctionWidening<R, A1, A2> extends BinaryFunction<R case SHORT: case INTEGER: case LONG: return NumberType.LONG; - default: return NumberType.DOUBLE; // The fallback used for unrecognized types. + default: return type.isReal() ? NumberType.DOUBLE : type; // The fallback used for unrecognized types. } } diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/ComparisonFilterTest.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/ComparisonFilterTest.java index bb54570867..5dd5449581 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/ComparisonFilterTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/ComparisonFilterTest.java @@ -24,6 +24,7 @@ import static org.apache.sis.test.Assertions.assertSerializedEquals; // Specific to the geoapi-3.1 and geoapi-4.0 branches: import org.opengis.feature.Feature; +import org.opengis.filter.Filter; import org.opengis.filter.Literal; import org.opengis.filter.FilterFactory; import org.opengis.filter.ComparisonOperator; @@ -49,16 +50,16 @@ public final class ComparisonFilterTest extends TestCase { /** * Expressions used as constant for the tests. */ - private final Literal<Feature,Integer> c05, c10, c20; + private final Literal<Feature, Integer> c05, c10, c20; /** - * Expected name of the filter to be evaluated. The {@code evaluate(…)} methods + * Expected name of the filter to be evaluated. The {@code assertTestEqual(…)} methods * will compare {@link ComparisonFilter#getFunctionName()} against this value. */ private ComparisonOperatorName expectedName; /** - * The filter tested by last call to {@code evaluate(…)} methods. + * The filter tested by last call to {@code assertTestEqual(…)} methods. */ private ComparisonOperator<Feature> filter; @@ -73,25 +74,44 @@ public final class ComparisonFilterTest extends TestCase { } /** - * Evaluates the given filter. The {@link #expectedName} field must be set before this method is invoked. + * Performs various assertions on the given filter, including serialization. * This method assumes that the first expression of all filters is {@link #c10}. + * + * @param expected the expected result of evaluating the given filter. + * @param filter the filter to test. */ - private boolean evaluate(final BinaryComparisonOperator<Feature> filter) { + private void verify(final boolean expected, final BinaryComparisonOperator<Feature> filter) { + assertTestEquals(expected, filter); + assertSame(c10, filter.getExpressions().get(0)); + assertSerializedEquals(filter); + } + + /** + * Evaluates the given filter and asserts that the returned value is equal to the expected value. + * The {@link #expectedName} field must be set before this method is invoked. + */ + private void assertTestEquals(final boolean expected, final BinaryComparisonOperator<Feature> filter) { this.filter = filter; assertInstanceOf(ComparisonFilter.class, filter); assertEquals(expectedName, filter.getOperatorType()); - assertSame(c10, filter.getExpressions().get(0)); - return filter.test(null); + assertEquals(expected, filter.test(null)); + + // Optimization of an expression with literals should result in a literal. + assertSame(expected ? Filter.include() : Filter.exclude(), new Optimization().apply(filter)); } /** - * Evaluates the given "Property is between" filter. + * Evaluates the given "Property is between" filter and asserts that the returned value + * is equal to the expected value. */ - private boolean evaluate(final BetweenComparisonOperator<Feature> filter) { + private void assertTestEquals(final boolean expected, final BetweenComparisonOperator<Feature> filter) { this.filter = filter; assertInstanceOf(ComparisonFilter.Between.class, filter); assertEquals(expectedName, filter.getOperatorType()); - return filter.test(null); + assertEquals(expected, filter.test(null)); + + // Optimization of an expression with literals should result in a literal. + assertSame(expected ? Filter.include() : Filter.exclude(), new Optimization().apply(filter)); } /** @@ -100,10 +120,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testLess() { expectedName = ComparisonOperatorName.PROPERTY_IS_LESS_THAN; - assertTrue (evaluate(factory.less(c10, c20))); - assertFalse(evaluate(factory.less(c10, c10))); - assertFalse(evaluate(factory.less(c10, c05))); - assertSerializedEquals(filter); + verify(true, factory.less(c10, c20)); + verify(false, factory.less(c10, c10)); + verify(false, factory.less(c10, c05)); } /** @@ -112,10 +131,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testLessOrEqual() { expectedName = ComparisonOperatorName.PROPERTY_IS_LESS_THAN_OR_EQUAL_TO; - assertTrue (evaluate(factory.lessOrEqual(c10, c20))); - assertTrue (evaluate(factory.lessOrEqual(c10, c10))); - assertFalse(evaluate(factory.lessOrEqual(c10, c05))); - assertSerializedEquals(filter); + verify(true, factory.lessOrEqual(c10, c20)); + verify(true, factory.lessOrEqual(c10, c10)); + verify(false, factory.lessOrEqual(c10, c05)); } /** @@ -124,10 +142,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testGreater() { expectedName = ComparisonOperatorName.PROPERTY_IS_GREATER_THAN; - assertFalse(evaluate(factory.greater(c10, c20))); - assertFalse(evaluate(factory.greater(c10, c10))); - assertTrue (evaluate(factory.greater(c10, c05))); - assertSerializedEquals(filter); + verify(false, factory.greater(c10, c20)); + verify(false, factory.greater(c10, c10)); + verify(true, factory.greater(c10, c05)); } /** @@ -136,10 +153,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testGreaterOrEqual() { expectedName = ComparisonOperatorName.PROPERTY_IS_GREATER_THAN_OR_EQUAL_TO; - assertFalse(evaluate(factory.greaterOrEqual(c10, c20))); - assertTrue (evaluate(factory.greaterOrEqual(c10, c10))); - assertTrue (evaluate(factory.greaterOrEqual(c10, c05))); - assertSerializedEquals(filter); + verify(false, factory.greaterOrEqual(c10, c20)); + verify(true, factory.greaterOrEqual(c10, c10)); + verify(true, factory.greaterOrEqual(c10, c05)); } /** @@ -148,10 +164,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testEqual() { expectedName = ComparisonOperatorName.PROPERTY_IS_EQUAL_TO; - assertFalse(evaluate(factory.equal(c10, c20))); - assertTrue (evaluate(factory.equal(c10, c10))); - assertFalse(evaluate(factory.equal(c10, c05))); - assertSerializedEquals(filter); + verify(false, factory.equal(c10, c20)); + verify(true, factory.equal(c10, c10)); + verify(false, factory.equal(c10, c05)); } /** @@ -160,10 +175,9 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testNotEqual() { expectedName = ComparisonOperatorName.PROPERTY_IS_NOT_EQUAL_TO; - assertTrue (evaluate(factory.notEqual(c10, c20))); - assertFalse(evaluate(factory.notEqual(c10, c10))); - assertTrue (evaluate(factory.notEqual(c10, c05))); - assertSerializedEquals(filter); + verify(true, factory.notEqual(c10, c20)); + verify(false, factory.notEqual(c10, c10)); + verify(true, factory.notEqual(c10, c05)); } /** @@ -172,9 +186,19 @@ public final class ComparisonFilterTest extends TestCase { @Test public void testBetween() { expectedName = ComparisonOperatorName.valueOf(FunctionNames.PROPERTY_IS_BETWEEN); - assertTrue (evaluate(factory.between(c10, c05, c20))); - assertFalse(evaluate(factory.between(c20, c05, c10))); - assertFalse(evaluate(factory.between(c05, c10, c20))); + assertTestEquals(true, factory.between(c10, c05, c20)); + assertTestEquals(false, factory.between(c20, c05, c10)); + assertTestEquals(false, factory.between(c05, c10, c20)); + assertSerializedEquals(filter); + } + + /** + * Tests "Equal" between Boolean values. + */ + @Test + public void testBooleans() { + expectedName = ComparisonOperatorName.PROPERTY_IS_EQUAL_TO; + assertTestEquals(true, factory.equal(factory.function("isNaN", c10), factory.literal(Boolean.FALSE))); assertSerializedEquals(filter); } } diff --git a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/DynamicOptimizationTest.java b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/DynamicOptimizationTest.java index add8852fd9..c41959b26f 100644 --- a/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/DynamicOptimizationTest.java +++ b/endorsed/src/org.apache.sis.feature/test/org/apache/sis/filter/DynamicOptimizationTest.java @@ -21,6 +21,7 @@ import java.util.HashSet; import java.util.Set; import org.apache.sis.feature.AbstractFeature; import org.apache.sis.feature.builder.FeatureTypeBuilder; +import org.apache.sis.util.internal.shared.Strings; import org.apache.sis.filter.base.Node; // Test dependencies @@ -172,5 +173,23 @@ public final class DynamicOptimizationTest extends TestCaseWithLogs { void assertAllPropertiesHaveBeenRead() { assertTrue(expectedQueries.isEmpty(), expectedQueries.toString()); } + + /** Overridden for avoiding interference with the debugger. */ + @Override public String toString() { + return Strings.toString(getClass(), "type", getType().getName()); + } + } + + /** + * Tests "Equal" between Boolean values. + */ + @Test + public void testBooleans() { + final var filter = factory.equal(factory.function("isNaN", factory.property("population")), factory.literal(Boolean.FALSE)); + final var negate = factory.not(filter); + assertTrue (filter.test(city)); + assertFalse(negate.test(city)); + assertTrue (new Optimization().apply(filter).test(city)); + assertFalse(new Optimization().apply(negate).test(city)); } } diff --git a/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java b/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java index 108a3a9168..2f7dddfe17 100644 --- a/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java +++ b/endorsed/src/org.apache.sis.util/main/org/apache/sis/math/NumberType.java @@ -276,7 +276,7 @@ public enum NumberType { * * @see #isInteger() * @see #isFractional() - * @see #isNumber() + * @see #isReal() */ private final byte category;
