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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 2bf0bc48a0 Clarify whether to inherit the intrinsic properties of a function or to inherit only the transitive properties. 2bf0bc48a0 is described below commit 2bf0bc48a0c5a310972b1eac800b5289ac398b52 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sat May 6 15:37:45 2023 +0200 Clarify whether to inherit the intrinsic properties of a function or to inherit only the transitive properties. --- .../org/apache/sis/filter/AssociationValue.java | 5 +- .../org/apache/sis/filter/ConvertFunction.java | 4 +- .../java/org/apache/sis/filter/LeafExpression.java | 24 ++++--- .../java/org/apache/sis/filter/Optimization.java | 4 +- .../java/org/apache/sis/filter/UnaryFunction.java | 11 --- .../sis/internal/feature/FeatureExpression.java | 2 +- .../java/org/apache/sis/internal/filter/Node.java | 79 +++++++++++----------- .../java/org/apache/sis/math/FunctionProperty.java | 73 ++++++++++++++++++++ .../org/apache/sis/math/FunctionPropertyTest.java | 49 ++++++++++++++ .../apache/sis/test/suite/UtilityTestSuite.java | 3 +- 10 files changed, 190 insertions(+), 64 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/AssociationValue.java b/core/sis-feature/src/main/java/org/apache/sis/filter/AssociationValue.java index 3a31459b1f..49cdb7f41e 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/AssociationValue.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/AssociationValue.java @@ -105,10 +105,13 @@ final class AssociationValue<V> extends LeafExpression<Feature, V> /** * Returns the manner in which values are computed from given resources. + * This method assumes an initially empty set of properties, then adds the transitive properties. + * This method does not inherit directly the properties of the {@linkplain #accessor} because it + * does not operate on the same resource, so the non-transitive function properties may not hold. */ @Override public Set<FunctionProperty> properties() { - return properties(accessor); + return transitiveProperties(accessor.getParameters()); } /** diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/ConvertFunction.java b/core/sis-feature/src/main/java/org/apache/sis/filter/ConvertFunction.java index d4627844fd..5d5ae63761 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/ConvertFunction.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/ConvertFunction.java @@ -123,10 +123,12 @@ final class ConvertFunction<R,S,V> extends UnaryFunction<R,S> /** * Returns the manner in which values are computed from given resources. + * This expression can be represented as the concatenation of the user-supplied expression with the converter. + * Because this {@code ConvertFunction} does nothing on its own, it does not have its own set of properties. */ @Override public Set<FunctionProperty> properties() { - return properties(expression, converter); + return FunctionProperty.concatenate(properties(expression), converter.properties()); } /** diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/LeafExpression.java b/core/sis-feature/src/main/java/org/apache/sis/filter/LeafExpression.java index 1a34137d86..c7801d8f02 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/LeafExpression.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/LeafExpression.java @@ -74,6 +74,8 @@ abstract class LeafExpression<R,V> extends Node implements FeatureExpression<R,V /** * Returns the manner in which values are computed from given resources. + * Since leaf expressions have no parameters, the only properties to return are the intrinsic properties + * of this function. The default implementation assumes that there is none, but subclasses may override. */ @Override public Set<FunctionProperty> properties() { @@ -91,6 +93,10 @@ abstract class LeafExpression<R,V> extends Node implements FeatureExpression<R,V * @param <V> the type of value computed by the expression. */ static class Literal<R,V> extends LeafExpression<R,V> implements org.opengis.filter.Literal<R,V> { + /** The properties of this function, which returns constants. */ + private static final Set<FunctionProperty> CONSTANT = + Set.of(FunctionProperty.ORDER_PRESERVING, FunctionProperty.ORDER_REVERSING); + /** For cross-version compatibility. */ private static final long serialVersionUID = -8383113218490957822L; @@ -124,6 +130,12 @@ abstract class LeafExpression<R,V> extends Node implements FeatureExpression<R,V return value; } + /** Notifies that results are constant. */ + @SuppressWarnings("ReturnOfCollectionOrArrayField") // Because immutable. + @Override public Set<FunctionProperty> properties() { + return CONSTANT; + } + /** * Returns an expression that provides values as instances of the specified class. * @@ -186,9 +198,13 @@ abstract class LeafExpression<R,V> extends Node implements FeatureExpression<R,V /** * A literal value which is the result of transforming another literal. + * This is used for example when a geometry is transformed to a different CRS for computational purposes. + * This transformation should be invisible to users, so we need to provide the original expressions when needed. * * @param <R> the type of resources used as inputs. * @param <V> the type of value computed by the expression. + * + * @see BinaryGeometryFilter#original(Expression) */ static final class Transformed<R,V> extends Literal<R,V> implements Optimization.OnExpression<R,V> { /** For cross-version compatibility. */ @@ -204,14 +220,6 @@ abstract class LeafExpression<R,V> extends Node implements FeatureExpression<R,V this.original = original; } - /** - * Returns the manner in which values are computed from given resources. - */ - @Override - public Set<FunctionProperty> properties() { - return properties(original); - } - /** * Returns the same literal without the reference to the original expression. * Since this {@code Transformed} instance will not longer be unwrapped, diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/Optimization.java b/core/sis-feature/src/main/java/org/apache/sis/filter/Optimization.java index ea85a11656..48f684cfb9 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/Optimization.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/Optimization.java @@ -502,7 +502,7 @@ public class Optimization { * @since 1.4 */ public static Set<FunctionProperty> properties(final Filter<?> filter) { - return Node.properties(filter.getExpressions()); + return Node.transitiveProperties(filter.getExpressions()); } /** @@ -511,8 +511,10 @@ public class Optimization { * The values of particular interest are: * * <ul> + * <li>{@link FunctionProperty#INJECTIVE} if the computed value is unique for each resource instance (e.g. identifiers).</li> * <li>{@link FunctionProperty#VOLATILE} if the computed value changes each time that the expression is evaluated, * even if the resource to evaluate stay the same immutable instance.</li> + * <li>{@link FunctionProperty#isConstant(Set)} if the expression returns a constant value.</li> * </ul> * * @param expression the expression for which to query function properties. diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/UnaryFunction.java b/core/sis-feature/src/main/java/org/apache/sis/filter/UnaryFunction.java index c7539db5dc..cff832db14 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/UnaryFunction.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/UnaryFunction.java @@ -16,15 +16,12 @@ */ package org.apache.sis.filter; -import java.util.Set; import java.util.List; import java.util.Collection; import java.util.Optional; import org.apache.sis.xml.NilReason; import org.apache.sis.util.ArgumentChecks; -import org.apache.sis.math.FunctionProperty; import org.apache.sis.internal.filter.Node; -import org.apache.sis.internal.feature.FeatureExpression; // Branch-dependent imports import org.opengis.filter.Filter; @@ -105,14 +102,6 @@ class UnaryFunction<R,V> extends Node { return getExpressions(); } - /** - * Returns the manner in which values are computed from given resources. - * Defined for {@link FeatureExpression#properties()} implementations. - */ - public Set<FunctionProperty> properties() { - return properties(expression); - } - /** * Filter operator that checks if an expression's value is {@code null}. A {@code null} diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/FeatureExpression.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/FeatureExpression.java index edb5b1f899..34acd54f98 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/FeatureExpression.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/FeatureExpression.java @@ -55,7 +55,7 @@ public interface FeatureExpression<R,V> extends Expression<R,V> { * @return the manners in which values are computed from resources. */ default Set<FunctionProperty> properties() { - return Node.properties(getParameters()); + return Node.transitiveProperties(getParameters()); } /** diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java index 629348d66a..0dedb78d65 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java @@ -34,7 +34,6 @@ import org.apache.sis.internal.feature.Geometries; import org.apache.sis.internal.feature.GeometryWrapper; import org.apache.sis.internal.feature.FeatureExpression; import org.apache.sis.util.iso.Names; -import org.apache.sis.util.ObjectConverter; import org.apache.sis.util.collection.DefaultTreeTable; import org.apache.sis.util.collection.TableColumn; import org.apache.sis.util.collection.TreeTable; @@ -227,15 +226,19 @@ public abstract class Node implements Serializable { } /** - * The set of all properties that make sense for {@link FeatureExpression#properties()}. - * In current version, only one property makes sense and that property is combined with - * all other property sets using a logical {@code OR} operation. More properties may be - * added in the future, and the logical operation will not necessarily be always "OR". + * The set of all properties that may be present in {@link FeatureExpression#properties()} + * when the only available information is the list of parameters. When we do not know how + * an expression is using the parameters, the function properties should be the empty set. + * When combining a function properties with the properties inherited from the parameters, + * the only properties that can be added to an initially empty set are the properties that + * are {@linkplain FunctionProperty#concatenate(Set, Set) concatenated} with the logical + * {@code OR} operation. In the current {@link FunctionProperty} enumeration, the only + * property handled that way is {@code VOLATILE}. */ - private static final Set<FunctionProperty> SUPPORTED_PROPERTIES = Set.of(FunctionProperty.VOLATILE); + private static final Set<FunctionProperty> TRANSITIVE_PROPERTIES = Set.of(FunctionProperty.VOLATILE); /** - * Whether the given set of properties contains the {@link #SUPPORTED_PROPERTIES} singleton value. + * Whether the given set of properties contains the {@link #TRANSITIVE_PROPERTIES} singleton value. * If a future version recognizes more properties, the return type will no longer be a boolean. */ private static boolean isVolatile(final Set<FunctionProperty> properties) { @@ -243,9 +246,12 @@ public abstract class Node implements Serializable { } /** - * Whether the combination of the function properties of all given expression is {@link #SUPPORTED_PROPERTIES}. - * This method assumes that {@code SUPPORTED_PROPERTIES} is a singleton and that the property can be combined + * Whether the combination of the function properties of all given expression is {@link #TRANSITIVE_PROPERTIES}. + * This method assumes that {@code TRANSITIVE_PROPERTIES} is a singleton and that the property can be combined * by a logical {@code OR} operation. + * + * @param operands the expressions from which to get the function properties. + * @return whether is present the single function property that may appear. */ private static <R> boolean isVolatile(final Iterable<Expression<R,?>> operands) { for (final Expression<R,?> operand : operands) { @@ -264,50 +270,43 @@ public abstract class Node implements Serializable { /** * Returns the manner in which values are computed from resources. + * This method delegates to {@link FeatureExpression#properties()} if possible. + * Otherwise this method assumes that the intrinsic properties of the given expression are unknown, + * and inherits from the parameters only the properties that can be added to an initially empty set. * - * @param operand the expression for which to query function properties, or {@code null}. + * @param function the expression for which to query function properties, or {@code null}. * @return the manners in which values are computed from resources. */ - @SuppressWarnings("ReturnOfCollectionOrArrayField") // Because immutable. - public static Set<FunctionProperty> properties(final Expression<?,?> operand) { - if (operand instanceof FeatureExpression<?,?>) { - return ((FeatureExpression<?,?>) operand).properties(); - } else if (operand != null && isVolatile(operand.getParameters())) { - return SUPPORTED_PROPERTIES; - } - return Set.of(); - } - - /** - * Returns the manner in which values are computed from resources in an expression followed by a conversion. - * - * @param operand the expression for which to query function properties, or {@code null}. - * @param converter conversion applied on the expression results. - * @return combined properties. - */ - @SuppressWarnings("ReturnOfCollectionOrArrayField") // Because immutable. - protected static Set<FunctionProperty> properties(final Expression<?,?> operand, final ObjectConverter<?,?> converter) { - if (isVolatile(properties(operand)) || isVolatile(converter.properties())) { - return SUPPORTED_PROPERTIES; + public static Set<FunctionProperty> properties(final Expression<?,?> function) { + if (function instanceof FeatureExpression<?,?>) { + return ((FeatureExpression<?,?>) function).properties(); + } else if (function != null) { + return transitiveProperties(function.getParameters()); + } else { + return Set.of(); } - return Set.of(); } /** * Returns the manner in which values are computed from resources in an expression having the given operands. + * This method assumes that the intrinsic properties of the parent expression or parent filter are unknown, + * and inherits from the operands only the properties that can be added to an initially empty set. + * + * <p>Note that {@code transitiveProperties(function.getParameters())} is <strong>not</strong> equivalent to + * {@code properties(function)}. It is rather equivalent to the following code, where the parent expression + * is not the final step of a chain of operations, and the next step has no known properties:</p> + * + * {@snippet lang="java" : + * FunctionProperty.concatenate(transitiveProperties(operands), Set.of()); + * } * * @param <R> the type of resources. - * @param operands the expressions for which to query function properties. + * @param operands the operands from which to inherit function properties. * @return the manners in which values are computed from resources. */ @SuppressWarnings("ReturnOfCollectionOrArrayField") // Because immutable. - public static <R> Set<FunctionProperty> properties(final Iterable<Expression<R,?>> operands) { - for (final Expression<?,?> operand : operands) { - if (isVolatile(properties(operand))) { - return SUPPORTED_PROPERTIES; - } - } - return Set.of(); + public static <R> Set<FunctionProperty> transitiveProperties(final Iterable<Expression<R,?>> operands) { + return isVolatile(operands) ? TRANSITIVE_PROPERTIES : Set.of(); } /** diff --git a/core/sis-utility/src/main/java/org/apache/sis/math/FunctionProperty.java b/core/sis-utility/src/main/java/org/apache/sis/math/FunctionProperty.java index 2e72f32e9d..1c44488abf 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/math/FunctionProperty.java +++ b/core/sis-utility/src/main/java/org/apache/sis/math/FunctionProperty.java @@ -41,6 +41,8 @@ import java.util.EnumSet; * <td><code>EnumSet.of({@linkplain #ORDER_PRESERVING}, {@linkplain #INJECTIVE})</code></td> * <tr><td>Strictly decreasing</td> * <td><code>EnumSet.of({@linkplain #ORDER_REVERSING}, {@linkplain #INJECTIVE})</code></td> + * <tr><td>{@linkplain #isConstant(Set) Constant}</td> + * <td><code>EnumSet.of({@linkplain #ORDER_PRESERVING}, {@linkplain #ORDER_REVERSING})</code></td> * </table> * * The Javadoc in this class uses the following terms: @@ -153,6 +155,13 @@ public enum FunctionProperty { */ private static final EnumSet<FunctionProperty> BIJECTIVE = EnumSet.of(INJECTIVE, SURJECTIVE); + /** + * A function which is both order preserving and order reversing can only return a constant value. + * + * @see #isConstant(Set) + */ + private static final EnumSet<FunctionProperty> CONSTANT = EnumSet.of(ORDER_PRESERVING, ORDER_REVERSING); + /** * Returns {@code true} if a function having the given set of properties is <cite>bijective</cite>. * Bijective functions have a one-to-one relationship between all input and output values. @@ -186,4 +195,68 @@ public enum FunctionProperty { public static boolean isMonotonic(final Set<FunctionProperty> properties) { return properties.contains(ORDER_PRESERVING) || properties.contains(ORDER_REVERSING); } + + /** + * Returns {@code true} if a function can only return a constant value. + * This convenience method tests if the given set contains <em>all</em> + * of the following properties: + * + * <ul> + * <li>{@link #ORDER_PRESERVING}</li> + * <li>{@link #ORDER_REVERSING}</li> + * </ul> + * + * @param properties the properties of the function to test. + * @return {@code true} if the function can only return a constant value. + * + * @since 1.4 + */ + public static boolean isConstant(final Set<FunctionProperty> properties) { + return properties.containsAll(CONSTANT); + } + + /** + * Returns the properties of a function defined as the concatenation of two other functions. + * The presence of a property in the returned set is determined by combining the presence of + * the same property in the two steps using the following logical operations: + * + * <ul> + * <li>{@link #INVERTIBLE}, {@link #INJECTIVE}, {@link #SURJECTIVE}, {@link #ORDER_PRESERVING}: + * logical {@code AND} operation.</li> + * <li>{@link #ORDER_REVERSING}: + * Logical {@code XOR} operation if the other step is also ordered.</li> + * <li>{@link #VOLATILE}: + * logical {@code OR} operation.</li> + * </ul> + * + * The returned set is always a new instance and is modifiable, + * thus allowing the caller to customize the property set. + * + * @param step1 properties of the first function. + * @param step2 properties of the second function. + * @return properties of the concatenated function as a new and modifiable set. + * + * @since 1.4 + */ + public static EnumSet<FunctionProperty> concatenate(final Set<FunctionProperty> step1, final Set<FunctionProperty> step2) { + final var properties = EnumSet.noneOf(FunctionProperty.class); + properties.addAll (step1); + properties.retainAll(step2); + if (step1.contains(FunctionProperty.VOLATILE) || + step2.contains(FunctionProperty.VOLATILE)) { + properties.add(FunctionProperty.VOLATILE); + } + if (!properties.contains(ORDER_PRESERVING)) { + final boolean r1 = step1.contains(ORDER_REVERSING); + final boolean r2 = step2.contains(ORDER_REVERSING); + if (r1 & r2) { + properties.add(ORDER_PRESERVING); + } else if ((r1 && step2.contains(ORDER_PRESERVING)) || + (r2 && step1.contains(ORDER_PRESERVING))) + { + properties.add(ORDER_REVERSING); + } + } + return properties; + } } diff --git a/core/sis-utility/src/test/java/org/apache/sis/math/FunctionPropertyTest.java b/core/sis-utility/src/test/java/org/apache/sis/math/FunctionPropertyTest.java new file mode 100644 index 0000000000..3c22f39512 --- /dev/null +++ b/core/sis-utility/src/test/java/org/apache/sis/math/FunctionPropertyTest.java @@ -0,0 +1,49 @@ +/* + * 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.sis.math; + +import java.util.Set; +import java.util.EnumSet; +import org.apache.sis.test.TestCase; +import org.junit.*; + +import static org.junit.Assert.*; + + +/** + * Tests the {@link FunctionProperty} class. + * + * @author Martin Desruisseaux (Geomatys) + * @version 1.4 + * @since 1.4 + */ +public final class FunctionPropertyTest extends TestCase { + /** + * Tests {@link FunctionProperty#concatenate(Set, Set)}. + */ + @Test + public void testConcatenate() { + var step1 = EnumSet.of(FunctionProperty.INJECTIVE, FunctionProperty.ORDER_PRESERVING); + var step2 = EnumSet.of(FunctionProperty.VOLATILE, FunctionProperty.ORDER_REVERSING, FunctionProperty.INVERTIBLE); + var concat = EnumSet.of(FunctionProperty.VOLATILE, FunctionProperty.ORDER_REVERSING); + assertEquals(concat, FunctionProperty.concatenate(step1, step2)); + + step1 = EnumSet.of(FunctionProperty.INJECTIVE); + concat = EnumSet.of(FunctionProperty.VOLATILE); + assertEquals(concat, FunctionProperty.concatenate(step1, step2)); + } +} diff --git a/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java b/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java index 3ed5f4d083..84a79d26cd 100644 --- a/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java +++ b/core/sis-utility/src/test/java/org/apache/sis/test/suite/UtilityTestSuite.java @@ -25,7 +25,7 @@ import org.junit.BeforeClass; * All tests from the {@code sis-utility} module, in rough dependency order. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.3 */ @Suite.SuiteClasses({ @@ -62,6 +62,7 @@ import org.junit.BeforeClass; org.apache.sis.math.StatisticsFormatTest.class, org.apache.sis.internal.util.StringsTest.class, org.apache.sis.internal.util.DoubleDoubleTest.class, + org.apache.sis.math.FunctionPropertyTest.class, org.apache.sis.math.LineTest.class, org.apache.sis.math.PlaneTest.class,