date:20200424

[GitHub] [calcite] xy2953396112 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

2020-04-24 Thread GitBox



xy2953396112 commented on a change in pull request #1737:
URL: https://github.com/apache/calcite/pull/1737#discussion_r414963439



##
File path: core/src/main/java/org/apache/calcite/interpreter/Bindables.java
##
@@ -569,6 +633,56 @@ public Node implement(InterpreterImplementor implementor) {
 }
   }
 
+  /** Implementation of {@link org.apache.calcite.rel.core.Intersect} in
+   * bindable calling convention. */
+  public static class BindableIntersect extends Intersect implements 
BindableRel {
+public BindableIntersect(RelOptCluster cluster, RelTraitSet traitSet,
+List inputs, boolean all) {
+  super(cluster, traitSet, inputs, all);
+}
+
+public BindableIntersect copy(RelTraitSet traitSet, List inputs, 
boolean all) {
+  return new BindableIntersect(getCluster(), traitSet, inputs, all);
+}
+
+public Class getElementType() {
+  return Object[].class;
+}
+
+public Enumerable bind(DataContext dataContext) {
+  return help(dataContext, this);
+}
+
+public Node implement(InterpreterImplementor implementor) {
+  return new SetOpNode(implementor.compiler, this);
+}
+  }
+
+  /** Implementation of {@link org.apache.calcite.rel.core.Minus} in
+   * bindable calling convention. */
+  public static class BindableMinus extends Minus implements BindableRel {
+public BindableMinus(RelOptCluster cluster, RelTraitSet traitSet,
+List inputs, boolean all) {
+  super(cluster, traitSet, inputs, all);
+}
+
+public BindableMinus copy(RelTraitSet traitSet, List inputs, 
boolean all) {
+  return new BindableMinus(getCluster(), traitSet, inputs, all);

Review comment:
   i think it is not , those Bindables should be return itself. Such as 
BindableMinus copy method return BindableMinus .





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:
us...@infra.apache.org

[GitHub] [calcite] xy2953396112 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

2020-04-24 Thread GitBox



xy2953396112 commented on a change in pull request #1737:
URL: https://github.com/apache/calcite/pull/1737#discussion_r414962981



##
File path: core/src/main/java/org/apache/calcite/interpreter/Bindables.java
##
@@ -545,6 +557,58 @@ public RelNode convert(RelNode rel) {
 }
   }
 
+  /**
+   * Rule to convert an {@link org.apache.calcite.rel.logical.LogicalIntersect}
+   * to a {@link BindableIntersect}.
+   */
+  public static class BindableIntersectRule extends ConverterRule {
+
+/**
+ * Creates a BindableIntersectRule.
+ *
+ * @param relBuilderFactory Builder for relational expressions
+ */
+public BindableIntersectRule(RelBuilderFactory relBuilderFactory) {
+  super(LogicalIntersect.class, (Predicate) r -> true,
+  Convention.NONE, BindableConvention.INSTANCE, relBuilderFactory,
+  "BindableIntersectRule");
+}
+
+public RelNode convert(RelNode rel) {
+  final LogicalIntersect intersect = (LogicalIntersect) rel;
+  final BindableConvention out = BindableConvention.INSTANCE;
+  final RelTraitSet traitSet = intersect.getTraitSet().replace(out);
+  return new BindableIntersect(rel.getCluster(), traitSet,
+  convertList(intersect.getInputs(), out), intersect.all);
+}
+  }
+
+  /**
+   * Rule to convert an {@link org.apache.calcite.rel.logical.LogicalMinus}
+   * to a {@link BindableMinus}.
+   */
+  public static class BindableMinusRule extends ConverterRule {

Review comment:
   yeah, will update it.





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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

2020-04-24 Thread GitBox



danny0405 commented on a change in pull request #1737:
URL: https://github.com/apache/calcite/pull/1737#discussion_r414957850



##
File path: core/src/main/java/org/apache/calcite/interpreter/Bindables.java
##
@@ -569,6 +633,56 @@ public Node implement(InterpreterImplementor implementor) {
 }
   }
 
+  /** Implementation of {@link org.apache.calcite.rel.core.Intersect} in
+   * bindable calling convention. */
+  public static class BindableIntersect extends Intersect implements 
BindableRel {
+public BindableIntersect(RelOptCluster cluster, RelTraitSet traitSet,
+List inputs, boolean all) {
+  super(cluster, traitSet, inputs, all);
+}
+
+public BindableIntersect copy(RelTraitSet traitSet, List inputs, 
boolean all) {
+  return new BindableIntersect(getCluster(), traitSet, inputs, all);
+}
+
+public Class getElementType() {
+  return Object[].class;
+}
+
+public Enumerable bind(DataContext dataContext) {
+  return help(dataContext, this);
+}
+
+public Node implement(InterpreterImplementor implementor) {
+  return new SetOpNode(implementor.compiler, this);
+}
+  }
+
+  /** Implementation of {@link org.apache.calcite.rel.core.Minus} in
+   * bindable calling convention. */
+  public static class BindableMinus extends Minus implements BindableRel {
+public BindableMinus(RelOptCluster cluster, RelTraitSet traitSet,
+List inputs, boolean all) {
+  super(cluster, traitSet, inputs, all);
+}
+
+public BindableMinus copy(RelTraitSet traitSet, List inputs, 
boolean all) {
+  return new BindableMinus(getCluster(), traitSet, inputs, all);

Review comment:
   These Bindables should have some common abstraction.





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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

2020-04-24 Thread GitBox



danny0405 commented on a change in pull request #1737:
URL: https://github.com/apache/calcite/pull/1737#discussion_r414957602



##
File path: core/src/main/java/org/apache/calcite/interpreter/Bindables.java
##
@@ -545,6 +557,58 @@ public RelNode convert(RelNode rel) {
 }
   }
 
+  /**
+   * Rule to convert an {@link org.apache.calcite.rel.logical.LogicalIntersect}
+   * to a {@link BindableIntersect}.
+   */
+  public static class BindableIntersectRule extends ConverterRule {
+
+/**
+ * Creates a BindableIntersectRule.
+ *
+ * @param relBuilderFactory Builder for relational expressions
+ */
+public BindableIntersectRule(RelBuilderFactory relBuilderFactory) {
+  super(LogicalIntersect.class, (Predicate) r -> true,
+  Convention.NONE, BindableConvention.INSTANCE, relBuilderFactory,
+  "BindableIntersectRule");
+}
+
+public RelNode convert(RelNode rel) {
+  final LogicalIntersect intersect = (LogicalIntersect) rel;
+  final BindableConvention out = BindableConvention.INSTANCE;
+  final RelTraitSet traitSet = intersect.getTraitSet().replace(out);
+  return new BindableIntersect(rel.getCluster(), traitSet,
+  convertList(intersect.getInputs(), out), intersect.all);
+}
+  }
+
+  /**
+   * Rule to convert an {@link org.apache.calcite.rel.logical.LogicalMinus}
+   * to a {@link BindableMinus}.
+   */
+  public static class BindableMinusRule extends ConverterRule {

Review comment:
   But at least the rule can be unified.





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:
us...@infra.apache.org

[calcite] branch master updated: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTableFunction

2020-04-24 Thread danny0405

This is an automated email from the ASF dual-hosted git repository.

danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new f62d6b9  [CALCITE-3955] Remove the first operand of RexCall from 
SqlWindowTableFunction
f62d6b9 is described below

commit f62d6b9a2f3dd877d4e2e5cbd2ff5d0d5f0efff1
Author: yuzhao.cyz 
AuthorDate: Fri Apr 24 17:42:42 2020 +0800

[CALCITE-3955] Remove the first operand of RexCall from 
SqlWindowTableFunction

In CALCITE-3382, we introduced TUMBLE window function to replace the
deprecated group tumble window.

But for query

```sql
select *
from table(tumble(table Shipments, descriptor(rowtime),
  INTERVAL '1' MINUTE))
```
the output plan is

```xml
LogicalProject
  LogicalTableFunctionScan(invocation=[TUMBLE($1, ...)], rowType=...)
  LogicalProject
LogicalTableScan
```
The first operand of TUMBLE rex call is always the last
input field, but actually it represents the source table
which is the input rel node.

Removes the first operand from the RexCall because
it is useless and confusing.
---
 .../apache/calcite/adapter/enumerable/RexImpTable.java  |  6 --
 .../apache/calcite/sql2rel/StandardConvertletTable.java | 17 +
 .../org/apache/calcite/test/SqlToRelConverterTest.java  |  4 
 .../org/apache/calcite/test/SqlToRelConverterTest.xml   |  4 ++--
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index d095c79..669c316 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -3129,8 +3129,10 @@ public class RexImpTable {
 @Override public Expression implement(RexToLixTranslator translator,
 Expression inputEnumerable,
 RexCall call, PhysType inputPhysType, PhysType outputPhysType) {
-  Expression intervalExpression = 
translator.translate(call.getOperands().get(2));
-  RexCall descriptor = (RexCall) call.getOperands().get(1);
+  // The table operand is removed from the RexCall because it
+  // represents the input, see 
StandardConvertletTable#convertWindowFunction.
+  Expression intervalExpression = 
translator.translate(call.getOperands().get(1));
+  RexCall descriptor = (RexCall) call.getOperands().get(0);
   List translatedOperands = new ArrayList<>();
   final ParameterExpression parameter =
   Expressions.parameter(Primitive.box(inputPhysType.getJavaRowType()), 
"_input");
diff --git 
a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java 
b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
index 245320b..747ac8e 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
@@ -46,6 +46,7 @@ import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlNumericLiteral;
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlUtil;
+import org.apache.calcite.sql.SqlWindowTableFunction;
 import org.apache.calcite.sql.fun.SqlArrayValueConstructor;
 import org.apache.calcite.sql.fun.SqlBetweenOperator;
 import org.apache.calcite.sql.fun.SqlCase;
@@ -663,6 +664,22 @@ public class StandardConvertletTable extends 
ReflectiveConvertletTable {
 return cx.getRexBuilder().makeCall(returnType, fun, exprs);
   }
 
+  public RexNode convertWindowFunction(
+  SqlRexContext cx,
+  SqlWindowTableFunction fun,
+  SqlCall call) {
+// The first operand of window function is actually a query, skip that.
+final List operands = Util.skip(call.getOperandList(), 1);
+final List exprs = convertExpressionList(cx, operands,
+SqlOperandTypeChecker.Consistency.NONE);
+RelDataType returnType =
+cx.getValidator().getValidatedNodeTypeIfKnown(call);
+if (returnType == null) {
+  returnType = cx.getRexBuilder().deriveReturnType(fun, exprs);
+}
+return cx.getRexBuilder().makeCall(returnType, fun, exprs);
+  }
+
   public RexNode convertSequenceValue(
   SqlRexContext cx,
   SqlSequenceValueOperator fun,
diff --git 
a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java 
b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 36c2914..338b99e 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -1776,16 +1776,12 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
 sql(sql).ok();
   }
 
-  // In generated

[GitHub] [calcite] danny0405 commented on pull request #1942: [CALCITE-3954] Always compare types using equals

2020-04-24 Thread GitBox



danny0405 commented on pull request #1942:
URL: https://github.com/apache/calcite/pull/1942#issuecomment-619303422


   > You must also change RelDataTypeImpl.equals.
   
   Yep, thanks for the reminder ~



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:
us...@infra.apache.org

[calcite] branch master updated: Revert "[CALCITE-3947] AbstractRelOptPlanner.classes should be LinkedHashSet so that rule match order is deterministic across runs"

2020-04-24 Thread hyuan

This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new af8c8ef  Revert "[CALCITE-3947] AbstractRelOptPlanner.classes should 
be LinkedHashSet so that rule match order is deterministic across runs"
af8c8ef is described below

commit af8c8efdc465435fa095197ff8365d527147ee7a
Author: Haisheng Yuan 
AuthorDate: Fri Apr 24 20:32:19 2020 -0500

Revert "[CALCITE-3947] AbstractRelOptPlanner.classes should be 
LinkedHashSet so that rule match order is deterministic across runs"

This reverts commit 53bbd2daff8104f3a1113eaaa01bf58ca3361beb.
---
 core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java 
b/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
index 85b060f..222fcbb 100644
--- a/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
@@ -38,7 +38,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -75,7 +74,7 @@ public abstract class AbstractRelOptPlanner implements 
RelOptPlanner {
 
   protected final AtomicBoolean cancelFlag;
 
-  private final Set> classes = new LinkedHashSet<>();
+  private final Set> classes = new HashSet<>();
 
   private final Set conventions = new HashSet<>();

[GitHub] [calcite] amaliujia commented on pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



amaliujia commented on pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#issuecomment-619180990


   Thanks for the PR. Have LGTMed it. 
   
   I will rebase and make changes in my PR accordingly after #1943 is merged.



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:
us...@infra.apache.org

[GitHub] [calcite] amaliujia commented on pull request #1943: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTabl…

2020-04-24 Thread GitBox



amaliujia commented on pull request #1943:
URL: https://github.com/apache/calcite/pull/1943#issuecomment-619180717


   LGTM
   
   The change in this PR makes sense to me.



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:
us...@infra.apache.org

[GitHub] [calcite] liyafan82 opened a new pull request #1944: [CALCITE-3956] Unify comparison logic for RelOptCost

2020-04-24 Thread GitBox



liyafan82 opened a new pull request #1944:
URL: https://github.com/apache/calcite/pull/1944


   Currently, comparisons between RelOptCost objects are based on 3 methods:
   1. ``boolean isLe(RelOptCost cost)``
   2. ``boolean isLt(RelOptCost cost)``
   3. ``boolean equals(RelOptCost cost)``
   
   The 3 methods used in combination determine the relation between RelOptCost 
objects.
   
   There are some problems with this implementation:
   1. Some logic is duplicate in the above methods, making it difficult to 
maintain.
   2. To determine the relation between RelOptCost objects, we often need to 
call more than one comparison methods, leading to performance overhead.
   3. Since the logic is spread in multiple methods, it is easy to end up with 
contradictive comparison logic, which will suprise the users. For example, the 
following assertion should hold according to common sense:
   
   ``if a >=b, then we have a > b or a == b``
   
   However, with the current implementation of VolcanoCost, we can easily 
create instances that violate the above assertion.
   
   To solve the problems, we want to make RelOptCost extends the 
Comparable, so the comparison logic is unified in the compareTo 
method, which solves the above problems.



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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 commented on pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



danny0405 commented on pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#issuecomment-618917599


   I have fire a fix for the mis-leading first operand of `TUMBLE` `RexCall`, 
please review if you have time https://github.com/apache/calcite/pull/1943/files



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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 opened a new pull request #1943: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTabl…

2020-04-24 Thread GitBox



danny0405 opened a new pull request #1943:
URL: https://github.com/apache/calcite/pull/1943


   …eFunction
   
   In CALCITE-3382, we introduced TUMBLE window function to replace the
   deprecated group tumble window.
   
   But for query
   
   ```sql
   select *
   from table(tumble(table Shipments, descriptor(rowtime), INTERVAL '1'
   MINUTE))
   ```
   the output plan is
   
   ```xml
   LogicalProject(ORDERID=[$0], ROWTIME=[$1], window_start=[$2],
   window_end=[$3])
 LogicalTableFunctionScan(invocation=[TUMBLE($1, DESCRIPTOR($1),
 6:INTERVAL MINUTE)], rowType=[RecordType(INTEGER ORDERID,
 TIMESTAMP(0) ROWTIME, TIMESTAMP(0) window_start, TIMESTAMP(0)
 window_end)])
 LogicalProject(ORDERID=[$0], ROWTIME=[$1])
   LogicalTableScan(table=[[CATALOG, SALES, SHIPMENTS]])
   ```
   The first operand of TUMBLE rex call is always the last
   input field, but actually it represents the source table
   which is the input rel node.
   
   Removes the first operand from the RexCall because
   it is useless and confusing.



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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



danny0405 commented on a change in pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#discussion_r414393255



##
File path: 
core/src/main/java/org/apache/calcite/sql/SqlSessionTableFunction.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+
+/**
+ * SqlSessionTableFunction implements an operator for per-key sessionization. 
It allows
+ * four parameters:
+ * 1. a table.
+ * 2. a descriptor to provide a watermarked column name from the input table.
+ * 3. a descriptor to provide a column as key, on which sessionization will be 
applied.
+ * 4. an interval parameter to specify a inactive activity gap to break 
sessions.
+ */
+public class SqlSessionTableFunction extends SqlWindowTableFunction {
+  public SqlSessionTableFunction() {
+super(SqlKind.SESSION.name());
+  }
+
+  @Override public SqlOperandCountRange getOperandCountRange() {
+return SqlOperandCountRanges.of(4);
+  }
+
+  @Override public boolean checkOperandTypes(SqlCallBinding callBinding,
+  boolean throwOnFailure) {
+final SqlNode operand0 = callBinding.operand(0);
+final SqlValidator validator = callBinding.getValidator();
+final RelDataType type = validator.getValidatedNodeType(operand0);
+if (type.getSqlTypeName() != SqlTypeName.ROW) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+final SqlNode operand1 = callBinding.operand(1);
+if (operand1.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand1).getOperandList());
+final SqlNode operand2 = callBinding.operand(2);
+if (operand2.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand2).getOperandList());
+final RelDataType type3 = 
validator.getValidatedNodeType(callBinding.operand(3));
+if (!SqlTypeUtil.isInterval(type3)) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+return true;
+  }
+
+  @Override public String getAllowedSignatures(String opNameToUse) {
+return getName() + "(TABLE table_name, DESCRIPTOR(col), "
++ "DESCRIPTOR(col), datetime interval)";

Review comment:
   Thanks for the explanations, personally i think one `DESCRIPTOR` is more 
concise, i'm wondering what other people think ?





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:
us...@infra.apache.org

[GitHub] [calcite] danny0405 opened a new pull request #1942: [CALCITE-3954] Always compare types using equals

2020-04-24 Thread GitBox



danny0405 opened a new pull request #1942:
URL: https://github.com/apache/calcite/pull/1942


   Current interning of types use weak references.
   Change from MUST intern to SHOULD intern, and always
   compare types using equals.
   
   We clearly want to do some interning, especially within a query, so that
   there aren't hundreds of copies of the same record type all over the
   place. But if people don't intern, or intern in different query-specific
   caches, then the logic will still work.
   
   If equals is written using the standard template
   ```java
   return this == o
 || o instanceof TheType && field1 == o.field1 && field2 == o.field2
   ```
   (that is, avoiding deep comparison if possible) then the performance
   will be pretty much the same.



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:
us...@infra.apache.org

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



amaliujia commented on a change in pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#discussion_r414350668



##
File path: 
core/src/main/java/org/apache/calcite/sql/SqlSessionTableFunction.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+
+/**
+ * SqlSessionTableFunction implements an operator for per-key sessionization. 
It allows
+ * four parameters:
+ * 1. a table.
+ * 2. a descriptor to provide a watermarked column name from the input table.
+ * 3. a descriptor to provide a column as key, on which sessionization will be 
applied.
+ * 4. an interval parameter to specify a inactive activity gap to break 
sessions.
+ */
+public class SqlSessionTableFunction extends SqlWindowTableFunction {
+  public SqlSessionTableFunction() {
+super(SqlKind.SESSION.name());
+  }
+
+  @Override public SqlOperandCountRange getOperandCountRange() {
+return SqlOperandCountRanges.of(4);
+  }
+
+  @Override public boolean checkOperandTypes(SqlCallBinding callBinding,
+  boolean throwOnFailure) {
+final SqlNode operand0 = callBinding.operand(0);
+final SqlValidator validator = callBinding.getValidator();
+final RelDataType type = validator.getValidatedNodeType(operand0);
+if (type.getSqlTypeName() != SqlTypeName.ROW) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+final SqlNode operand1 = callBinding.operand(1);
+if (operand1.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand1).getOperandList());
+final SqlNode operand2 = callBinding.operand(2);
+if (operand2.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand2).getOperandList());
+final RelDataType type3 = 
validator.getValidatedNodeType(callBinding.operand(3));
+if (!SqlTypeUtil.isInterval(type3)) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+return true;
+  }
+
+  @Override public String getAllowedSignatures(String opNameToUse) {
+return getName() + "(TABLE table_name, DESCRIPTOR(col), "
++ "DESCRIPTOR(col), datetime interval)";

Review comment:
   In fact, `DESCRIPTOR` is a part of  `SQL standard 2016: Polymorphic 
table functions` (https://issues.apache.org/jira/browse/CALCITE-2270). 
`DESCRIPTOR` is designed to specify columns explicitly in table functions. That 
was one of the purpose  it was created for. 
   
   
   You can find a free copy of `SQL standard 2016:  Polymorphic table 
functions` in this link: https://modern-sql.com/standard





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:
us...@infra.apache.org

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



amaliujia commented on a change in pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#discussion_r414350668



##
File path: 
core/src/main/java/org/apache/calcite/sql/SqlSessionTableFunction.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+
+/**
+ * SqlSessionTableFunction implements an operator for per-key sessionization. 
It allows
+ * four parameters:
+ * 1. a table.
+ * 2. a descriptor to provide a watermarked column name from the input table.
+ * 3. a descriptor to provide a column as key, on which sessionization will be 
applied.
+ * 4. an interval parameter to specify a inactive activity gap to break 
sessions.
+ */
+public class SqlSessionTableFunction extends SqlWindowTableFunction {
+  public SqlSessionTableFunction() {
+super(SqlKind.SESSION.name());
+  }
+
+  @Override public SqlOperandCountRange getOperandCountRange() {
+return SqlOperandCountRanges.of(4);
+  }
+
+  @Override public boolean checkOperandTypes(SqlCallBinding callBinding,
+  boolean throwOnFailure) {
+final SqlNode operand0 = callBinding.operand(0);
+final SqlValidator validator = callBinding.getValidator();
+final RelDataType type = validator.getValidatedNodeType(operand0);
+if (type.getSqlTypeName() != SqlTypeName.ROW) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+final SqlNode operand1 = callBinding.operand(1);
+if (operand1.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand1).getOperandList());
+final SqlNode operand2 = callBinding.operand(2);
+if (operand2.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand2).getOperandList());
+final RelDataType type3 = 
validator.getValidatedNodeType(callBinding.operand(3));
+if (!SqlTypeUtil.isInterval(type3)) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+return true;
+  }
+
+  @Override public String getAllowedSignatures(String opNameToUse) {
+return getName() + "(TABLE table_name, DESCRIPTOR(col), "
++ "DESCRIPTOR(col), datetime interval)";

Review comment:
   In fact, it is a feature defined in SQL standard 2016: Polymorphic table 
functions(https://issues.apache.org/jira/browse/CALCITE-2270). `DESCRIPTOR` is 
designed to specify columns explicitly in table functions. That was one of the 
purpose  it was created for. 
   
   
   You can find a free copy of `SQL standard 2016:  Polymorphic table 
functions` in this link: https://modern-sql.com/standard





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:
us...@infra.apache.org

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

2020-04-24 Thread GitBox



amaliujia commented on a change in pull request #1761:
URL: https://github.com/apache/calcite/pull/1761#discussion_r414350668



##
File path: 
core/src/main/java/org/apache/calcite/sql/SqlSessionTableFunction.java
##
@@ -0,0 +1,71 @@
+/*
+ * 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.sql;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+
+/**
+ * SqlSessionTableFunction implements an operator for per-key sessionization. 
It allows
+ * four parameters:
+ * 1. a table.
+ * 2. a descriptor to provide a watermarked column name from the input table.
+ * 3. a descriptor to provide a column as key, on which sessionization will be 
applied.
+ * 4. an interval parameter to specify a inactive activity gap to break 
sessions.
+ */
+public class SqlSessionTableFunction extends SqlWindowTableFunction {
+  public SqlSessionTableFunction() {
+super(SqlKind.SESSION.name());
+  }
+
+  @Override public SqlOperandCountRange getOperandCountRange() {
+return SqlOperandCountRanges.of(4);
+  }
+
+  @Override public boolean checkOperandTypes(SqlCallBinding callBinding,
+  boolean throwOnFailure) {
+final SqlNode operand0 = callBinding.operand(0);
+final SqlValidator validator = callBinding.getValidator();
+final RelDataType type = validator.getValidatedNodeType(operand0);
+if (type.getSqlTypeName() != SqlTypeName.ROW) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+final SqlNode operand1 = callBinding.operand(1);
+if (operand1.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand1).getOperandList());
+final SqlNode operand2 = callBinding.operand(2);
+if (operand2.getKind() != SqlKind.DESCRIPTOR) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+validateColumnNames(validator, type.getFieldNames(), ((SqlCall) 
operand2).getOperandList());
+final RelDataType type3 = 
validator.getValidatedNodeType(callBinding.operand(3));
+if (!SqlTypeUtil.isInterval(type3)) {
+  return throwValidationSignatureErrorOrReturnFalse(callBinding, 
throwOnFailure);
+}
+return true;
+  }
+
+  @Override public String getAllowedSignatures(String opNameToUse) {
+return getName() + "(TABLE table_name, DESCRIPTOR(col), "
++ "DESCRIPTOR(col), datetime interval)";

Review comment:
   In fact, it is a feature defined in SQL standard 2016: Polymorphic table 
functions(https://issues.apache.org/jira/browse/CALCITE-2270). `DESCRIPTOR` is 
designed to specify columns explicitly in table functions. That was one of the 
purpose  it was created for. 
   





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:
us...@infra.apache.org

[GitHub] [calcite] xy2953396112 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

[GitHub] [calcite] xy2953396112 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

[GitHub] [calcite] danny0405 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

[GitHub] [calcite] danny0405 commented on a change in pull request #1737: [CALCITE-3718] Support Intersect and Minus in Bindables

[calcite] branch master updated: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTableFunction

[GitHub] [calcite] danny0405 commented on pull request #1942: [CALCITE-3954] Always compare types using equals

[calcite] branch master updated: Revert "[CALCITE-3947] AbstractRelOptPlanner.classes should be LinkedHashSet so that rule match order is deterministic across runs"

[GitHub] [calcite] amaliujia commented on pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

[GitHub] [calcite] amaliujia commented on pull request #1943: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTabl…

[GitHub] [calcite] liyafan82 opened a new pull request #1944: [CALCITE-3956] Unify comparison logic for RelOptCost

[GitHub] [calcite] danny0405 commented on pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

[GitHub] [calcite] danny0405 opened a new pull request #1943: [CALCITE-3955] Remove the first operand of RexCall from SqlWindowTabl…

[GitHub] [calcite] danny0405 commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

[GitHub] [calcite] danny0405 opened a new pull request #1942: [CALCITE-3954] Always compare types using equals

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

[GitHub] [calcite] amaliujia commented on a change in pull request #1761: [CALCITE-3737][CALCITE-3780] Implement HOP and SESSION table functions

17 matches

Site Navigation

Mail list logo

Footer information