[GitHub] [calcite] jamesstarr commented on a change in pull request #2585: [CALCITE-4551] Reusing Immutable metadata cache keys

2021-10-27 Thread GitBox


jamesstarr commented on a change in pull request #2585:
URL: https://github.com/apache/calcite/pull/2585#discussion_r737982613



##
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/janino/DescriptiveCacheKey.java
##
@@ -16,14 +16,18 @@
  */
 package org.apache.calcite.rel.metadata.janino;
 
+import org.apiguardian.api.API;
+
 /**
  * A key used in caching with descriptive to string.  Note the key uses
  * reference equality for performance.
  */
+@API(status = API.Status.INTERNAL)

Review comment:
   Janino does not work with internal scoping.  These are called from 
Janino.




-- 
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] chunweilei merged pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


chunweilei merged pull request #2559:
URL: https://github.com/apache/calcite/pull/2559


   


-- 
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 master updated: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGTH for BigQuery dialect

2021-10-27 Thread chunwei
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new f61541d  [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, 
ARRAY_LENGTH for BigQuery dialect
f61541d is described below

commit f61541d633cfde53a4b0de0c23a010250c93274e
Author: snuyanzin 
AuthorDate: Tue May 11 21:21:29 2021 +0200

[CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGTH for 
BigQuery dialect
---
 .../calcite/adapter/enumerable/RexImpTable.java| 39 ++
 .../org/apache/calcite/runtime/SqlFunctions.java   |  7 
 .../main/java/org/apache/calcite/sql/SqlKind.java  |  6 
 .../calcite/sql/fun/SqlLibraryOperators.java   | 30 +
 .../org/apache/calcite/sql/type/OperandTypes.java  | 12 +++
 .../org/apache/calcite/util/BuiltInMethod.java |  5 ++-
 .../calcite/sql/test/SqlOperatorBaseTest.java  | 38 +
 site/_docs/reference.md|  3 ++
 8 files changed, 139 insertions(+), 1 deletion(-)

diff --git 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index 4aa6f4f..4921339 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -109,7 +109,10 @@ import static 
org.apache.calcite.linq4j.tree.ExpressionType.Subtract;
 import static org.apache.calcite.linq4j.tree.ExpressionType.UnaryPlus;
 import static org.apache.calcite.sql.fun.SqlInternalOperators.THROW_UNLESS;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY_AGG;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY_CONCAT;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY_CONCAT_AGG;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY_LENGTH;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.ARRAY_REVERSE;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.BOOL_AND;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.BOOL_OR;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR;
@@ -510,10 +513,14 @@ public class RexImpTable {
 // Multisets & arrays
 defineMethod(CARDINALITY, BuiltInMethod.COLLECTION_SIZE.method,
 NullPolicy.STRICT);
+defineMethod(ARRAY_LENGTH, BuiltInMethod.COLLECTION_SIZE.method,
+NullPolicy.STRICT);
 defineMethod(SLICE, BuiltInMethod.SLICE.method, NullPolicy.NONE);
 defineMethod(ELEMENT, BuiltInMethod.ELEMENT.method, NullPolicy.STRICT);
 defineMethod(STRUCT_ACCESS, BuiltInMethod.STRUCT_ACCESS.method, 
NullPolicy.ANY);
 defineMethod(MEMBER_OF, BuiltInMethod.MEMBER_OF.method, NullPolicy.NONE);
+defineMethod(ARRAY_REVERSE, BuiltInMethod.ARRAY_REVERSE.method, 
NullPolicy.STRICT);
+map.put(ARRAY_CONCAT, new ArrayConcatImplementor());
 final MethodImplementor isEmptyImplementor =
 new MethodImplementor(BuiltInMethod.IS_EMPTY.method, NullPolicy.NONE,
 false);
@@ -2657,6 +2664,38 @@ public class RexImpTable {
 }
   }
 
+  /** Implementor for a array concat. */
+  private static class ArrayConcatImplementor extends 
AbstractRexCallImplementor {
+
+ArrayConcatImplementor() {
+  super(NullPolicy.STRICT, false);
+}
+
+@Override String getVariableName() {
+  return "array_concat";
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator, RexCall 
call,
+List argValueList) {
+  final BlockBuilder blockBuilder = translator.getBlockBuilder();
+  final Expression list =
+  blockBuilder.append("list", Expressions.new_(ArrayList.class), 
false);
+  final Expression nullValue = Expressions.constant(null);
+  for (Expression expression : argValueList) {
+blockBuilder.add(
+Expressions.ifThenElse(
+Expressions.or(
+Expressions.equal(nullValue, list),
+Expressions.equal(nullValue, expression)),
+Expressions.assign(list, nullValue),
+Expressions.statement(
+Expressions.call(list, 
BuiltInMethod.COLLECTION_ADDALL.method, expression)))
+);
+  }
+  return list;
+}
+  }
+
   /** Implementor for a value-constructor. */
   private static class ValueConstructorImplementor
   extends AbstractRexCallImplementor {
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java 
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 382e091..17eeb00 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -63,6 +63,7 @@ import 

[GitHub] [calcite] chunweilei closed pull request #2589: Fix typo in reference.md

2021-10-27 Thread GitBox


chunweilei closed pull request #2589:
URL: https://github.com/apache/calcite/pull/2589


   


-- 
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 master updated: Fix typo in reference.md

2021-10-27 Thread chunwei
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 5bec337  Fix typo in reference.md
5bec337 is described below

commit 5bec337baad02a8cbf62df6b1260f52326d59eef
Author: snuyanzin 
AuthorDate: Mon Oct 18 18:03:04 2021 +0200

Fix typo in reference.md

Close apache/calcite#2589
---
 site/_docs/reference.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index caf0588..f186cca 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -2054,7 +2054,7 @@ completeness. Session is applied per product.
 **Note**: The `Tumble`, `Hop` and `Session` window table functions assign
 each row in the original table to a window. The output table has all
 the same columns as the original table plus two additional columns 
`window_start`
-and `window_end`, which repesent the start and end of the window interval, 
respectively.
+and `window_end`, which represent the start and end of the window interval, 
respectively.
 
 ### Grouped window functions
 **warning**: grouped window functions are deprecated.
@@ -2674,7 +2674,7 @@ Result
 SQL
 
 {% highlight sql %}
-ELECT JSON_KEYS(v) AS c1,
+SELECT JSON_KEYS(v) AS c1,
   JSON_KEYS(v, 'lax $.a') AS c2,
   JSON_KEYS(v, 'lax $.b') AS c2,
   JSON_KEYS(v, 'strict $.a[0]') AS c3,


[calcite] branch master updated: [CALCITE-4818] AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls (Taras Ledkov)

2021-10-27 Thread chunwei
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 3af1d78  [CALCITE-4818] AggregateExpandDistinctAggregatesRule must 
infer correct data type for top aggregate calls  (Taras Ledkov)
3af1d78 is described below

commit 3af1d78f7e60588c2497f8e0095d715ab179149a
Author: tledkov 
AuthorDate: Mon Oct 4 09:55:00 2021 +0300

[CALCITE-4818] AggregateExpandDistinctAggregatesRule must infer correct 
data type for top aggregate calls  (Taras Ledkov)

Close apache/calcite#2560
---
 .../AggregateExpandDistinctAggregatesRule.java |  7 ++--
 .../org/apache/calcite/test/RelOptRulesTest.java   | 42 ++
 .../org/apache/calcite/test/RelOptRulesTest.xml| 21 +++
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java
 
b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java
index 46b1ceb..38ae615 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java
@@ -26,7 +26,6 @@ import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.logical.LogicalAggregate;
-import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexInputRef;
@@ -368,14 +367,12 @@ public final class AggregateExpandDistinctAggregatesRule
 final int arg = bottomGroups.size() + nonDistinctAggCallProcessedSoFar;
 final List newArgs = ImmutableList.of(arg);
 if (aggCall.getAggregation().getKind() == SqlKind.COUNT) {
-  RelDataTypeFactory typeFactory = 
aggregate.getCluster().getTypeFactory();
-
   newCall =
   AggregateCall.create(new SqlSumEmptyIsZeroAggFunction(), false,
   aggCall.isApproximate(), aggCall.ignoreNulls(),
   newArgs, -1, aggCall.distinctKeys, aggCall.collation,
   originalGroupSet.cardinality(), relBuilder.peek(),
-  typeFactory.getTypeSystem().deriveSumType(typeFactory, 
aggCall.getType()),
+  null,
   aggCall.getName());
 } else {
   newCall =
@@ -383,7 +380,7 @@ public final class AggregateExpandDistinctAggregatesRule
   aggCall.isApproximate(), aggCall.ignoreNulls(),
   newArgs, -1, aggCall.distinctKeys, aggCall.collation,
   originalGroupSet.cardinality(),
-  relBuilder.peek(), aggCall.getType(), aggCall.name);
+  relBuilder.peek(), null, aggCall.name);
 }
 nonDistinctAggCallProcessedSoFar++;
   }
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java 
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 1a1e572..2cad6a9 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -6999,4 +6999,46 @@ class RelOptRulesTest extends RelOptTestBase {
 .withRule(CoreRules.AGGREGATE_EXPAND_DISTINCT_AGGREGATES_TO_JOIN)
 .check();
   }
+
+  /**
+   * Test case for https://issues.apache.org/jira/browse/CALCITE-4818;>[CALCITE-4818]
+   * AggregateExpandDistinctAggregatesRule must infer correct data type for 
top aggregate calls.
+   * 
+   * Checks AggregateExpandDistinctAggregatesRule when return type of the SUM 
aggregate
+   * is changed (expanded) by define custom type factory.
+   */
+  @Test void testSumAndDistinctSumWithExpandSumType() {
+// Define new type system to expand SUM return type.
+RelDataTypeSystemImpl typeSystem = new RelDataTypeSystemImpl() {
+  @Override public RelDataType deriveSumType(RelDataTypeFactory 
typeFactory,
+  RelDataType argumentType) {
+switch (argumentType.getSqlTypeName()) {
+case INTEGER:
+  return typeFactory.createSqlType(SqlTypeName.BIGINT);
+case BIGINT:
+  return typeFactory.createSqlType(SqlTypeName.DECIMAL);
+
+default:
+  return super.deriveSumType(typeFactory, argumentType);
+}
+  }
+};
+
+Supplier typeFactorySupplier = () -> new 
SqlTypeFactoryImpl(typeSystem);
+
+// Expected plan:
+// LogicalProject(EXPR$0=[CAST($0):BIGINT], EXPR$1=[$1])
+//  LogicalAggregate(group=[{}], EXPR$0=[SUM($1)], EXPR$1=[SUM($0)]) // 
RowType[DECIMAL, BIGINT]
+//LogicalAggregate(group=[{0}], EXPR$0=[SUM($0)])  // RowType[INTEGER, 
BIGINT]
+//  

[GitHub] [calcite] chunweilei closed pull request #2560: [CALCITE-4818] AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls

2021-10-27 Thread GitBox


chunweilei closed pull request #2560:
URL: https://github.com/apache/calcite/pull/2560


   


-- 
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] xy2953396112 commented on a change in pull request #2546: [CALCITE-4803] AggregateProjectMergeRule lost alias after remove the …

2021-10-27 Thread GitBox


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



##
File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectMergeRule.java
##
@@ -125,14 +125,17 @@ public AggregateProjectMergeRule(
 newGroupSet, newGroupingSets, aggCalls.build());
 
 // Add a project if the group set is not in the same order or
-// contains duplicates.
+// contains duplicates or contains alias.
 final RelBuilder relBuilder = call.builder();
 relBuilder.push(newAggregate);
 final List newKeys =
 Util.transform(aggregate.getGroupSet().asList(),
 key -> requireNonNull(map.get(key),
 () -> "no value found for key " + key + " in " + map));
-if (!newKeys.equals(newGroupSet.asList())) {
+final List newaggFieldNames = 
newAggregate.getRowType().getFieldNames();
+final List aggFieldNames = aggregate.getRowType().getFieldNames();

Review comment:
   > newaggFieldNames -> newAggFieldNames?
   
   ok, 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.

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

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




[GitHub] [calcite] NobiGo commented on a change in pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


NobiGo commented on a change in pull request #2559:
URL: https://github.com/apache/calcite/pull/2559#discussion_r737959127



##
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
##
@@ -2657,6 +2664,38 @@ private static RelDataType nullifyType(JavaTypeFactory 
typeFactory,
 }
   }
 
+  /** Implementor for a array concat. */
+  private static class ArrayConcatImplementor extends 
AbstractRexCallImplementor {
+
+ArrayConcatImplementor() {
+  super(NullPolicy.STRICT, false);
+}
+
+@Override String getVariableName() {
+  return "array_concat";
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator, RexCall 
call,
+List argValueList) {
+  final BlockBuilder blockBuilder = translator.getBlockBuilder();
+  final Expression list =
+  blockBuilder.append("list", Expressions.new_(ArrayList.class), 
false);
+  final Expression nullValue = Expressions.constant(null);

Review comment:
   nullValue can be replced by "NULL_EXPR"




-- 
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] NobiGo commented on a change in pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


NobiGo commented on a change in pull request #2559:
URL: https://github.com/apache/calcite/pull/2559#discussion_r737958944



##
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
##
@@ -2657,6 +2664,38 @@ private static RelDataType nullifyType(JavaTypeFactory 
typeFactory,
 }
   }
 
+  /** Implementor for a array concat. */
+  private static class ArrayConcatImplementor extends 
AbstractRexCallImplementor {
+
+ArrayConcatImplementor() {
+  super(NullPolicy.STRICT, false);
+}
+
+@Override String getVariableName() {
+  return "array_concat";
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator, RexCall 
call,
+List argValueList) {
+  final BlockBuilder blockBuilder = translator.getBlockBuilder();
+  final Expression list =
+  blockBuilder.append("list", Expressions.new_(ArrayList.class), 
false);

Review comment:
   +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-unsubscr...@calcite.apache.org

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




[GitHub] [calcite] NobiGo commented on a change in pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


NobiGo commented on a change in pull request #2559:
URL: https://github.com/apache/calcite/pull/2559#discussion_r737958837



##
File path: site/_docs/reference.md
##
@@ -2513,6 +2513,9 @@ semantics.
 | C | Operator syntax| Description
 |:- |:---|:---
 | p | expr :: type   | Casts *expr* to *type*
+| b | ARRAY_CONCAT(array [, array ]*)| Concatenates one or 
more arrays. If any input argument is `NULL` the function returns `NULL`

Review comment:
   Thanks for your reply. +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-unsubscr...@calcite.apache.org

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




[GitHub] [calcite-avatica] joshelser commented on pull request #132: [CALCITE-4152] Switch to ConfigurableSpnego Jetty implementations

2021-10-27 Thread GitBox


joshelser commented on pull request #132:
URL: https://github.com/apache/calcite-avatica/pull/132#issuecomment-953398364


   Also, fyi @stoty 


-- 
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] jcamachor commented on a change in pull request #2585: [CALCITE-4551] Reusing Immutable metadata cache keys

2021-10-27 Thread GitBox


jcamachor commented on a change in pull request #2585:
URL: https://github.com/apache/calcite/pull/2585#discussion_r737893951



##
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/janino/CacheGenerator.java
##
@@ -0,0 +1,419 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.janino;
+
+import org.apache.calcite.linq4j.Ord;
+import org.apache.calcite.linq4j.tree.Primitive;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.CyclicMetadataException;
+import org.apache.calcite.rel.metadata.DelegatingMetadataRel;
+import org.apache.calcite.rel.metadata.NullSentinel;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.runtime.FlatLists;
+
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Method;
+import java.util.stream.Collectors;
+
+import static org.apache.calcite.rel.metadata.janino.CodeGeneratorUtil.argList;
+import static 
org.apache.calcite.rel.metadata.janino.CodeGeneratorUtil.paramList;
+
+/**
+ * Generates caching code for janino backed metadata.
+ */
+class CacheGenerator {

Review comment:
   This seems to be a collection of utility methods, thus I suggest we 
rename the class: `CacheGenerator` -> `CacheGeneratorUtil`

##
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/janino/DescriptiveCacheKey.java
##
@@ -16,14 +16,18 @@
  */
 package org.apache.calcite.rel.metadata.janino;
 
+import org.apiguardian.api.API;
+
 /**
  * A key used in caching with descriptive to string.  Note the key uses
  * reference equality for performance.
  */
+@API(status = API.Status.INTERNAL)

Review comment:
   Why labeling as INTERNAL rather than changing the visibility (same 
question for other usages of INTERNAL which seem within the package scope, 
i.e., `CacheUtil`)?

##
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/janino/RelMetadataHandlerGenerator.java
##
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.janino;
+
+import org.apache.calcite.rel.metadata.MetadataDef;
+import org.apache.calcite.rel.metadata.MetadataHandler;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.immutables.value.Value;
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.calcite.linq4j.Nullness.castNonNull;
+
+/**
+ * Generates the {@link MetadataHandler} code.
+ */
+public class RelMetadataHandlerGenerator {

Review comment:
   `RelMetadataHandlerGenerator` -> `RelMetadataHandlerGeneratorUtil`.




-- 
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] jbalint commented on pull request #2560: [CALCITE-4818] AggregateExpandDistinctAggregatesRule must infer correct data type for top aggregate calls

2021-10-27 Thread GitBox


jbalint commented on pull request #2560:
URL: https://github.com/apache/calcite/pull/2560#issuecomment-953114778


   @tledkov-gridgain can you squash (and rebase) and then we'll merge?


-- 
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] asolimando commented on pull request #2592: [CALCITE-4768] cassandra4

2021-10-27 Thread GitBox


asolimando commented on pull request #2592:
URL: https://github.com/apache/calcite/pull/2592#issuecomment-953100170


   Never mind, I have added the description because there was a conflict to fix 
with master (gradle.properties).


-- 
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 master updated: [CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests

2021-10-27 Thread zabetak
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 2280879  [CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) 
in tests
2280879 is described below

commit 2280879f38c8347801749998f2801aaf7364c6f0
Author: Stamatis Zampetakis 
AuthorDate: Fri Oct 15 17:15:31 2021 +0200

[CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests

1. Replace slf4j-log4j12 dependency (using Log4j 1.x)  with
log4j-slf4j-impl (using Log4j 2.x) when possible. Cannot remove
Log4j 1.x from Pig/Piglet modules due to compilation dependency of the
latter directly to log4j.
2. Use XML syntax instead of property syntax for Log4j configuration. A
Log4j configuration is hierarchical by nature so formats with natural
support for nesting (such as XML) are easier to use.
3. Exclude slf4j-log4j12 when it comes transitively from other
dependencies. It gets in conflict with log4j-slf4j-impl and it may also
appear transitively in projects using Calcite.
4. Add log4j-slf4j-impl in modules (kafka, mongodb, plus, example) to
silence error messages due to missing logger implementation.
5. Always use testRuntimeOnly annotation for dependencies on SLF4J
logger implementation. Calcite shouldn't enforce a specific logger
implementation so they should never appear as compile or runtime
dependencies. A concrete implementation is only needed when we want to
log messages for testing purposes.
6. Bump Cassandra version to 3.11.3 to use Log4j2 in tests. Previous
versions did not allow the use of any custom logger implementation
(only logback). This was resolved CASSANDRA-13396 so now we can use
Log4j2 as the rest of the tests.
7. Unify display pattern in elasticsearch log4j2.xml with the rest of
tests.
8. Add/Modify Log4j2 configuration (log4j2-test.xml) to silence errors
and not display logging messages in tests. Instead of globally turning
off logs, specific logger entries were set in the configuration files:
(i) to avoid hiding by accident important problems;
(ii) to faciliate switching log levels during debugging in the future.

Close apache/calcite#2587
---
 babel/build.gradle.kts |  3 ++-
 bom/build.gradle.kts   |  5 +++-
 cassandra/build.gradle.kts | 12 -
 .../src/test/resources/log4j2-test.xml | 15 ++-
 core/build.gradle.kts  |  7 +++--
 core/src/test/resources/log4j.properties   | 31 --
 .../src/test/resources/log4j2-test.xml | 21 ++-
 druid/build.gradle.kts |  2 +-
 druid/src/test/resources/log4j.properties  | 30 -
 .../src/test/resources/log4j2-test.xml | 12 -
 .../test/resources/{log4j2.xml => log4j2-test.xml} | 12 -
 example/csv/build.gradle.kts   |  2 ++
 example/function/build.gradle.kts  |  1 +
 file/build.gradle.kts  |  1 +
 geode/build.gradle.kts |  2 +-
 geode/src/test/resources/log4j.properties  | 28 ---
 .../src/test/resources/log4j2-test.xml | 16 ++-
 gradle.properties  |  2 +-
 innodb/build.gradle.kts|  7 +++--
 .../src/test/resources/log4j2-test.xml | 12 -
 kafka/build.gradle.kts |  1 +
 .../src/test/resources/log4j2-test.xml | 12 -
 mongodb/build.gradle.kts   |  1 +
 mongodb/src/test/resources/log4j.properties| 26 --
 .../src/test/resources/log4j2-test.xml | 16 ++-
 plus/build.gradle.kts  |  1 +
 .../src/test/resources/log4j2-test.xml | 31 +++---
 redis/build.gradle.kts |  2 +-
 .../src/test/resources/log4j2-test.xml | 15 ++-
 server/build.gradle.kts|  2 +-
 .../src/test/resources/log4j2-test.xml | 14 +-
 spark/build.gradle.kts |  6 -
 spark/src/test/resources/log4j.properties  | 29 
 .../src/test/resources/log4j2-test.xml | 16 ++-
 splunk/build.gradle.kts|  2 +-
 splunk/src/test/resources/log4j.properties | 26 --
 .../src/test/resources/log4j2-test.xml | 13 -
 37 files changed, 162 insertions(+), 272 deletions(-)

diff --git a/babel/build.gradle.kts b/babel/build.gradle.kts
index 8b689d1..0a5d8dd 

[GitHub] [calcite] zabetak closed pull request #2587: [CALCITE-4858] Use Log4j2 instead of unsupported Log4j (1.x) in tests

2021-10-27 Thread GitBox


zabetak closed pull request #2587:
URL: https://github.com/apache/calcite/pull/2587


   


-- 
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] asolimando edited a comment on pull request #2592: [CALCITE-4768] cassandra4

2021-10-27 Thread GitBox


asolimando edited a comment on pull request #2592:
URL: https://github.com/apache/calcite/pull/2592#issuecomment-952730479


   On a second thought, there are some important info in the PR description 
that I think should be in the git description as well, could you add it when 
merging the PR?
   
   Here is what I think could be added, feel free to rephrase it if needed:
   
   ```
   * Updated Cassandra from 3.11.2 to 4.0.1
   * Updated Datastax Driver from 3.6.0 to 4.13.0
   * Updated cassandra-unit from 3.5.0.1 to 4.3.1.0
   * Cassandra tests are:
- now compatible with Guava >= 25 but are not anymore compatible with Guava 
< 20
- still incompatible with Eclipse OpenJ9 (due to Cassandra)
- still incompatible with JDK11+ (due to cassandra-unit)
   ```


-- 
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] asolimando commented on pull request #2592: [CALCITE-4768] cassandra4

2021-10-27 Thread GitBox


asolimando commented on pull request #2592:
URL: https://github.com/apache/calcite/pull/2592#issuecomment-952730479


   On a second thought, there are some important info in the PR description 
that I think should be in the git description as well, could you add it when 
merging the PR?
   
   Here is what I think could be added, feel free to rephrase it if needed:
   
   * Updated Cassandra from 3.11.2 to 4.0.1
   * Updated Datastax Driver from 3.6.0 to 4.13.0
   * Updated cassandra-unit from 3.5.0.1 to 4.3.1.0
   * Cassandra tests are:
- now compatible with Guava >= 25 but are not anymore compatible with Guava 
< 20
- still incompatible with Eclipse OpenJ9 (due to Cassandra)
- still incompatible with JDK11+ (due to cassandra-unit)
   


-- 
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] snuyanzin commented on a change in pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


snuyanzin commented on a change in pull request #2559:
URL: https://github.com/apache/calcite/pull/2559#discussion_r737287102



##
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
##
@@ -2657,6 +2664,38 @@ private static RelDataType nullifyType(JavaTypeFactory 
typeFactory,
 }
   }
 
+  /** Implementor for a array concat. */
+  private static class ArrayConcatImplementor extends 
AbstractRexCallImplementor {
+
+ArrayConcatImplementor() {
+  super(NullPolicy.STRICT, false);
+}
+
+@Override String getVariableName() {
+  return "array_concat";
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator, RexCall 
call,
+List argValueList) {
+  final BlockBuilder blockBuilder = translator.getBlockBuilder();
+  final Expression list =
+  blockBuilder.append("list", Expressions.new_(ArrayList.class), 
false);

Review comment:
   As far as I know `Expressions.assign()` is good when you have left and 
right parts. Here we do not have and need to create a new variable. Or ... 
could you please elaborate your suggestion?




-- 
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] snuyanzin commented on a change in pull request #2559: [CALCITE-4822] Add functions ARRAY_CONCAT, ARRAY_REVERSE, ARRAY_LENGT…

2021-10-27 Thread GitBox


snuyanzin commented on a change in pull request #2559:
URL: https://github.com/apache/calcite/pull/2559#discussion_r737273675



##
File path: site/_docs/reference.md
##
@@ -2513,6 +2513,9 @@ semantics.
 | C | Operator syntax| Description
 |:- |:---|:---
 | p | expr :: type   | Casts *expr* to *type*
+| b | ARRAY_CONCAT(array [, array ]*)| Concatenates one or 
more arrays. If any input argument is `NULL` the function returns `NULL`

Review comment:
   Not sure what exactly you are referring to... Probably this
   ```
   tester.checkScalar(
   "array_concat(array['hello', 'world'], array['!'], array[cast(null 
as char)])",
   "[hello, world, !, null]", "CHAR(5) ARRAY NOT NULL");
   tester.checkNull("array_concat(cast(null as integer array), array[1])");
   ```
   
   The key difference here is that in the first test there are elements 
`array['hello', 'world', '!']` and `array[null]` while in the second there are 
`null` and `array[1]`.
   So the tests check that if argument is null then return `null` however if an 
argument is a collection containing `null` value it should do normal 
concatenation.
   
   In case you are referring to something else please elaborate




-- 
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] asolimando edited a comment on pull request #2592: [CALCITE-4768] cassandra4

2021-10-27 Thread GitBox


asolimando edited a comment on pull request #2592:
URL: https://github.com/apache/calcite/pull/2592#issuecomment-952638170


   Thank you both for the review, I have squashed the commits and updated the 
commit message, once the test are over (even though there are no code changes 
since last run and the only commit missing from master was a site update) I 
think it can be merged.


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

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

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




[GitHub] [calcite] asolimando commented on pull request #2592: [CALCITE-4768] cassandra4

2021-10-27 Thread GitBox


asolimando commented on pull request #2592:
URL: https://github.com/apache/calcite/pull/2592#issuecomment-952638170


   Thank you both for the review, I have squashed the commits and updated the 
commit message, once the test are over I think it can be merged.


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

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

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