You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "normanj-bitquill (via GitHub)" <gi...@apache.org> on 2024/08/30 18:35:45 UTC

[PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

normanj-bitquill opened a new pull request, #3942:
URL: https://github.com/apache/calcite/pull/3942

   * All three functions accept either a pair of Integer or Binary values as arguments
   * Binary arguments must be of the same length
   * Returns NULL if any argument is NULL
   * Returns a value of the same type as the first argument if both arguments are not null


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #3942:
URL: https://github.com/apache/calcite/pull/3942


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1742792245


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -2829,6 +2829,20 @@ public static long bitAnd(long b0, long b1) {
     return b0 & b1;
   }
 
+  /** Bitwise function <code>BITAND</code> applied to a Long and int value.
+   *  Needed for handling NULL for the first argument.
+   */
+  public static long bitAnd(Long b0, int b1) {
+    return b0 & b1;
+  }
+
+  /** Bitwise function <code>BITAND</code> applied to a Long and int value.

Review Comment:
   Added some more tests with different integer types for the arguments (integer and bigint).
   
   Needed to rework the return type detection. It will now use the largest integer type if both arguments are integer types. There may be ways of cleaning the code up further for the return type inference.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1750717382


##########
core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties:
##########
@@ -207,6 +207,7 @@ ArgumentMustBeNumericLiteralInRange=Argument to function ''{0}'' must be a numer
 ValidationError=Validation Error: {0}
 IllegalLocaleFormat=Locale ''{0}'' in an illegal format
 ArgumentMustNotBeNull=Argument to function ''{0}'' must not be NULL
+AtLeastOneArgumentMustNotBeNull=At least one argument to function ''{0}'' must not be NULL

Review Comment:
   there are other functions which give an error in this case, but this error message is frankly better. maybe we should update the other ones.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1741338994


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -2829,6 +2829,20 @@ public static long bitAnd(long b0, long b1) {
     return b0 & b1;
   }
 
+  /** Bitwise function <code>BITAND</code> applied to a Long and int value.
+   *  Needed for handling NULL for the first argument.
+   */
+  public static long bitAnd(Long b0, int b1) {
+    return b0 & b1;
+  }
+
+  /** Bitwise function <code>BITAND</code> applied to a Long and int value.

Review Comment:
   Bitwise function <code>BITAND</code> applied to a int and Long value.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2322179403

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [8 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [47.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1743161556


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);

Review Comment:
   why are all these types nullable?



##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(
+            typeFactory.createSqlType(SqlTypeName.INTEGER), true);
+      };
+
+  /**
+   * Returns true if type2 is of a larger integer type than type1.
+   *
+   * @param type1 first type
+   * @param type2 second type
+   * @return true if type2 is of a larger integer type than type1
+   */
+  private static boolean isIntegerTypeLarger(RelDataType type1, RelDataType type2) {
+    if (SqlTypeName.TINYINT == type1.getSqlTypeName()) {

Review Comment:
   you can use the precision to make this simpler, and perhaps safer too.
   In some type systems maybe some of these types could be the same?



##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   When can this happen?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2341952633

   Commits have been squashed.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2338797847

   Sorry, but it looks like other merges have introduced conflicts, please fix them so we can merge this one too


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1744280290


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   For line 642 above, it would get there when at least one argument is a non integer type and not null. This is probably not quite correct. I only want to exclude the `NULL` type. A nullable value is fine as long as it is an integer or binary type.
   
   I'll see about correcting this.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2330252379

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [94.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [46.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2327624158

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [94.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [43.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1744276136


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);

Review Comment:
   Since `NULL` is a possible result.
   ```
   BITAND(1, NULL) = NULL
   BITAND(CAST(x'01' AS BINARY(1)), NULL) = NULL
   ```
   
   If nullability is set to `false` here, then `NULL` results are coerced back to integer types with the value `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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1745918891


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   In fact, if any argument has the NULL type the result type can also have the NULL type, since there is only one value with this type, NULL.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1745882348


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   I have reworked the logic here.
   
   If all arguments are of the NULL type, then the return type is INTEGER nullable.
   
   If all arguments are integer types, then the return type is the largest type found. It is nullable if at least one argument is nullable.
   
   As a fallback, the first argument that is not of the NULL type is the return type. It is nullable if at least one argument is nullable.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1744277880


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);

Review Comment:
   the nullability of the result should depend on the nullability of the arguments, if they are not null, the result can be not null 



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1745921208


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   BTW: there is a "leastRestrictive" function which does much of this, why doesn't it satisfy the requirements? After you know that the inputs are integer or NULL, that function should do the right thing.



##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   I was hoping that the nullpolicy would take care of that, but I guess I am wrong (the STRONG policy ensures that a function with null arguments is never called). But I guess you still need a return type, it can't be NULL. So probably the right solution is to return a compile-time error when all arguments have NULL type, and otherwise the type of the first non-NULL typed argument will do fine if any argument has type NULL.  



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1752684748


##########
core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties:
##########
@@ -207,6 +207,7 @@ ArgumentMustBeNumericLiteralInRange=Argument to function ''{0}'' must be a numer
 ValidationError=Validation Error: {0}
 IllegalLocaleFormat=Locale ''{0}'' in an illegal format
 ArgumentMustNotBeNull=Argument to function ''{0}'' must not be NULL
+AtLeastOneArgumentMustNotBeNull=At least one argument to function ''{0}'' must not be NULL

Review Comment:
   let's leave this for another PR, this has been strenuous enough



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2338781327

   From my pov we can squash the commits.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1745918040


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   The fallback could be coded more explicitly as a check for the NULL type and then an assertion failure.
   I like to be as explicit as possible, it's more future-proof.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1747781019


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   I agree with you in theory, but I don't know how to do this in practice. When I make the change so that the NULL type is returned if any argument is of the NULL type, then the tests fail. They fail with this error occurring when trying to convert the result.
   ```
   java.lang.NoSuchMethodException: java.lang.Void.valueOf(void)
   ```
   
   It looks like it is trying to the result to a Void type in Java. It never went to an implementation of the `BITAND()` function. I also tried use a return type of `java.lang.Long` for the relevant implementation of the `BITAND()` function. This gives the a similar error but for the Long type error:
   ```
   java.lang.NoSuchMethodException: java.lang.Long.voidValue()
   ```
   
   Another way to look at this is that `BITAND(1, NULL)` should return a `NULL` value of the type of the first argument. I am treating `BITAND(1, NULL)` as the same as `BITAND(NULL, 1)`.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1752682419


##########
core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties:
##########
@@ -207,6 +207,7 @@ ArgumentMustBeNumericLiteralInRange=Argument to function ''{0}'' must be a numer
 ValidationError=Validation Error: {0}
 IllegalLocaleFormat=Locale ''{0}'' in an illegal format
 ArgumentMustNotBeNull=Argument to function ''{0}'' must not be NULL
+AtLeastOneArgumentMustNotBeNull=At least one argument to function ''{0}'' must not be NULL

Review Comment:
   Do you know what I can look for? I could update them.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2327596880

   I think this pattern is common in all arithmetic operations for integer types. Hopefully you could have reused some code.
   Your runtime only supported int and long, so I suspected the results would be wrong for short.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1739931926


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -15470,6 +15470,111 @@ private static void checkLogicalOrFunc(SqlOperatorFixture f) {
     f.checkAgg("logical_or(x)", values4, isNullValue());
   }
 
+  @Test void testBitAndScalarFunc() {
+    final SqlOperatorFixture f = fixture();
+    f.setFor(SqlStdOperatorTable.BITAND, VmName.EXPAND);
+    f.checkFails("bitand(^*^)", "Unknown identifier '\\*'", false);
+    f.checkScalar("bitand(2, 3)", "2", "INTEGER");
+    f.checkType("bitand(CAST(2 AS TINYINT), CAST(6 AS TINYINT))", "TINYINT");
+    f.checkType("bitand(CAST(2 AS SMALLINT), CAST(6 AS SMALLINT))", "SMALLINT");
+    f.checkType("bitand(CAST(2 AS BIGINT), CAST(6 AS BIGINT))", "BIGINT");
+    f.checkScalar("bitand(CAST(x'0201' AS BINARY(2)), CAST(x'07f9' AS BINARY(2)))", "0201",
+        "BINARY(2)");
+    f.checkScalar("bitand(CAST(x'0201' AS VARBINARY(2)), CAST(x'07f9' AS VARBINARY(2)))", "0201",
+        "VARBINARY(2)");
+    f.checkFails("^bitand(1.2, 1.3)^",
+        "Cannot apply 'BITAND' to arguments of type '"
+            + "BITAND\\(<DECIMAL\\(2, 1\\)>, <DECIMAL\\(2, 1\\)>\\)'\\. Supported form\\(s\\): '"
+            + "BITAND\\(<INTEGER>, <INTEGER>\\)'\n"
+            + "'BITAND\\(<BINARY>, <BINARY>\\)'",
+        false);
+    f.checkFails("^bitand()^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkFails("^bitand(1)^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkFails("^bitand(1, 2, 3)^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkNull("bitand(NULL, 1)");
+    f.checkNull("bitand(1, NULL)");
+    f.checkNull("bitand(NULL, NULL)");
+    f.checkFails("bitand(CAST(x'0201' AS VARBINARY), CAST(x'02' AS VARBINARY))",
+        "Different length for bitwise operands: the first: 2, the second: 1",
+        true);
+  }
+
+  @Test void testBitOrScalarFunc() {
+    final SqlOperatorFixture f = fixture();
+    f.setFor(SqlStdOperatorTable.BITOR, VmName.EXPAND);
+    f.checkFails("bitor(^*^)", "Unknown identifier '\\*'", false);
+    f.checkScalar("bitor(2, 4)", "6", "INTEGER");
+    f.checkType("bitor(CAST(2 AS TINYINT), CAST(6 AS TINYINT))", "TINYINT");

Review Comment:
   Can we have a few tests with negative numbers, and a few with negative shorter integers?
   



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1750610190


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(

Review Comment:
   Updated. Now it gives an error when all arguments are NULL.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1742791360


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -15470,6 +15470,111 @@ private static void checkLogicalOrFunc(SqlOperatorFixture f) {
     f.checkAgg("logical_or(x)", values4, isNullValue());
   }
 
+  @Test void testBitAndScalarFunc() {
+    final SqlOperatorFixture f = fixture();
+    f.setFor(SqlStdOperatorTable.BITAND, VmName.EXPAND);
+    f.checkFails("bitand(^*^)", "Unknown identifier '\\*'", false);
+    f.checkScalar("bitand(2, 3)", "2", "INTEGER");
+    f.checkType("bitand(CAST(2 AS TINYINT), CAST(6 AS TINYINT))", "TINYINT");
+    f.checkType("bitand(CAST(2 AS SMALLINT), CAST(6 AS SMALLINT))", "SMALLINT");
+    f.checkType("bitand(CAST(2 AS BIGINT), CAST(6 AS BIGINT))", "BIGINT");
+    f.checkScalar("bitand(CAST(x'0201' AS BINARY(2)), CAST(x'07f9' AS BINARY(2)))", "0201",
+        "BINARY(2)");
+    f.checkScalar("bitand(CAST(x'0201' AS VARBINARY(2)), CAST(x'07f9' AS VARBINARY(2)))", "0201",
+        "VARBINARY(2)");
+    f.checkFails("^bitand(1.2, 1.3)^",
+        "Cannot apply 'BITAND' to arguments of type '"
+            + "BITAND\\(<DECIMAL\\(2, 1\\)>, <DECIMAL\\(2, 1\\)>\\)'\\. Supported form\\(s\\): '"
+            + "BITAND\\(<INTEGER>, <INTEGER>\\)'\n"
+            + "'BITAND\\(<BINARY>, <BINARY>\\)'",
+        false);
+    f.checkFails("^bitand()^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkFails("^bitand(1)^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkFails("^bitand(1, 2, 3)^",
+        "Invalid number of arguments to function 'BITAND'. Was expecting 2 arguments",
+        false);
+    f.checkNull("bitand(NULL, 1)");
+    f.checkNull("bitand(1, NULL)");
+    f.checkNull("bitand(NULL, NULL)");
+    f.checkFails("bitand(CAST(x'0201' AS VARBINARY), CAST(x'02' AS VARBINARY))",
+        "Different length for bitwise operands: the first: 2, the second: 1",
+        true);
+  }
+
+  @Test void testBitOrScalarFunc() {
+    final SqlOperatorFixture f = fixture();
+    f.setFor(SqlStdOperatorTable.BITOR, VmName.EXPAND);
+    f.checkFails("bitor(^*^)", "Unknown identifier '\\*'", false);
+    f.checkScalar("bitor(2, 4)", "6", "INTEGER");
+    f.checkType("bitor(CAST(2 AS TINYINT), CAST(6 AS TINYINT))", "TINYINT");

Review Comment:
   Added a few more tests with negative numbers.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2327594970

   > I don't really understand why the result for TINYINT will be correct, since today Calcite incorrectly does not truncate the results for short arithmetic.
   
   @mihaibudiu I have created a custom `SqlReturnTypeInference` class. It will look at the arguments and choose the largest integer type (if both are integer types), otherwise use the type of the first argument. If both arguments are `NULL`, then just assume integer nullable return type.
   
   Perhaps you mean when the result of the BITAND is converted from a long back to a TINYINT. If this is the case, do you have any examples where it fails to truncate the value correctly? No truncation should be needed, since the result cannot exceed the largest argument type.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2322202101

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [8 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [47.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1752684371


##########
core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties:
##########
@@ -207,6 +207,7 @@ ArgumentMustBeNumericLiteralInRange=Argument to function ''{0}'' must be a numer
 ValidationError=Validation Error: {0}
 IllegalLocaleFormat=Locale ''{0}'' in an illegal format
 ArgumentMustNotBeNull=Argument to function ''{0}'' must not be NULL
+AtLeastOneArgumentMustNotBeNull=At least one argument to function ''{0}'' must not be NULL

Review Comment:
   I have seen this: "Illegal use of ''NULL''"



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2341987665

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [8 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [94.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [47.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2342014308

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [8 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [94.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [47.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1744280709


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);
+        }
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (!opType.isNullable()) {
+            return typeFactory.createTypeWithNullability(opType, true);
+          }
+        }
+        return typeFactory.createTypeWithNullability(
+            typeFactory.createSqlType(SqlTypeName.INTEGER), true);
+      };
+
+  /**
+   * Returns true if type2 is of a larger integer type than type1.
+   *
+   * @param type1 first type
+   * @param type2 second type
+   * @return true if type2 is of a larger integer type than type1
+   */
+  private static boolean isIntegerTypeLarger(RelDataType type1, RelDataType type2) {
+    if (SqlTypeName.TINYINT == type1.getSqlTypeName()) {

Review Comment:
   Thanks, that helps a lot. I have cleaned up the code to make use of precision.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3942:
URL: https://github.com/apache/calcite/pull/3942#issuecomment-2329826980

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3942&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3942&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [95.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [47.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3942&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3942)
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-3779] Implement BITAND, BITOR, BITXOR scalar functions [calcite]

Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #3942:
URL: https://github.com/apache/calcite/pull/3942#discussion_r1744540521


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -613,6 +613,59 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
       ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Returns the type of the first non NULL argument or INTEGER if all arguments
+   * are NULL.
+   */
+  public static final SqlReturnTypeInference LARGEST_INT_OR_FIRST_NON_NULL_DEFAULT_INTEGER =
+      opBinding -> {
+        final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+        RelDataType integerReturnType = null;
+        boolean allArgsInteger = true;
+        for (RelDataType opType : opBinding.collectOperandTypes()) {
+          if (SqlTypeName.INT_TYPES.contains(opType.getSqlTypeName())) {
+            if (integerReturnType == null) {
+              integerReturnType = opType;
+            } else if (isIntegerTypeLarger(integerReturnType, opType)) {
+              integerReturnType = opType;
+            }
+          } else {
+            allArgsInteger = false;
+            break;
+          }
+        }
+        if (integerReturnType != null && allArgsInteger) {
+          return typeFactory.createTypeWithNullability(integerReturnType, true);

Review Comment:
   I see what you mean. I have reworked the return type detection. It is now only nullable if at least one argument type is nullable.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org