[GitHub] [calcite] alimans3 commented on a diff in pull request #2887: [CALCITE-5247] FilterJoinRule cannot simplify left join to inner join for `WHERE RHS.C1 IS NOT NULL OR RHS.C2 IS NOT NULL`

2022-08-30 Thread GitBox


alimans3 commented on code in PR #2887:
URL: https://github.com/apache/calcite/pull/2887#discussion_r958796188


##
core/src/main/java/org/apache/calcite/plan/Strong.java:
##
@@ -151,12 +151,41 @@ public static boolean allStrong(List operands) {
 
   /** Returns whether an expression is definitely not true. */
   public boolean isNotTrue(RexNode node) {
+return isPredicateNotTrue(node) || isNull(node);

Review Comment:
   it is doable, it covers more cases this way hence the new change in 
RelOptRules.xml
   The case is AND/OR(DEFINITELY FALSE, DEFINITELY NULL) -> DEFINITELY FALSE
   done!



-- 
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-unsubscr...@calcite.apache.org

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



[GitHub] [calcite] libenchao commented on a diff in pull request #2887: [CALCITE-5247] FilterJoinRule cannot simplify left join to inner join for `WHERE RHS.C1 IS NOT NULL OR RHS.C2 IS NOT NULL`

2022-08-30 Thread GitBox


libenchao commented on code in PR #2887:
URL: https://github.com/apache/calcite/pull/2887#discussion_r958463132


##
core/src/main/java/org/apache/calcite/plan/Strong.java:
##
@@ -151,12 +151,41 @@ public static boolean allStrong(List operands) {
 
   /** Returns whether an expression is definitely not true. */
   public boolean isNotTrue(RexNode node) {
+return isPredicateNotTrue(node) || isNull(node);

Review Comment:
   I'm still not sure why we cannot merge `isNotTrue` and `isPredicateNotTrue` 
like below:
   
   ```
 public boolean isNotTrue(RexNode node) {
   switch (node.getKind()) {
   //TODO Enrich with more possible cases?
   case IS_NOT_NULL:
 return isNull(((RexCall) node).getOperands().get(0));
   case OR:
 return allNotTrue(((RexCall) node).getOperands());
   case AND:
 return anyNotTrue(((RexCall) node).getOperands());
   default:
 return isNull(node);
   }
 }
   
 /** Returns whether all expressions in a list are definitely not true. */
 private boolean allNotTrue(List operands) {
   for (RexNode operand : operands) {
 if (!isNotTrue(operand)) {
   return false;
 }
   }
   return true;
 }
   
 /** Returns whether any expressions in a list are definitely not true. */
 private boolean anyNotTrue(List operands) {
   for (RexNode operand : operands) {
 if (isNotTrue(operand)) {
   return true;
 }
   }
   return 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.

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

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



[GitHub] [calcite] Fanoid commented on pull request #1766: [CALCITE-3745] UnitCompiler can not find required class information.

2022-08-30 Thread GitBox


Fanoid commented on PR #1766:
URL: https://github.com/apache/calcite/pull/1766#issuecomment-1231581297

   Hi, I am also blocked by the same issue @gr4ve encountered. I think this 
issue is not resolved by @birschick-bq 's commits.
   Are there any updates about the original problem?


-- 
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-unsubscr...@calcite.apache.org

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



[GitHub] [calcite] rubenada merged pull request #2712: [CALCITE-4999] ARRAY, MULTISET functions should return a collection of scalars if a sub-query returns 1 column

2022-08-30 Thread GitBox


rubenada merged PR #2712:
URL: https://github.com/apache/calcite/pull/2712


-- 
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-unsubscr...@calcite.apache.org

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



[calcite] branch main updated: [CALCITE-4999] ARRAY, MULTISET functions should return a collection of scalars if a sub-query returns 1 column

2022-08-30 Thread rubenql
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/main by this push:
 new 71e30e2c70 [CALCITE-4999] ARRAY, MULTISET functions should return a 
collection of scalars if a sub-query returns 1 column
71e30e2c70 is described below

commit 71e30e2c70e454bdd09eef12ad239ba510ce
Author: dssysolyatin 
AuthorDate: Mon Aug 29 12:13:50 2022 +0300

[CALCITE-4999] ARRAY, MULTISET functions should return a collection of 
scalars if a sub-query returns 1 column
---
 .../adapter/enumerable/EnumerableCollect.java  | 30 ++-
 .../java/org/apache/calcite/rel/core/Collect.java  | 45 ++
 .../apache/calcite/rel/mutable/MutableRels.java|  3 +-
 .../calcite/rel/rules/SubQueryRemoveRule.java  | 14 ++-
 .../java/org/apache/calcite/rex/RexSubQuery.java   |  9 -
 .../calcite/sql/fun/SqlArrayQueryConstructor.java  |  2 +-
 .../sql/fun/SqlMultisetQueryConstructor.java   |  2 +-
 .../apache/calcite/sql/type/SqlTypeTransforms.java | 21 ++
 .../org/apache/calcite/sql/type/SqlTypeUtil.java   | 22 +++
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |  6 +--
 .../calcite/sql2rel/StandardConvertletTable.java   | 16 ++--
 .../java/org/apache/calcite/test/JdbcTest.java |  2 +-
 .../org/apache/calcite/test/SqlValidatorTest.java  | 32 ++-
 .../org/apache/calcite/test/SqlOperatorTest.java   | 31 +++
 14 files changed, 189 insertions(+), 46 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
index da50dff617..64ead33167 100644
--- 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
+++ 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
@@ -25,8 +25,11 @@ import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Collect;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
 import org.apache.calcite.util.BuiltInMethod;
 
+import static java.util.Objects.requireNonNull;
+
 /** Implementation of {@link org.apache.calcite.rel.core.Collect} in
  * {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable calling convention}. */
 public class EnumerableCollect extends Collect implements EnumerableRel {
@@ -91,15 +94,24 @@ public class EnumerableCollect extends Collect implements 
EnumerableRel {
 Expression child_ =
 builder.append(
 "child", result.block);
-// In the internal representation of multisets , every element must be a 
record. In case the
-// result above is a scalar type we have to wrap it around a physical type 
capable of
-// representing records. For this reason the following conversion is 
necessary.
-// REVIEW zabetak January 7, 2019: If we can ensure that the input to this 
operator
-// has the correct physical type (e.g., respecting the Prefer.ARRAY above) 
then this conversion
-// can be removed.
-Expression conv_ =
-builder.append(
-"converted", result.physType.convertTo(child_, 
JavaRowFormat.ARRAY));
+
+RelDataType collectionComponentType =
+
requireNonNull(rowType().getFieldList().get(0).getType().getComponentType());
+RelDataType childRecordType = 
result.physType.getRowType().getFieldList().get(0).getType();
+
+Expression conv_ = child_;
+if (!SqlTypeUtil.sameNamedType(collectionComponentType, childRecordType)) {
+  // In the internal representation of multisets , every element must be a 
record. In case the
+  // result above is a scalar type we have to wrap it around a physical 
type capable of
+  // representing records. For this reason the following conversion is 
necessary.
+  // REVIEW zabetak January 7, 2019: If we can ensure that the input to 
this operator
+  // has the correct physical type (e.g., respecting the Prefer.ARRAY 
above)
+  // then this conversion can be removed.
+  conv_ =
+  builder.append(
+  "converted", result.physType.convertTo(child_, 
JavaRowFormat.ARRAY));
+}
+
 Expression list_ =
 builder.append("list",
 Expressions.call(conv_,
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Collect.java 
b/core/src/main/java/org/apache/calcite/rel/core/Collect.java
index 28b83c5ff5..50891d5a7e 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Collect.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Collect.java
@@ -25,6 +25,7 @@ import org.apache.calcite.rel.RelWriter;
 import org.apache.calcite.rel.SingleRel;
 import org.apache.calcite.rel.type.RelDataType;
 import