[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-25 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r317439450
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java
 ##
 @@ -132,11 +132,19 @@ RelDataType deriveCovarType(RelDataTypeFactory 
typeFactory,
* @return the result type for a decimal addition.
*/
   default RelDataType deriveDecimalPlusType(RelDataTypeFactory typeFactory,
-RelDataType type1, RelDataType 
type2) {
+  RelDataType type1, RelDataType type2) {
 if (SqlTypeUtil.isExactNumeric(type1)
 && SqlTypeUtil.isExactNumeric(type2)) {
   if (SqlTypeUtil.isDecimal(type1)
   || SqlTypeUtil.isDecimal(type2)) {
+// Java numeric will always have invalid precision/scale,
+// use its default decimal precision/scale instead.
+type1 = RelDataTypeFactoryImpl.isJavaType(type1)
 
 Review comment:
   Sure, let me add them in, i believe this patch would be merged soon.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-21 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r316018431
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/type/CompositeOperandTypeChecker.java
 ##
 @@ -311,6 +324,22 @@ private boolean check(SqlCallBinding callBinding) {
   throw new AssertionError();
 }
   }
+
+  private boolean checkWithOutTypeCoercion(SqlCallBinding callBinding) {
+if (!callBinding.getValidator().isTypeCoercionEnabled()) {
+  return false;
+}
+for (Ord ord : 
Ord.zip(allowedRules)) {
 
 Review comment:
   Thanks Julian, method name renamed to `checkWithoutTypeCoercion`.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-16 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r314599694
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
 ##
 @@ -0,0 +1,647 @@
+/*
+ * 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.validate.implicit;
+
+import org.apache.calcite.rel.type.DynamicRecordType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCollation;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.type.SqlTypeAssignmentRules;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorNamespace;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.util.Pair;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * Base class for all the type coercion rules. If you want to have a custom 
type coercion rules,
+ * inheriting this class is not necessary, but would have some convenient tool 
methods.
+ *
+ * We make tool methods: {@link #coerceIthOperandTo}, {@link 
#coerceIthColumnTo},
+ * {@link #needToCast}, {@link #updateInferredRowType},
+ * {@link #updateInferredTypeFor}, {@link #updateInferredRowType}
+ * all overridable by derived classes, you can define system specific type 
coercion logic.
+ *
+ * Caution that these methods may modify the {@link SqlNode} tree, you 
should know what the
+ * effect is when using these methods to customize your type coercion 
rules.
+ *
+ * This class also defines the default implementation of the type widening 
strategies, see
+ * {@link TypeCoercion} doc and methods: {@link #getTightestCommonType}, 
{@link #getWiderTypeFor},
+ * {@link #getWiderTypeForTwo}, {@link #getWiderTypeForDecimal},
+ * {@link #commonTypeForBinaryComparison} for the detail strategies.
+ */
+public abstract class AbstractTypeCoercion implements TypeCoercion {
+  protected SqlValidator validator;
+  protected RelDataTypeFactory factory;
+
+  //~ Constructors ---
+
+  AbstractTypeCoercion(SqlValidator validator) {
+Objects.requireNonNull(validator);
+this.validator = validator;
+this.factory = validator.getTypeFactory();
+  }
+
+  //~ Methods 
+
+  public RelDataTypeFactory getFactory() {
+return this.factory;
+  }
+
+  public SqlValidator getValidator() {
+return this.validator;
+  }
+
+  /**
+   * Cast ith operand to target type, we do this base on the fact that
+   * validate happens before type coercion.
+   */
+  protected boolean coerceIthOperandTo(
 
 Review comment:
   Thanks, do you have any suggested name ? 


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-16 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r314590031
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
 ##
 @@ -2952,7 +2952,7 @@ private void 
checkReduceNullableToNotNull(ReduceExpressionsRule rule) {
 // in addition to the casts.
 checkPlanning(program,
 "select * from emp "
-+ "where cast((empno + (10/2)) as int) = 13");
++ "where cast((empno + (10/2)) as double) = 13");
 
 Review comment:
   This behavior is very relative with the SQL dialect, except for `POSTGRE 
SQL`, almost all the other sql engines would produce a `DECIMAL` or `DOUBLE` 
for 2 integers divide, i.3. `2/4` returns o.5 instead of 0.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-16 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r314587914
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlKind.java
 ##
 @@ -1279,6 +1279,46 @@
   LESS_THAN, GREATER_THAN,
   GREATER_THAN_OR_EQUAL, LESS_THAN_OR_EQUAL);
 
+  /**
+   * Category of binary arithmetic.
+   *
+   * Consists of:
+   * {@link #PLUS}
+   * {@link #MINUS}
+   * {@link #TIMES}
+   * {@link #DIVIDE}
+   * {@link #MOD}.
+   */
+  public static final Set BINARY_ARITHMETIC =
+  EnumSet.of(PLUS, MINUS, TIMES, DIVIDE, MOD);
+
+  /**
+   * Category of binary equality.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   */
+  public static final Set BINARY_EQUALITY =
+  EnumSet.of(EQUALS, NOT_EQUALS);
+
+  /**
+   * Category of binary comparison.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   * {@link #GREATER_THAN}
+   * {@link #GREATER_THAN_OR_EQUAL}
+   * {@link #LESS_THAN}
+   * {@link #LESS_THAN_OR_EQUAL}
+   */
+  public static final Set BINARY_COMPARISON =
+  EnumSet.of(
+  EQUALS, NOT_EQUALS,
+  GREATER_THAN, GREATER_THAN_OR_EQUAL,
+  LESS_THAN, LESS_THAN_OR_EQUAL);
 
 Review comment:
   Finally i decide to support is not distinct from, thanks, because i tried `2 
is distinct from '2'` in POSTGRE SQL, and it returns false.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-14 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r313799396
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlKind.java
 ##
 @@ -1279,6 +1279,46 @@
   LESS_THAN, GREATER_THAN,
   GREATER_THAN_OR_EQUAL, LESS_THAN_OR_EQUAL);
 
+  /**
+   * Category of binary arithmetic.
+   *
+   * Consists of:
+   * {@link #PLUS}
+   * {@link #MINUS}
+   * {@link #TIMES}
+   * {@link #DIVIDE}
+   * {@link #MOD}.
+   */
+  public static final Set BINARY_ARITHMETIC =
+  EnumSet.of(PLUS, MINUS, TIMES, DIVIDE, MOD);
+
+  /**
+   * Category of binary equality.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   */
+  public static final Set BINARY_EQUALITY =
+  EnumSet.of(EQUALS, NOT_EQUALS);
+
+  /**
+   * Category of binary comparison.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   * {@link #GREATER_THAN}
+   * {@link #GREATER_THAN_OR_EQUAL}
+   * {@link #LESS_THAN}
+   * {@link #LESS_THAN_OR_EQUAL}
+   */
+  public static final Set BINARY_COMPARISON =
+  EnumSet.of(
+  EQUALS, NOT_EQUALS,
+  GREATER_THAN, GREATER_THAN_OR_EQUAL,
+  LESS_THAN, LESS_THAN_OR_EQUAL);
 
 Review comment:
   I found that only DB2 and PostgreSQL support "is not distinct from", while 
PostgreSQL does not support implicit type cast, to keep rigorous, let's give up 
the support for it in this patch.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424393
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlOperator.java
 ##
 @@ -510,7 +510,7 @@ public RelDataType deriveType(
 final SqlOperator sqlOperator =
 SqlUtil.lookupRoutine(validator.getOperatorTable(), getNameAsId(),
 argTypes, null, null, getSyntax(), getKind(),
-validator.getCatalogReader().nameMatcher());
+validator.getCatalogReader().nameMatcher(), false);
 
 Review comment:
   Always disable type coercion for builtin operators operands, they are 
handled by the `TypeCoercion` specifically.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424393
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlOperator.java
 ##
 @@ -510,7 +510,7 @@ public RelDataType deriveType(
 final SqlOperator sqlOperator =
 SqlUtil.lookupRoutine(validator.getOperatorTable(), getNameAsId(),
 argTypes, null, null, getSyntax(), getKind(),
-validator.getCatalogReader().nameMatcher());
+validator.getCatalogReader().nameMatcher(), false);
 
 Review comment:
   Always disable type coercion for builtin operators, they are handled by the 
`TypeCoercion` specifically.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310426356
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
 ##
 @@ -310,6 +310,16 @@ RelDataType createDecimalQuotient(
   RelDataType type1,
   RelDataType type2);
 
+  /**
+   * Create a decimal type equivalent to the numeric {@code type},
+   * this is somehow related to specific system implementation,
 
 Review comment:
   removed


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310426281
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java
 ##
 @@ -0,0 +1,647 @@
+/*
+ * 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.validate.implicit;
+
+import org.apache.calcite.rel.type.DynamicRecordType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCollation;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.type.SqlTypeAssignmentRules;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorNamespace;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.util.Pair;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * Base class for all the type coercion rules. If you want to have a custom 
type coercion rules,
+ * inheriting this class is not necessary, but would have some convenient tool 
methods.
+ *
+ * We make tool methods: {@link #coerceIthOperandTo}, {@link 
#coerceIthColumnTo},
+ * {@link #needToCast}, {@link #updateInferredRowType},
+ * {@link #updateInferredTypeFor}, {@link #updateInferredRowType}
+ * all overridable by derived classes, you can define system specific type 
coercion logic.
+ *
+ * Caution that these methods may modify the {@link SqlNode} tree, you 
should know what the
+ * effect is when using these methods to customize your type coercion 
rules.
+ *
+ * This class also defines the default implementation of the type widening 
strategies, see
+ * {@link TypeCoercion} doc and methods: {@link #getTightestCommonType}, 
{@link #getWiderTypeFor},
+ * {@link #getWiderTypeForTwo}, {@link #getWiderTypeForDecimal},
+ * {@link #commonTypeForBinaryComparison} for the detail strategies.
+ */
+public abstract class AbstractTypeCoercion implements TypeCoercion {
+  protected SqlValidator validator;
+  protected RelDataTypeFactory factory;
+
+  //~ Constructors ---
+
+  AbstractTypeCoercion(SqlValidator validator) {
+Objects.requireNonNull(validator);
+this.validator = validator;
+this.factory = validator.getTypeFactory();
+  }
+
+  //~ Methods 
+
+  public RelDataTypeFactory getFactory() {
+return this.factory;
+  }
+
+  public SqlValidator getValidator() {
+return this.validator;
+  }
+
+  /**
+   * Cast ith operand to target type, we do this base on the fact that
+   * validate happens before type coercion.
+   */
+  protected boolean coerceIthOperandTo(
+  SqlValidatorScope scope,
+  SqlCall call,
+  int index,
+  RelDataType targetType) {
+SqlNode operand = call.getOperandList().get(index);
+// Check it early.
+if (!needToCast(scope, operand, targetType)) {
+  return false;
+}
+// fix nullable attr.
+RelDataType targetType1 = syncAttributes(validator.deriveType(scope, 
operand), targetType);
+SqlNode desired = castTo(operand, targetType1);
+call.setOperand(index, desired);
+updateInferredTypeFor(desired, targetType1);
+return true;
+  }
+
+
+  /**
+   * Cast ith column to target type.
+   *
+   * @param scope  validator scope for the node list
+   * @param nodeList   column node list
+   * @param index  index of column
+   * 

[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310426132
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/validate/SqlValidator.java
 ##
 @@ -768,6 +769,27 @@ boolean validateModality(SqlSelect select, SqlModality 
modality,
   void validateSequenceValue(SqlValidatorScope scope, SqlIdentifier id);
 
   SqlValidatorScope getWithScope(SqlNode withItem);
+
+  /**
+   * Set if implicit type coercion is allowed when the validator does 
validation.
+   * See {@link org.apache.calcite.sql.validate.implicit.TypeCoercionImpl} for 
the details.
+   * @param enabled default as true.
+   */
+  SqlValidator setEnableTypeCoercion(boolean enabled);
+
+  /** Get if this validator supports implicit type coercion. */
+  boolean isEnableTypeCoercion();
 
 Review comment:
   Renamed, thanks.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310425911
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlKind.java
 ##
 @@ -1279,6 +1279,46 @@
   LESS_THAN, GREATER_THAN,
   GREATER_THAN_OR_EQUAL, LESS_THAN_OR_EQUAL);
 
+  /**
+   * Category of binary arithmetic.
+   *
+   * Consists of:
+   * {@link #PLUS}
+   * {@link #MINUS}
+   * {@link #TIMES}
+   * {@link #DIVIDE}
+   * {@link #MOD}.
+   */
+  public static final Set BINARY_ARITHMETIC =
+  EnumSet.of(PLUS, MINUS, TIMES, DIVIDE, MOD);
+
+  /**
+   * Category of binary equality.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   */
+  public static final Set BINARY_EQUALITY =
+  EnumSet.of(EQUALS, NOT_EQUALS);
+
+  /**
+   * Category of binary comparison.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   * {@link #GREATER_THAN}
+   * {@link #GREATER_THAN_OR_EQUAL}
+   * {@link #LESS_THAN}
+   * {@link #LESS_THAN_OR_EQUAL}
+   */
+  public static final Set BINARY_COMPARISON =
+  EnumSet.of(
+  EQUALS, NOT_EQUALS,
+  GREATER_THAN, GREATER_THAN_OR_EQUAL,
+  LESS_THAN, LESS_THAN_OR_EQUAL);
 
 Review comment:
   But i need to make sure if the implicit type coercion behaviors for them are 
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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310425531
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlKind.java
 ##
 @@ -1279,6 +1279,46 @@
   LESS_THAN, GREATER_THAN,
   GREATER_THAN_OR_EQUAL, LESS_THAN_OR_EQUAL);
 
+  /**
+   * Category of binary arithmetic.
+   *
+   * Consists of:
+   * {@link #PLUS}
+   * {@link #MINUS}
+   * {@link #TIMES}
+   * {@link #DIVIDE}
+   * {@link #MOD}.
+   */
+  public static final Set BINARY_ARITHMETIC =
+  EnumSet.of(PLUS, MINUS, TIMES, DIVIDE, MOD);
+
+  /**
+   * Category of binary equality.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   */
+  public static final Set BINARY_EQUALITY =
+  EnumSet.of(EQUALS, NOT_EQUALS);
+
+  /**
+   * Category of binary comparison.
+   *
+   * Consists of:
+   * {@link #EQUALS}
+   * {@link #NOT_EQUALS}
+   * {@link #GREATER_THAN}
+   * {@link #GREATER_THAN_OR_EQUAL}
+   * {@link #LESS_THAN}
+   * {@link #LESS_THAN_OR_EQUAL}
+   */
+  public static final Set BINARY_COMPARISON =
+  EnumSet.of(
+  EQUALS, NOT_EQUALS,
+  GREATER_THAN, GREATER_THAN_OR_EQUAL,
+  LESS_THAN, LESS_THAN_OR_EQUAL);
 
 Review comment:
   I think we can, the only difference for `is distinct from` with `<>` and `=` 
is the behaviors of null values.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310425057
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
 ##
 @@ -310,6 +310,16 @@ RelDataType createDecimalQuotient(
   RelDataType type1,
   RelDataType type2);
 
+  /**
+   * Create a decimal type equivalent to the numeric {@code type},
+   * this is somehow related to specific system implementation,
 
 Review comment:
   Removed, thanks.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424936
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
 ##
 @@ -2952,7 +2952,7 @@ private void 
checkReduceNullableToNotNull(ReduceExpressionsRule rule) {
 // in addition to the casts.
 checkPlanning(program,
 "select * from emp "
-+ "where cast((empno + (10/2)) as int) = 13");
++ "where cast((empno + (10/2)) as double) = 13");
 
 Review comment:
   13 is int, but with implicit coercion, the `/` operator always returns 
double now.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424833
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/fun/SqlCaseOperator.java
 ##
 @@ -247,7 +248,22 @@ private RelDataType inferTypeFromValidator(
 
 RelDataType ret = callBinding.getTypeFactory().leastRestrictive(argTypes);
 if (null == ret) {
-  throw callBinding.newValidationError(RESOURCE.illegalMixingOfTypes());
+  boolean changed = false;
 
 Review comment:
   or coerced ?


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424393
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlOperator.java
 ##
 @@ -510,7 +510,7 @@ public RelDataType deriveType(
 final SqlOperator sqlOperator =
 SqlUtil.lookupRoutine(validator.getOperatorTable(), getNameAsId(),
 argTypes, null, null, getSyntax(), getKind(),
-validator.getCatalogReader().nameMatcher());
+validator.getCatalogReader().nameMatcher(), false);
 
 Review comment:
   A better way should be the flag of whether validator allows the type 
coercion.


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


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit type cast support

2019-08-04 Thread GitBox
danny0405 commented on a change in pull request #706: [CALCITE-2302] Implicit 
type cast support
URL: https://github.com/apache/calcite/pull/706#discussion_r310424123
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
 ##
 @@ -287,6 +287,25 @@ public static boolean isDatetime(RelDataType type) {
 return SqlTypeFamily.DATETIME.contains(type);
   }
 
+  /**
+   * @return true if type is DATE
+   */
+  public static boolean isDate(RelDataType type) {
+SqlTypeName typeName = type.getSqlTypeName();
+if (typeName == null) {
+  return false;
+}
+
+return type.getSqlTypeName() == SqlTypeName.DATE;
+  }
+
+  /**
+   * @return true if type is TIMESTAMP
+   */
+  public static boolean isTimestamp(RelDataType type) {
+return SqlTypeFamily.TIMESTAMP.contains(type);
 
 Review comment:
   Yes, currently i ignore the null check here.


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


With regards,
Apache Git Services