michaelmior commented on a change in pull request #2111:
URL: https://github.com/apache/calcite/pull/2111#discussion_r473361767



##########
File path: 
core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java
##########
@@ -385,7 +410,18 @@ private static RelDataType toSql(RelDataTypeFactory 
typeFactory,
   }
 
   public List<SqlOperator> getOperatorList() {
-    return null;
+    final ImmutableList.Builder<SqlOperator> b = ImmutableList.builder();

Review comment:
       Can this be called `builder` instead of `b`?

##########
File path: 
core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java
##########
@@ -285,66 +286,95 @@ public static SqlOperatorTable operatorTable(String... 
classNames) {
           className, "*", true);
     }
 
-    // The following is technical debt; see [CALCITE-2082] Remove
-    // RelDataTypeFactory argument from SqlUserDefinedAggFunction constructor
-    final SqlTypeFactoryImpl typeFactory =
-        new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
-
     final ListSqlOperatorTable table = new ListSqlOperatorTable();
     for (String name : schema.getFunctionNames()) {
-      for (Function function : schema.getFunctions(name, true)) {
+      schema.getFunctions(name, true).forEach(function -> {
         final SqlIdentifier id = new SqlIdentifier(name, SqlParserPos.ZERO);
-        table.add(
-            toOp(typeFactory, id, function));
-      }
+        table.add(toOp(id, function));
+      });
     }
     return table;
   }
 
-  private SqlOperator toOp(SqlIdentifier name, final Function function) {
-    return toOp(typeFactory, name, function);
-  }
-
-  /** Converts a function to a {@link org.apache.calcite.sql.SqlOperator}.
-   *
-   * <p>The {@code typeFactory} argument is technical debt; see [CALCITE-2082]
-   * Remove RelDataTypeFactory argument from SqlUserDefinedAggFunction
-   * constructor. */
-  private static SqlOperator toOp(RelDataTypeFactory typeFactory,
-      SqlIdentifier name, final Function function) {
-    List<RelDataType> argTypes = new ArrayList<>();
-    List<SqlTypeFamily> typeFamilies = new ArrayList<>();
-    for (FunctionParameter o : function.getParameters()) {
-      final RelDataType type = o.getType(typeFactory);
-      argTypes.add(type);
-      typeFamilies.add(
-          Util.first(type.getSqlTypeName().getFamily(), SqlTypeFamily.ANY));
-    }
-    final FamilyOperandTypeChecker typeChecker =
-        OperandTypes.family(typeFamilies, i ->
-            function.getParameters().get(i).isOptional());
-    final List<RelDataType> paramTypes = toSql(typeFactory, argTypes);
+  /** Converts a function to a {@link org.apache.calcite.sql.SqlOperator}. */
+  private static SqlOperator toOp(SqlIdentifier name,
+      final org.apache.calcite.schema.Function function) {
+    final Function<RelDataTypeFactory, List<RelDataType>> argTypesFactory =
+        typeFactory -> function.getParameters()
+            .stream()
+            .map(o -> o.getType(typeFactory))
+            .collect(Util.toImmutableList());
+    final Function<RelDataTypeFactory, List<SqlTypeFamily>> 
typeFamiliesFactory =
+        typeFactory -> argTypesFactory.apply(typeFactory)
+            .stream()
+            .map(type ->
+                Util.first(type.getSqlTypeName().getFamily(),
+                    SqlTypeFamily.ANY))
+            .collect(Util.toImmutableList());
+    final Function<RelDataTypeFactory, List<RelDataType>> paramTypesFactory =
+        typeFactory ->
+            argTypesFactory.apply(typeFactory)
+                .stream()
+                .map(type -> toSql(typeFactory, type))
+                .collect(Util.toImmutableList());
+
+    // Use a short-lived type factory to populate "typeFamilies" and 
"argTypes".
+    // SqlOperandMetadata.paramTypes will use the real type factory, during
+    // validation.
+    final RelDataTypeFactory dummyTypeFactory = new JavaTypeFactoryImpl();
+    final List<RelDataType> argTypes = argTypesFactory.apply(dummyTypeFactory);
+    final List<SqlTypeFamily> typeFamilies =
+        typeFamiliesFactory.apply(dummyTypeFactory);
+
+    final SqlOperandTypeInference operandTypeInference =
+        InferTypes.explicit(argTypes);
+
+    final SqlOperandMetadata operandMetadata =
+        OperandTypes.operandMetadata(typeFamilies, paramTypesFactory,
+            i -> function.getParameters().get(i).getName(),
+            i -> function.getParameters().get(i).isOptional());
+
+    final SqlKind kind = kind(function);
     if (function instanceof ScalarFunction) {
-      return new SqlUserDefinedFunction(name, infer((ScalarFunction) function),
-          InferTypes.explicit(argTypes), typeChecker, paramTypes, function);
+      final SqlReturnTypeInference returnTypeInference =
+          infer((ScalarFunction) function);
+      return new SqlUserDefinedFunction(name, kind, returnTypeInference,
+          operandTypeInference, operandMetadata, function);
     } else if (function instanceof AggregateFunction) {
-      return new SqlUserDefinedAggFunction(name,
-          infer((AggregateFunction) function), InferTypes.explicit(argTypes),
-          typeChecker, (AggregateFunction) function, false, false,
-          Optionality.FORBIDDEN, typeFactory);
+      final SqlReturnTypeInference returnTypeInference =
+          infer((AggregateFunction) function);
+      return new SqlUserDefinedAggFunction(name, kind,
+          returnTypeInference, operandTypeInference,
+          operandMetadata, (AggregateFunction) function, false, false,
+          Optionality.FORBIDDEN);
     } else if (function instanceof TableMacro) {
-      return new SqlUserDefinedTableMacro(name, ReturnTypes.CURSOR,
-          InferTypes.explicit(argTypes), typeChecker, paramTypes,
-          (TableMacro) function);
+      return new SqlUserDefinedTableMacro(name, kind, ReturnTypes.CURSOR,
+          operandTypeInference, operandMetadata, (TableMacro) function);
     } else if (function instanceof TableFunction) {
-      return new SqlUserDefinedTableFunction(name, ReturnTypes.CURSOR,
-          InferTypes.explicit(argTypes), typeChecker, paramTypes,
-          (TableFunction) function);
+      return new SqlUserDefinedTableFunction(name, kind, ReturnTypes.CURSOR,
+          operandTypeInference, operandMetadata, (TableFunction) function);
     } else {
       throw new AssertionError("unknown function type " + function);
     }
   }
 
+  /** Deduces the {@link org.apache.calcite.sql.SqlKind} of a user-defined
+   * function based on a {@link Hints} annotation, if present. */
+  private static SqlKind kind(org.apache.calcite.schema.Function function) {
+    if (function instanceof ScalarFunctionImpl) {
+      Hints hints =
+          ((ScalarFunctionImpl) function).method.getAnnotation(Hints.class);
+      if (hints != null) {
+        for (String hint : hints.value()) {
+          if (hint.startsWith("SqlKind:")) {
+            return SqlKind.valueOf(hint.substring("SqlKind:".length()));

Review comment:
       Having hints just be plain strings seems like it could up creating a bit 
of a mess if they start getting used more. As a simple extension, what if a 
hint was just a pair with the first part being an enum defined in the hints 
class specifying the type?

##########
File path: core/src/main/java/org/apache/calcite/rel/rules/SpatialRules.java
##########
@@ -0,0 +1,322 @@
+/*
+ * 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.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptPredicateList;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.runtime.GeoFunctions;
+import org.apache.calcite.runtime.Geometries;
+import org.apache.calcite.runtime.HilbertCurve2D;
+import org.apache.calcite.runtime.SpaceFillingCurve2D;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+
+import com.esri.core.geometry.Envelope;
+import com.esri.core.geometry.Point;
+import com.google.common.collect.ImmutableList;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+
+/**
+ * Collection of planner rules that convert
+ * calls to spatial functions into more efficient expressions.
+ *
+ * <p>The rules allow Calcite to use spatial indexes. For example the following
+ * query:
+ *
+ * <blockquote>SELECT ...
+ * FROM Restaurants AS r
+ * WHERE ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)
+ * </blockquote>
+ *
+ * <p>is rewritten to
+ *
+ * <blockquote>SELECT ...
+ * FROM Restaurants AS r
+ * WHERE (r.h BETWEEN 100 AND 150
+ *        OR r.h BETWEEN 170 AND 185)
+ * AND ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)
+ * </blockquote>
+ *
+ * <p>if there is the constraint
+ *
+ * <blockquote>CHECK (h = Hilbert(8, r.longitude, r.latitude))</blockquote>
+ *
+ * <p>If the {@code Restaurants} table is sorted on {@code h} then the latter
+ * query can be answered using two limited range-scans, and so is much more
+ * efficient.
+ *
+ * <p>Note that the original predicate
+ * {@code ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)}
+ * is still present, but is evaluated after the approximate predicate has
+ * eliminated many potential matches.
+ */
+public abstract class SpatialRules {
+
+  private SpatialRules() {}
+
+  private static final RexUtil.RexFinder DWITHIN_FINDER =
+      RexUtil.find(EnumSet.of(SqlKind.ST_DWITHIN, SqlKind.ST_CONTAINS));
+
+  private static final RexUtil.RexFinder HILBERT_FINDER =
+      RexUtil.find(SqlKind.HILBERT);
+
+  public static final RelOptRule INSTANCE =
+      FilterHilbertRule.Config.DEFAULT.toRule();
+
+  /** Returns a geometry if an expression is constant, null otherwise. */
+  private static Geometries.Geom constantGeom(RexNode e) {
+    switch (e.getKind()) {
+    case CAST:
+      return constantGeom(((RexCall) e).getOperands().get(0));
+    case LITERAL:
+      return (Geometries.Geom) ((RexLiteral) e).getValue();
+    default:
+      return null;
+    }
+  }
+
+  /** Rule that converts ST_DWithin in a Filter condition into a predicate on
+   * a Hilbert curve. */
+  @SuppressWarnings("WeakerAccess")
+  public static class FilterHilbertRule
+      extends RelRule<FilterHilbertRule.Config> {
+    protected FilterHilbertRule(Config config) {
+      super(config);
+    }
+
+    @Override public void onMatch(RelOptRuleCall call) {
+      final Filter filter = call.rel(0);
+      final List<RexNode> conjunctions = new ArrayList<>();
+      RelOptUtil.decomposeConjunction(filter.getCondition(), conjunctions);
+
+      // Match a predicate
+      //   r.hilbert = hilbert(r.longitude, r.latitude)
+      // to one of the conjunctions
+      //   ST_DWithin(ST_Point(x, y), ST_Point(r.longitude, r.latitude), d)
+      // and if it matches add a new conjunction before it,
+      //   r.hilbert between h1 and h2
+      //   or r.hilbert between h3 and h4
+      // where {[h1, h2], [h3, h4]} are the ranges of the Hilbert curve
+      // intersecting the square
+      //   (r.longitude - d, r.latitude - d, r.longitude + d, r.latitude + d)
+      final RelOptPredicateList predicates =
+          call.getMetadataQuery().getAllPredicates(filter.getInput());
+      int changeCount = 0;
+      for (RexNode predicate : predicates.pulledUpPredicates) {
+        final RelBuilder builder = call.builder();
+        if (predicate.getKind() == SqlKind.EQUALS) {
+          final RexCall eqCall = (RexCall) predicate;
+          if (eqCall.operands.get(0) instanceof RexInputRef
+              && eqCall.operands.get(1).getKind() == SqlKind.HILBERT) {
+            final RexInputRef ref  = (RexInputRef) eqCall.operands.get(0);

Review comment:
       In the future, it would be nice if this equality condition was not 
necessary, but I understand this is likely to require DB-specific 
implementation so it's a reasonable starting point.

##########
File path: core/src/main/java/org/apache/calcite/rel/rules/SpatialRules.java
##########
@@ -0,0 +1,322 @@
+/*
+ * 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.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptPredicateList;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.runtime.GeoFunctions;
+import org.apache.calcite.runtime.Geometries;
+import org.apache.calcite.runtime.HilbertCurve2D;
+import org.apache.calcite.runtime.SpaceFillingCurve2D;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+
+import com.esri.core.geometry.Envelope;
+import com.esri.core.geometry.Point;
+import com.google.common.collect.ImmutableList;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+
+/**
+ * Collection of planner rules that convert
+ * calls to spatial functions into more efficient expressions.
+ *
+ * <p>The rules allow Calcite to use spatial indexes. For example the following
+ * query:
+ *
+ * <blockquote>SELECT ...
+ * FROM Restaurants AS r
+ * WHERE ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)
+ * </blockquote>
+ *
+ * <p>is rewritten to
+ *
+ * <blockquote>SELECT ...
+ * FROM Restaurants AS r
+ * WHERE (r.h BETWEEN 100 AND 150
+ *        OR r.h BETWEEN 170 AND 185)
+ * AND ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)
+ * </blockquote>
+ *
+ * <p>if there is the constraint
+ *
+ * <blockquote>CHECK (h = Hilbert(8, r.longitude, r.latitude))</blockquote>
+ *
+ * <p>If the {@code Restaurants} table is sorted on {@code h} then the latter
+ * query can be answered using two limited range-scans, and so is much more
+ * efficient.
+ *
+ * <p>Note that the original predicate
+ * {@code ST_DWithin(ST_Point(10, 20), ST_Point(r.longitude, r.latitude), 5)}
+ * is still present, but is evaluated after the approximate predicate has
+ * eliminated many potential matches.
+ */
+public abstract class SpatialRules {
+
+  private SpatialRules() {}
+
+  private static final RexUtil.RexFinder DWITHIN_FINDER =
+      RexUtil.find(EnumSet.of(SqlKind.ST_DWITHIN, SqlKind.ST_CONTAINS));
+
+  private static final RexUtil.RexFinder HILBERT_FINDER =
+      RexUtil.find(SqlKind.HILBERT);
+
+  public static final RelOptRule INSTANCE =
+      FilterHilbertRule.Config.DEFAULT.toRule();
+
+  /** Returns a geometry if an expression is constant, null otherwise. */
+  private static Geometries.Geom constantGeom(RexNode e) {
+    switch (e.getKind()) {
+    case CAST:
+      return constantGeom(((RexCall) e).getOperands().get(0));
+    case LITERAL:
+      return (Geometries.Geom) ((RexLiteral) e).getValue();
+    default:
+      return null;
+    }
+  }
+
+  /** Rule that converts ST_DWithin in a Filter condition into a predicate on
+   * a Hilbert curve. */
+  @SuppressWarnings("WeakerAccess")
+  public static class FilterHilbertRule
+      extends RelRule<FilterHilbertRule.Config> {
+    protected FilterHilbertRule(Config config) {
+      super(config);
+    }
+
+    @Override public void onMatch(RelOptRuleCall call) {
+      final Filter filter = call.rel(0);
+      final List<RexNode> conjunctions = new ArrayList<>();
+      RelOptUtil.decomposeConjunction(filter.getCondition(), conjunctions);
+
+      // Match a predicate
+      //   r.hilbert = hilbert(r.longitude, r.latitude)
+      // to one of the conjunctions
+      //   ST_DWithin(ST_Point(x, y), ST_Point(r.longitude, r.latitude), d)
+      // and if it matches add a new conjunction before it,
+      //   r.hilbert between h1 and h2
+      //   or r.hilbert between h3 and h4
+      // where {[h1, h2], [h3, h4]} are the ranges of the Hilbert curve
+      // intersecting the square
+      //   (r.longitude - d, r.latitude - d, r.longitude + d, r.latitude + d)
+      final RelOptPredicateList predicates =
+          call.getMetadataQuery().getAllPredicates(filter.getInput());
+      int changeCount = 0;
+      for (RexNode predicate : predicates.pulledUpPredicates) {
+        final RelBuilder builder = call.builder();
+        if (predicate.getKind() == SqlKind.EQUALS) {
+          final RexCall eqCall = (RexCall) predicate;
+          if (eqCall.operands.get(0) instanceof RexInputRef
+              && eqCall.operands.get(1).getKind() == SqlKind.HILBERT) {
+            final RexInputRef ref  = (RexInputRef) eqCall.operands.get(0);
+            final RexCall hilbert = (RexCall) eqCall.operands.get(1);
+            final RexUtil.RexFinder finder = RexUtil.find(ref);
+            if (finder.anyContain(conjunctions)) {
+              // If the condition already contains "ref", it is probable that
+              // this rule has already fired once.
+              continue;
+            }
+            for (int i = 0; i < conjunctions.size();) {
+              final List<RexNode> replacements =
+                  replaceSpatial(conjunctions.get(i), builder, ref, hilbert);
+              if (replacements != null) {
+                conjunctions.remove(i);
+                conjunctions.addAll(i, replacements);
+                i += replacements.size();
+                ++changeCount;
+              } else {
+                ++i;
+              }
+            }
+          }
+        }
+        if (changeCount > 0) {
+          call.transformTo(
+              builder.push(filter.getInput())
+                  .filter(conjunctions)
+                  .build());
+          return; // we found one useful constraint; don't look for more

Review comment:
       I'm sure there's a good reason, but it's not immediately obvious to me 
why you wouldn't want to keep looking.

##########
File path: 
core/src/main/java/org/apache/calcite/rel/RelReferentialConstraint.java
##########
@@ -27,10 +27,13 @@
 public interface RelReferentialConstraint {
   //~ Methods ----------------------------------------------------------------
 
-  /**
-   * Returns the number of columns in the keys.
-   */
-  int getNumColumns();
+  /** Returns the number of columns in the keys.
+   *
+   * @deprecated Use {@code getColumnPairs().size()} */
+  @Deprecated // to be removed before 2.0
+  default int getNumColumns() {
+    return getColumnPairs().size();
+  }

Review comment:
       This deprecation isn't really part of this PR and putting a deprecation 
in here probably means it won't end up in the release notes. I don't imagine 
this is widely used though, so maybe it doesn't really matter.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to