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 b0c524e Remove the static `FeatureExpression.expectedType(…)` method
because its heuristic (return the type of the singleton property if a feature
contains only one property) is dangerous: there is no reason for an operation
to have results of the same type.
b0c524e is described below
commit b0c524ed86fb7685f6261cea2154ca0efe297f77
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Fri Dec 31 05:24:12 2021 +0100
Remove the static `FeatureExpression.expectedType(…)` method because its
heuristic
(return the type of the singleton property if a feature contains only one
property)
is dangerous: there is no reason for an operation to have results of the
same type.
---
.../org/apache/sis/filter/ConvertFunction.java | 12 ++++--
.../java/org/apache/sis/filter/Optimization.java | 2 +-
.../sis/internal/feature/FeatureExpression.java | 49 ++++++++++++----------
.../apache/sis/internal/feature/package-info.java | 2 +-
.../sis/internal/filter/sqlmm/SpatialFunction.java | 19 +++++----
.../sis/internal/filter/sqlmm/package-info.java | 2 +-
.../java/org/apache/sis/storage/FeatureQuery.java | 28 +++++++++----
7 files changed, 67 insertions(+), 47 deletions(-)
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 e37caa8..50014b3 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
@@ -37,7 +37,7 @@ import org.opengis.feature.FeatureType;
* Expression whose results are converted to a different type.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
*
* @param <R> the type of resources (e.g. {@link
org.opengis.feature.Feature}) used as inputs.
* @param <S> the type of value computed by the wrapped exception. This is
the type to convert.
@@ -157,12 +157,16 @@ final class ConvertFunction<R,S,V> extends
UnaryFunction<R,S>
}
/**
- * Provides the expected type of values produced by this expression
- * when a feature of the given type is evaluated.
+ * Provides the type of values produced by this expression when a feature
of the given type is evaluated.
+ * May return {@code null} if the type can not be determined.
*/
@Override
public PropertyTypeBuilder expectedType(final FeatureType valueType, final
FeatureTypeBuilder addTo) {
- final PropertyTypeBuilder p =
FeatureExpression.expectedType(expression, valueType, addTo);
+ final FeatureExpression<?,?> fex =
FeatureExpression.castOrCopy(expression);
+ if (fex == null) {
+ return null;
+ }
+ final PropertyTypeBuilder p = fex.expectedType(valueType, addTo);
if (p instanceof AttributeTypeBuilder<?>) {
return ((AttributeTypeBuilder<?>)
p).setValueClass(getValueClass());
}
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 ccd3fd7..ccffdb4 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
@@ -475,7 +475,7 @@ public class Optimization {
/**
* Creates a constant, literal value that can be used in expressions.
- * This is a helper methods for optimizations that simplified an
expression to a constant value.
+ * This is a helper methods for optimizations which simplified an
expression to a constant value.
*
* @param <R> the type of resources used as inputs.
* @param <V> the type of the value of the literal.
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 90ab6ae..f1572b8 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
@@ -17,10 +17,12 @@
package org.apache.sis.internal.feature;
import org.opengis.feature.FeatureType;
-import org.opengis.feature.PropertyType;
import org.opengis.feature.AttributeType;
+import org.opengis.filter.Literal;
import org.opengis.filter.Expression;
-import org.apache.sis.internal.util.CollectionsExt;
+import org.opengis.filter.ValueReference;
+import org.apache.sis.filter.Optimization;
+import org.apache.sis.filter.DefaultFilterFactory;
import org.apache.sis.feature.builder.FeatureTypeBuilder;
import org.apache.sis.feature.builder.PropertyTypeBuilder;
@@ -34,7 +36,7 @@ import org.apache.sis.feature.builder.PropertyTypeBuilder;
*
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
*
* @param <R> the type of resources (e.g. {@link
org.opengis.feature.Feature}) used as inputs.
* @param <V> the type of values computed by the expression.
@@ -67,34 +69,35 @@ public interface FeatureExpression<R,V> extends
Expression<R,V> {
PropertyTypeBuilder expectedType(FeatureType valueType, FeatureTypeBuilder
addTo);
/**
- * Provides the type of results computed by the given expression.
- * This method executes the first of the following choices that apply:
+ * Tries to cast or convert the given expression to a {@link
FeatureExpression}.
+ * If the given expression can not be casted, then this method creates a
copy
+ * provided that the expression is one of the following type:
*
* <ol>
- * <li>If the expression implements {@link FeatureExpression}, delegate
to {@link #expectedType(FeatureType,
- * FeatureTypeBuilder)}. Note that the invoked method may throw an
{@link IllegalArgumentException}.</li>
- * <li>Otherwise if the given feature type contains exactly one property
(including inherited properties),
- * adds that property to the given builder.</li>
- * <li>Otherwise returns {@code null}.</li>
+ * <li>{@link Literal}.</li>
+ * <li>{@link ValueReference}, assuming that the expression expects
feature instances.</li>
* </ol>
*
+ * Otherwise this method returns {@code null}.
* It is caller's responsibility to verify if this method returns {@code
null} and to throw an exception in such case.
* We leave that responsibility to the caller because (s)he may be able to
provide better error messages.
*
- * @param expression the expression for which to get the result type, or
{@code null}.
- * @param valueType the type of features to be evaluated by the given
expression.
- * @param addTo where to add the type of properties evaluated by
the given expression.
- * @return builder of the added property, or {@code null} if this method
can not add a property.
- * @throws IllegalArgumentException if this method can operate only on
some feature types
- * and the given type is not one of them.
+ * @param candidate the expression to cast or copy. Can be null.
+ * @return the given expression as a feature expression, or {@code null}
if it can not be casted or converted.
*/
- public static PropertyTypeBuilder expectedType(final Expression<?,?>
expression,
- final FeatureType valueType, final FeatureTypeBuilder addTo)
- {
- if (expression instanceof FeatureExpression<?,?>) {
- return ((FeatureExpression<?,?>)
expression).expectedType(valueType, addTo);
+ public static FeatureExpression<?,?> castOrCopy(final Expression<?,?>
candidate) {
+ if (candidate instanceof FeatureExpression<?,?>) {
+ return (FeatureExpression<?,?>) candidate;
+ }
+ final Expression<?,?> copy;
+ if (candidate instanceof Literal<?,?>) {
+ copy = Optimization.literal(((Literal<?,?>) candidate).getValue());
+ } else if (candidate instanceof ValueReference<?,?>) {
+ final String xpath = ((ValueReference<?,?>) candidate).getXPath();
+ copy = DefaultFilterFactory.forFeatures().property(xpath);
+ } else {
+ return null;
}
- final PropertyType pt =
CollectionsExt.singletonOrNull(valueType.getProperties(true));
- return (pt == null) ? null : addTo.addProperty(pt);
+ return (FeatureExpression<?,?>) copy;
}
}
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/package-info.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/package-info.java
index 2cfcc6e..4e460ef 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/package-info.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/package-info.java
@@ -28,7 +28,7 @@
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
* @author Alexis Manin (Geomatys)
- * @version 1.1
+ * @version 1.2
* @since 0.7
* @module
*/
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/SpatialFunction.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/SpatialFunction.java
index 0c49c10..1615ed3 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/SpatialFunction.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/SpatialFunction.java
@@ -42,7 +42,7 @@ import org.opengis.filter.InvalidFilterValueException;
*
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
*
* @param <R> the type of resources (e.g. {@link
org.opengis.feature.Feature}) used as inputs.
*
@@ -210,13 +210,16 @@ abstract class SpatialFunction<R> extends Node implements
FeatureExpression<R,Ob
public PropertyTypeBuilder expectedType(final FeatureType valueType, final
FeatureTypeBuilder addTo) {
AttributeTypeBuilder<?> att;
cases: if (operation.isGeometryInOut()) {
- final PropertyTypeBuilder type =
FeatureExpression.expectedType(getParameters().get(0), valueType, addTo);
- if (type instanceof AttributeTypeBuilder<?>) {
- att = (AttributeTypeBuilder<?>) type;
- final Geometries<?> library =
Geometries.implementation(att.getValueClass());
- if (library != null) {
- att = att.setValueClass(operation.getReturnType(library));
- break cases;
+ final FeatureExpression<?,?> fex =
FeatureExpression.castOrCopy(getParameters().get(0));
+ if (fex != null) {
+ final PropertyTypeBuilder type = fex.expectedType(valueType,
addTo);
+ if (type instanceof AttributeTypeBuilder<?>) {
+ att = (AttributeTypeBuilder<?>) type;
+ final Geometries<?> library =
Geometries.implementation(att.getValueClass());
+ if (library != null) {
+ att =
att.setValueClass(operation.getReturnType(library));
+ break cases;
+ }
}
}
throw new
InvalidFilterValueException(Resources.format(Resources.Keys.NotAGeometryAtFirstExpression));
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/package-info.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/package-info.java
index ceaa381..e25ed2f 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/package-info.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/package-info.java
@@ -31,7 +31,7 @@
*
* @author Johann Sorel (Geomatys)
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
* @since 1.1
* @module
*/
diff --git
a/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
b/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
index 9368d53..6b5626d 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/FeatureQuery.java
@@ -570,8 +570,8 @@ public class FeatureQuery extends Query implements
Cloneable, Serializable {
* If some expressions have no name, default names are computed as below:
*
* <ul>
- * <li>If the expression is an instance of {@link ValueReference},
- * the tip of the {@linkplain ValueReference#getXPath() x-path}.</li>
+ * <li>If the expression is an instance of {@link ValueReference}, the
name of the
+ * property referenced by the {@linkplain ValueReference#getXPath()
x-path}.</li>
* <li>Otherwise the localized string "Unnamed #1" with increasing
numbers.</li>
* </ul>
*
@@ -596,8 +596,9 @@ public class FeatureQuery extends Query implements
Cloneable, Serializable {
*/
GenericName name = projection[column].alias;
final Expression<?,?> expression = projection[column].expression;
- final PropertyTypeBuilder resultType =
FeatureExpression.expectedType(expression, valueType, ftb);
- if (resultType == null) {
+ final FeatureExpression<?,?> fex =
FeatureExpression.castOrCopy(expression);
+ final PropertyTypeBuilder resultType;
+ if (fex == null || (resultType = fex.expectedType(valueType, ftb))
== null) {
throw new
InvalidFilterValueException(Resources.format(Resources.Keys.InvalidExpression_2,
expression.getFunctionName().toInternationalString(), column));
}
@@ -611,24 +612,33 @@ public class FeatureQuery extends Query implements
Cloneable, Serializable {
names = new
HashSet<>(Containers.hashMapCapacity(projection.length));
for (final NamedExpression p : projection) {
if (p.alias != null) {
- names.add(p.alias.tip().toString());
+ names.add(p.alias.toString());
}
}
}
/*
- * The rules for creating a default name are documented in
this method Javadoc.
- * Note that despite the use of `Vocabulary` resources, the
name will be unlocalized
- * (for easier programmatic use) because `GenericName`
implementation is designed for
- * providing localized names only if explicitly requested.
+ * If the expression is a `ValueReference`, the `PropertyType`
instance can be taken directly
+ * from the source feature (the Apache SIS implementation does
just that). If the name is set,
+ * then we assume that it is correct. Otherwise we take the
tip of the XPath.
*/
CharSequence text = null;
if (expression instanceof ValueReference<?,?>) {
+ final GenericName current = resultType.getName();
+ if (current != null && names.add(current.toString())) {
+ continue;
+ }
String xpath = ((ValueReference<?,?>)
expression).getXPath().trim();
xpath = xpath.substring(xpath.lastIndexOf('/') + 1); //
Works also if '/' is not found.
if (!(xpath.isEmpty() || names.contains(xpath))) {
text = xpath;
}
}
+ /*
+ * If we still have no name at this point, create a name like
"Unnamed #1".
+ * Note that despite the use of `Vocabulary` resources, the
name will be unlocalized
+ * (for easier programmatic use) because `GenericName`
implementation is designed for
+ * providing localized names only if explicitly requested.
+ */
if (text == null) do {
text =
Vocabulary.formatInternational(Vocabulary.Keys.Unnamed_1, ++unnamedNumber);
} while (!names.add(text.toString()));
