[GitHub] [calcite] danny0405 commented on issue #1803: [CALCITE-3793] AssertionError throws for planner digest cache key whe…

2020-02-12 Thread GitBox
danny0405 commented on issue #1803: [CALCITE-3793] AssertionError throws for 
planner digest cache key whe…
URL: https://github.com/apache/calcite/pull/1803#issuecomment-585588663
 
 
   Apache Flink and Apache Druid all the customize type for rel node which is 
not a strict 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 opened a new pull request #1803: [CALCITE-3793] AssertionError throws for planner digest cache key whe…

2020-02-12 Thread GitBox
danny0405 opened a new pull request #1803: [CALCITE-3793] AssertionError throws 
for planner digest cache key whe…
URL: https://github.com/apache/calcite/pull/1803
 
 
   …n the rel type is not struct


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 opened a new pull request #1802: [CALCITE-3792] Remove the generic type declaration of method

2020-02-12 Thread GitBox
danny0405 opened a new pull request #1802: [CALCITE-3792] Remove the generic 
type declaration of method
URL: https://github.com/apache/calcite/pull/1802
 
 
   * The type inference of Scala code 'val mq = cluster.getMetadataQuery'
   is failed if we have that generic type declaration for the Java method
   * After this change, we need a explicit cast for RelMetadataQuery
   sub-class
   * Fix the RelMetadataTest to reset the RelMetadataQuery instance as
   default after a test
   * Also rename RelOptCluster#withHintStrategies to setHintStrategies
   because it does not really return a new copy


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] chunweilei opened a new pull request #1801: [CALCITE-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens

2020-02-12 Thread GitBox
chunweilei opened a new pull request #1801: [CALCITE-3791] HepPlanner does not 
clear metadata cache for the ancestors of discarded node when a transformation 
happens
URL: https://github.com/apache/calcite/pull/1801
 
 
   JIRA: https://issues.apache.org/jira/browse/CALCITE-3791.
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] liyafan82 opened a new pull request #1800: [CALCITE-3790] Make the URL of FileSource available

2020-02-12 Thread GitBox
liyafan82 opened a new pull request #1800: [CALCITE-3790] Make the URL of 
FileSource available
URL: https://github.com/apache/calcite/pull/1800
 
 
   When a FileSource is constructed with only a File object, its URL is left 
null. This makes it inconvenient for some scenarios where a valid URL is 
required.
   
   This can be resolved, as each file in the local file system corresponds to a 
file URL, and we fix it by converting a file object to a file URL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[calcite] branch master updated (555da95 -> 9b2a559)

2020-02-12 Thread sereda
This is an automated email from the ASF dual-hosted git repository.

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


from 555da95  [CALCITE-3774] In RelBuilder and ProjectMergeRule, prevent 
merges when it would increase expression complexity
 add 9b2a559  [CALCITE-2442] Remove .toDelete cassandra temp folder on 
Windows after tests

No new revisions were added by this update.

Summary of changes:
 cassandra/gradle.properties|   2 -
 .../calcite/test/AbstractCassandraAdapterTest.java | 110 --
 .../test/CassandraAdapterDataTypesTest.java|  29 +--
 .../apache/calcite/test/CassandraAdapterTest.java  |  24 ++-
 .../apache/calcite/test/CassandraExtension.java| 221 +
 cassandra/src/test/resources/logback-test.xml  |   3 +
 6 files changed, 255 insertions(+), 134 deletions(-)
 delete mode 100644 
cassandra/src/test/java/org/apache/calcite/test/AbstractCassandraAdapterTest.java
 create mode 100644 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java



[GitHub] [calcite] asereda-gs merged pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs merged pull request #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585528506
 
 
   @asereda-gs Thanks! It works well on both machines.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585522304
 
 
   Try now pls (refresh your local copy).
   On machine1 where startup was successful.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1794: [CALCITE-3781] HintStrategy can specify excluded rules for planner

2020-02-12 Thread GitBox
danny0405 commented on a change in pull request #1794: [CALCITE-3781] 
HintStrategy can specify excluded rules for planner
URL: https://github.com/apache/calcite/pull/1794#discussion_r378620001
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
 ##
 @@ -160,6 +160,11 @@ protected void onMatch() {
 return;
   }
 
+  if (isRuleExcluded()) {
+LOGGER.debug("Rule [{}] not fired due to exclusion hint", getRule());
 
 Review comment:
   It's hard to do that, because the operands can be matched from children to 
parent (ASENDING), for current implementation, i only want to match the root 
operand of the final match.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585517278
 
 
   
![snapshot](https://user-images.githubusercontent.com/29158321/74395667-eb45fb80-4e4a-11ea-95c5-a1dcedb83363.png)
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585516671
 
 
   I checked both of them, `.toDelete` file exists.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585514761
 
 
   For machine1 logs are fine (`SigarException` is expected). Can you check if 
`.toDelete` exists after running tests on machine1 ? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[calcite] 01/03: Add RelBuilder.transform, which allows you to clone a RelBuilder with slightly different Config

2020-02-12 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

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

commit 051b6919dfc5b60406e81d5e8b8d5efb263def87
Author: Julian Hyde 
AuthorDate: Fri Feb 7 18:10:42 2020 -0800

Add RelBuilder.transform, which allows you to clone a RelBuilder with 
slightly different Config

Add class RelFactories.Struct, which contains an instance of
each RelNode factory. This allows more efficient initialization
of RelBuilder.
---
 .../org/apache/calcite/rel/core/RelFactories.java  | 132 +--
 .../java/org/apache/calcite/tools/RelBuilder.java  | 184 +
 2 files changed, 193 insertions(+), 123 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java 
b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
index 13b2cfd..e077008 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/RelFactories.java
@@ -17,6 +17,7 @@
 package org.apache.calcite.rel.core;
 
 import org.apache.calcite.linq4j.function.Experimental;
+import org.apache.calcite.plan.Context;
 import org.apache.calcite.plan.Contexts;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptTable;
@@ -53,6 +54,7 @@ import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
 import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -60,6 +62,7 @@ import com.google.common.collect.ImmutableSet;
 import java.lang.reflect.Type;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.SortedSet;
 import javax.annotation.Nonnull;
@@ -116,24 +119,28 @@ public class RelFactories {
   public static final RepeatUnionFactory DEFAULT_REPEAT_UNION_FACTORY =
   new RepeatUnionFactoryImpl();
 
+  public static final Struct DEFAULT_STRUCT =
+  new Struct(DEFAULT_FILTER_FACTORY,
+  DEFAULT_PROJECT_FACTORY,
+  DEFAULT_AGGREGATE_FACTORY,
+  DEFAULT_SORT_FACTORY,
+  DEFAULT_EXCHANGE_FACTORY,
+  DEFAULT_SORT_EXCHANGE_FACTORY,
+  DEFAULT_SET_OP_FACTORY,
+  DEFAULT_JOIN_FACTORY,
+  DEFAULT_CORRELATE_FACTORY,
+  DEFAULT_VALUES_FACTORY,
+  DEFAULT_TABLE_SCAN_FACTORY,
+  DEFAULT_TABLE_FUNCTION_SCAN_FACTORY,
+  DEFAULT_SNAPSHOT_FACTORY,
+  DEFAULT_MATCH_FACTORY,
+  DEFAULT_SPOOL_FACTORY,
+  DEFAULT_REPEAT_UNION_FACTORY);
+
   /** A {@link RelBuilderFactory} that creates a {@link RelBuilder} that will
* create logical relational expressions for everything. */
   public static final RelBuilderFactory LOGICAL_BUILDER =
-  RelBuilder.proto(
-  Contexts.of(DEFAULT_PROJECT_FACTORY,
-  DEFAULT_FILTER_FACTORY,
-  DEFAULT_JOIN_FACTORY,
-  DEFAULT_SORT_FACTORY,
-  DEFAULT_EXCHANGE_FACTORY,
-  DEFAULT_SORT_EXCHANGE_FACTORY,
-  DEFAULT_AGGREGATE_FACTORY,
-  DEFAULT_MATCH_FACTORY,
-  DEFAULT_SET_OP_FACTORY,
-  DEFAULT_VALUES_FACTORY,
-  DEFAULT_TABLE_SCAN_FACTORY,
-  DEFAULT_SNAPSHOT_FACTORY,
-  DEFAULT_SPOOL_FACTORY,
-  DEFAULT_REPEAT_UNION_FACTORY));
+  RelBuilder.proto(Contexts.of(DEFAULT_STRUCT));
 
   private RelFactories() {
   }
@@ -673,4 +680,99 @@ public class RelFactories {
   return LogicalRepeatUnion.create(seed, iterative, all, iterationLimit);
 }
   }
+
+  /** Immutable record that contains an instance of each factory. */
+  public static class Struct {
+public final FilterFactory filterFactory;
+public final ProjectFactory projectFactory;
+public final AggregateFactory aggregateFactory;
+public final SortFactory sortFactory;
+public final ExchangeFactory exchangeFactory;
+public final SortExchangeFactory sortExchangeFactory;
+public final SetOpFactory setOpFactory;
+public final JoinFactory joinFactory;
+public final CorrelateFactory correlateFactory;
+public final ValuesFactory valuesFactory;
+public final TableScanFactory scanFactory;
+public final TableFunctionScanFactory tableFunctionScanFactory;
+public final SnapshotFactory snapshotFactory;
+public final MatchFactory matchFactory;
+public final SpoolFactory spoolFactory;
+public final RepeatUnionFactory repeatUnionFactory;
+
+private Struct(FilterFactory filterFactory,
+ProjectFactory projectFactory,
+AggregateFactory aggregateFactory,
+SortFactory sortFactory,
+ExchangeFactory exchangeFactory,
+SortExchangeFactory sortExchangeFactory,
+SetOpFactory 

[calcite] 02/03: [CALCITE-3763] RelBuilder.aggregate should prune unused fields from the input, if the input is a Project

2020-02-12 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

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

commit ceb972952739929c175dfd0895407e8e17e0b502
Author: Julian Hyde 
AuthorDate: Fri Jan 31 16:57:31 2020 -0800

[CALCITE-3763] RelBuilder.aggregate should prune unused fields from the 
input, if the input is a Project
---
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  13 +-
 .../rel/rules/AbstractMaterializedViewRule.java|   2 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  |  72 +-
 .../java/org/apache/calcite/test/JdbcTest.java |  11 +-
 .../org/apache/calcite/test/PigRelBuilderTest.java |  34 +++--
 .../org/apache/calcite/test/RelBuilderTest.java| 160 ++---
 .../org/apache/calcite/test/RelOptRulesTest.xml| 116 ---
 .../java/org/apache/calcite/test/PigRelOpTest.java |   4 +-
 .../java/org/apache/calcite/test/PigletTest.java   |   4 +-
 9 files changed, 309 insertions(+), 107 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java 
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 0183486..99469d6 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -627,7 +627,8 @@ public abstract class RelOptUtil {
* Creates a plan suitable for use in EXISTS or IN
* statements.
*
-   * @see org.apache.calcite.sql2rel.SqlToRelConverter#convertExists
+   * @see org.apache.calcite.sql2rel.SqlToRelConverter
+   * SqlToRelConverter#convertExists
*
* @param seekRelA query rel, for example the resulting rel from 'select 
*
*   from emp' or 'values (1,2,3)' or '('Foo', 34)'.
@@ -898,9 +899,15 @@ public abstract class RelOptUtil {
 
   /** Gets all fields in an aggregate. */
   public static Set getAllFields(Aggregate aggregate) {
+return getAllFields2(aggregate.getGroupSet(), aggregate.getAggCallList());
+  }
+
+  /** Gets all fields in an aggregate. */
+  public static Set getAllFields2(ImmutableBitSet groupSet,
+  List aggCallList) {
 final Set allFields = new TreeSet<>();
-allFields.addAll(aggregate.getGroupSet().asList());
-for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+allFields.addAll(groupSet.asList());
+for (AggregateCall aggregateCall : aggCallList) {
   allFields.addAll(aggregateCall.getArgList());
   if (aggregateCall.filterArg >= 0) {
 allFields.add(aggregateCall.filterArg);
diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
 
b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
index cd24090..80eb1ff 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
@@ -506,7 +506,7 @@ public abstract class AbstractMaterializedViewRule extends 
RelOptRule {
 // Then, we trigger the unifying method. This method will either 
create a
 // Project or an Aggregate operator on top of the view. It will 
also compute
 // the output expressions for the query.
-RelBuilder builder = call.builder();
+RelBuilder builder = call.builder().transform(c -> 
c.withPruneInputOfAggregate(false));
 RelNode viewWithFilter;
 if (!viewCompensationPred.isAlwaysTrue()) {
   RexNode newPred =
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java 
b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index e4ef056..800f6d9 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -110,6 +110,7 @@ import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Deque;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -1584,7 +1585,7 @@ public class RelBuilder {
 final Registrar registrar =
 new Registrar(fields(), peek().getRowType().getFieldNames());
 final GroupKeyImpl groupKey_ = (GroupKeyImpl) groupKey;
-final ImmutableBitSet groupSet =
+ImmutableBitSet groupSet =
 ImmutableBitSet.of(registrar.registerExpressions(groupKey_.nodes));
   label:
 if (Iterables.isEmpty(aggCalls)) {
@@ -1610,7 +1611,7 @@ public class RelBuilder {
 return project(fields(groupSet));
   }
 }
-final ImmutableList groupSets;
+ImmutableList groupSets;
 if (groupKey_.nodeLists != null) {
   final int sizeBefore = registrar.extraNodes.size();
   final SortedSet groupSetSet =
@@ -1646,7 +1647,7 @@ public class RelBuilder {
 project(registrar.extraNodes);
 rename(registrar.names);
 final Frame frame = stack.pop();
-

[calcite] 03/03: [CALCITE-3774] In RelBuilder and ProjectMergeRule, prevent merges when it would increase expression complexity

2020-02-12 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

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

commit 555da953fe758a7d310aeb3aed463f3f2f3cdc3b
Author: Julian Hyde 
AuthorDate: Wed Feb 5 17:02:22 2020 -0800

[CALCITE-3774] In RelBuilder and ProjectMergeRule, prevent merges when it 
would increase expression complexity

Add an option RelBuilder.Config.bloat(), default 100.
Set it, using RelBuilder.Config.withBloat(int),
to -1 to never merge,
0 to merge only if complexity does not increase,
b to merge if complexity increases by no more than b.

Deprecate RelBuilder.shouldMergeProject().

Cache the nodeCount value in RexCall and RexWindow. Compute nodeCount
each time for RexOver (a sub-class of RexCall with an extra window),
because caching it would increase the complexity of RexCall's
constructor.
---
 .../java/org/apache/calcite/plan/RelOptUtil.java   | 23 +++
 .../rel/rules/AbstractMaterializedViewRule.java|  4 +-
 .../apache/calcite/rel/rules/ProjectMergeRule.java | 24 ++-
 .../main/java/org/apache/calcite/rex/RexCall.java  |  6 ++
 .../main/java/org/apache/calcite/rex/RexNode.java  | 12 
 .../main/java/org/apache/calcite/rex/RexOver.java  |  4 ++
 .../main/java/org/apache/calcite/rex/RexUtil.java  | 15 
 .../java/org/apache/calcite/rex/RexWindow.java | 18 +++--
 .../org/apache/calcite/rex/RexWindowBound.java | 13 
 .../java/org/apache/calcite/tools/RelBuilder.java  | 56 ++-
 .../org/apache/calcite/rex/RexProgramTest.java |  2 +-
 .../org/apache/calcite/rex/RexProgramTestBase.java | 11 ---
 .../org/apache/calcite/test/RelBuilderTest.java| 80 ++
 .../java/org/apache/calcite/tools/PlannerTest.java |  2 +-
 .../org/apache/calcite/piglet/PigRelBuilder.java   | 17 +++--
 15 files changed, 259 insertions(+), 28 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java 
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 99469d6..9b73fd1 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -135,6 +135,7 @@ import java.util.TreeSet;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 /**
  * RelOptUtil defines static utility methods for use in optimizing
@@ -2967,6 +2968,28 @@ public abstract class RelOptUtil {
 return list;
   }
 
+  /** As {@link #pushPastProject}, but returns null if the resulting 
expressions
+   * are significantly more complex.
+   *
+   * @param bloat Maximum allowable increase in complexity */
+  public static @Nullable List pushPastProjectUnlessBloat(
+  List nodes, Project project, int bloat) {
+if (bloat < 0) {
+  // If bloat is negative never merge.
+  return null;
+}
+final List list = pushPastProject(nodes, project);
+final int bottomCount = RexUtil.nodeCount(project.getProjects());
+final int topCount = RexUtil.nodeCount(nodes);
+final int mergedCount = RexUtil.nodeCount(list);
+if (mergedCount > bottomCount + topCount + bloat) {
+  // The merged expression is more complex than the input expressions.
+  // Do not merge.
+  return null;
+}
+return list;
+  }
+
   private static RexShuttle pushShuttle(final Project project) {
 return new RexShuttle() {
   @Override public RexNode visitInputRef(RexInputRef ref) {
diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
 
b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
index 80eb1ff..d1338a9 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java
@@ -960,7 +960,9 @@ public abstract class AbstractMaterializedViewRule extends 
RelOptRule {
   Filter.class, relBuilderFactory, Aggregate.class);
   this.aggregateProjectPullUpConstantsRule = new 
AggregateProjectPullUpConstantsRule(
   Aggregate.class, Filter.class, relBuilderFactory, 
"AggFilterPullUpConstants");
-  this.projectMergeRule = new ProjectMergeRule(true, relBuilderFactory);
+  this.projectMergeRule =
+  new ProjectMergeRule(true, ProjectMergeRule.DEFAULT_BLOAT,
+  relBuilderFactory);
 }
 
 @Override protected boolean isValidPlan(Project topProject, RelNode node,
diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java 
b/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java
index 2d550f9..818f7b5 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectMergeRule.java
@@ -37,14 +37,20 @@ import 

[calcite] branch master updated (a09923f -> 555da95)

2020-02-12 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

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


from a09923f  [CALCITE-3785] HepPlanner.belongToDag() doesn't have to use 
mapDigestToVertex (Xiening Dai)
 new 051b691  Add RelBuilder.transform, which allows you to clone a 
RelBuilder with slightly different Config
 new ceb9729  [CALCITE-3763] RelBuilder.aggregate should prune unused 
fields from the input, if the input is a Project
 new 555da95  [CALCITE-3774] In RelBuilder and ProjectMergeRule, prevent 
merges when it would increase expression complexity

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  36 ++-
 .../org/apache/calcite/rel/core/RelFactories.java  | 132 -
 .../rel/rules/AbstractMaterializedViewRule.java|   6 +-
 .../apache/calcite/rel/rules/ProjectMergeRule.java |  24 +-
 .../main/java/org/apache/calcite/rex/RexCall.java  |   6 +
 .../main/java/org/apache/calcite/rex/RexNode.java  |  12 +
 .../main/java/org/apache/calcite/rex/RexOver.java  |   4 +
 .../main/java/org/apache/calcite/rex/RexUtil.java  |  15 +
 .../java/org/apache/calcite/rex/RexWindow.java |  18 +-
 .../org/apache/calcite/rex/RexWindowBound.java |  13 +
 .../java/org/apache/calcite/tools/RelBuilder.java  | 312 +
 .../org/apache/calcite/rex/RexProgramTest.java |   2 +-
 .../org/apache/calcite/rex/RexProgramTestBase.java |  11 -
 .../java/org/apache/calcite/test/JdbcTest.java |  11 +-
 .../org/apache/calcite/test/PigRelBuilderTest.java |  34 ++-
 .../org/apache/calcite/test/RelBuilderTest.java| 240 ++--
 .../java/org/apache/calcite/tools/PlannerTest.java |   2 +-
 .../org/apache/calcite/test/RelOptRulesTest.xml| 116 
 .../org/apache/calcite/piglet/PigRelBuilder.java   |  17 +-
 .../java/org/apache/calcite/test/PigRelOpTest.java |   4 +-
 .../java/org/apache/calcite/test/PigletTest.java   |   4 +-
 21 files changed, 761 insertions(+), 258 deletions(-)



[GitHub] [calcite] DonnyZone edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete 
cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585513650
 
 
   Gralde build log in machine 1:
   ```
   > Task :cassandra:test
   
   CassandraAdapterDataTypesTest STANDARD_OUT
   2020-02-13 09:30:02 [pool-2-thread-1] ERROR 
o.a.cassandra.service.StartupChecks - cassandra.jmx.local.port missing from 
cassandra-env.sh, unable to start local JMX service.
   
   CassandraAdapterDataTypesTest STANDARD_ERROR
   no sigar-amd64-winnt.dll in java.library.path
   org.hyperic.sigar.SigarException: no sigar-amd64-winnt.dll in 
java.library.path
   at org.hyperic.sigar.Sigar.loadLibrary(Sigar.java:172)
   at org.hyperic.sigar.Sigar.(Sigar.java:100)
   at 
org.apache.cassandra.utils.SigarLibrary.(SigarLibrary.java:47)
   at 
org.apache.cassandra.utils.SigarLibrary.(SigarLibrary.java:28)
   at 
org.apache.cassandra.service.StartupChecks$7.execute(StartupChecks.java:266)
   at 
org.apache.cassandra.service.StartupChecks.verify(StartupChecks.java:125)
   at 
org.apache.cassandra.service.CassandraDaemon.setup(CassandraDaemon.java:200)
   at 
org.apache.cassandra.service.CassandraDaemon.activate(CassandraDaemon.java:602)
   at 
org.cassandraunit.utils.EmbeddedCassandraServerHelper.lambda$startEmbeddedCassandra$1(EmbeddedCassandraServerHelper.java:150)
   at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
   at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
   at java.lang.Thread.run(Thread.java:745)
 4.0sec, org.apache.calcite.test.CassandraAdapterDataTypesTest > 
testCollectionsRowType()
   WARNING  40.0sec,   10 completed,   0 failed,   1 skipped, 
org.apache.calcite.test.CassandraAdapterDataTypesTest
   
   
..
   
..
   
   > Task :cassandra:test
32.6sec,   11 completed,   0 failed,   0 skipped, 
org.apache.calcite.test.CassandraAdapterTest
   
   Gradle Test Executor 18 STANDARD_OUT
   2020-02-13 09:31:09 [ForkJoinPool-1-worker-1] ERROR 
o.a.cassandra.service.StorageService - Stopping gossiper
   WARNING  83.1sec,   21 completed,   0 failed,   1 skipped, Gradle Test Run 
:cassandra:test
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585513914
 
 
   Gradle build log in machine 2.
   ```
   > Task :cassandra:test
   FAILURE   0.0sec, org.apache.calcite.test.CassandraAdapterTest > 
initializationError
   
   CassandraAdapterTest > initializationError FAILED
   org.junit.jupiter.api.extension.ParameterResolutionException: Failed to 
resolve parameter [com.datastax.driver.core.Session arg0] in method [static 
void 
org.apache.calcite.test.CassandraAdapterTest.load(com.datastax.driver.core.Session)]:
 All host(s) tried for query failed (tried: localhost/0:0:0:0:0:0:0:1:9142 
(com.datastax.driver.core.exceptions.TransportException: 
[localhost/0:0:0:0:0:0:0:1:9142] Cannot connect), localhost/127.0.0.1:9142 
(com.datastax.driver.core.exceptions.TransportException: 
[localhost/127.0.0.1:9142] Cannot connect))
   at 
org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameter(ExecutableInvoker.java:239)
   at 
org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:183)
   at 
org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:144)
   at 
org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:96)
   at 
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllMethods$8(ClassBasedTestDescriptor.java:371)
   at 
org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   at 
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllMethods(ClassBasedTestDescriptor.java:369)
   at 
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:193)
   at 
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:77)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:132)
   at 
org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
   at 
org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
   at 
org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
   at 
org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
   at 
org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.executeNonConcurrentTasks(ForkJoinPoolHierarchicalTestExecutorService.java:141)
   at 
org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:121)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
   at 
org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
   at 
org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
   at 
org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
   at 
org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
   at 
org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
   at 
java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
   at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
   at 
java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
   at 
java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
   at 

[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585513650
 
 
   Build log in machine 1:
   ```
   > Task :cassandra:test
   
   CassandraAdapterDataTypesTest STANDARD_OUT
   2020-02-13 09:30:02 [pool-2-thread-1] ERROR 
o.a.cassandra.service.StartupChecks - cassandra.jmx.local.port missing from 
cassandra-env.sh, unable to start local JMX service.
   
   CassandraAdapterDataTypesTest STANDARD_ERROR
   no sigar-amd64-winnt.dll in java.library.path
   org.hyperic.sigar.SigarException: no sigar-amd64-winnt.dll in 
java.library.path
   at org.hyperic.sigar.Sigar.loadLibrary(Sigar.java:172)
   at org.hyperic.sigar.Sigar.(Sigar.java:100)
   at 
org.apache.cassandra.utils.SigarLibrary.(SigarLibrary.java:47)
   at 
org.apache.cassandra.utils.SigarLibrary.(SigarLibrary.java:28)
   at 
org.apache.cassandra.service.StartupChecks$7.execute(StartupChecks.java:266)
   at 
org.apache.cassandra.service.StartupChecks.verify(StartupChecks.java:125)
   at 
org.apache.cassandra.service.CassandraDaemon.setup(CassandraDaemon.java:200)
   at 
org.apache.cassandra.service.CassandraDaemon.activate(CassandraDaemon.java:602)
   at 
org.cassandraunit.utils.EmbeddedCassandraServerHelper.lambda$startEmbeddedCassandra$1(EmbeddedCassandraServerHelper.java:150)
   at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
   at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
   at java.lang.Thread.run(Thread.java:745)
 4.0sec, org.apache.calcite.test.CassandraAdapterDataTypesTest > 
testCollectionsRowType()
   WARNING  40.0sec,   10 completed,   0 failed,   1 skipped, 
org.apache.calcite.test.CassandraAdapterDataTypesTest
   
   
..
   
..
   
   > Task :cassandra:test
32.6sec,   11 completed,   0 failed,   0 skipped, 
org.apache.calcite.test.CassandraAdapterTest
   
   Gradle Test Executor 18 STANDARD_OUT
   2020-02-13 09:31:09 [ForkJoinPool-1-worker-1] ERROR 
o.a.cassandra.service.StorageService - Stopping gossiper
   WARNING  83.1sec,   21 completed,   0 failed,   1 skipped, Gradle Test Run 
:cassandra:test
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585513458
 
 
   `.toDelete` folder is still there.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585510606
 
 
   By "fails to delete" do you mean throws exception or `.toDelete` folder is 
still there ? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] gr4ve commented on issue #1405: [CALCITE-2772] Support varargs UDF for scalar function

2020-02-12 Thread GitBox
gr4ve commented on issue #1405: [CALCITE-2772] Support varargs UDF for scalar 
function
URL: https://github.com/apache/calcite/pull/1405#issuecomment-585507281
 
 
   I have merged the latest master commit to this branch


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete 
cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585506346
 
 
   @asereda-gs I checked this fix on two Windows OS machines. However, it still 
fails to delete the `.toDelete` file.  Any further information or operations I 
can do to help find root cause?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
DonnyZone commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585506346
 
 
   @asereda-gs I checked this fix on two Windows OS machines. However, it still 
fails to delete the `.toDelete` file.  Any further information or operations I 
can do to help?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] hsyuan closed pull request #1799: [CALCITE-3785] HepPlanner.belongToDag() doesn't have to use mapDigest…

2020-02-12 Thread GitBox
hsyuan closed pull request #1799: [CALCITE-3785] HepPlanner.belongToDag() 
doesn't have to use mapDigest…
URL: https://github.com/apache/calcite/pull/1799
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[calcite] branch master updated: [CALCITE-3785] HepPlanner.belongToDag() doesn't have to use mapDigestToVertex (Xiening Dai)

2020-02-12 Thread hyuan
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new a09923f  [CALCITE-3785] HepPlanner.belongToDag() doesn't have to use 
mapDigestToVertex (Xiening Dai)
a09923f is described below

commit a09923ff8dcb707c5573b37e182308f90dfb52fe
Author: Xiening Dai 
AuthorDate: Tue Feb 11 15:12:20 2020 -0800

[CALCITE-3785] HepPlanner.belongToDag() doesn't have to use 
mapDigestToVertex (Xiening Dai)

To test whether or not a vertex belongs to DAG, we can simply do
graph.vertexSet().contains(vertex). There's no need to look up in
mapDigestToVertex map, which incurs overhead of creating the map key. This
problem was amplified by CALCITE-3713.

Close #1799
---
 .../main/java/org/apache/calcite/plan/hep/HepPlanner.java | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java 
b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
index 71f1c5b..a018b01 100644
--- a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
@@ -498,17 +498,11 @@ public class HepPlanner extends AbstractRelOptPlanner {
 }
   }
 
-  /** Returns whether the vertex is valid. */
-  private boolean belongsToDag(HepRelVertex vertex) {
-Pair> key = key(vertex.getCurrentRel());
-return mapDigestToVertex.get(key) != null;
-  }
-
   private HepRelVertex applyRule(
   RelOptRule rule,
   HepRelVertex vertex,
   boolean forceConversions) {
-if (!belongsToDag(vertex)) {
+if (!graph.vertexSet().contains(vertex)) {
   return null;
 }
 RelTrait parentTrait = null;
@@ -629,6 +623,13 @@ public class HepPlanner extends AbstractRelOptPlanner {
 if (!operand.matches(rel)) {
   return false;
 }
+for (RelNode input : rel.getInputs()) {
+  if (!(input instanceof HepRelVertex)) {
+// The graph could be partially optimized for materialized view. In 
that
+// case, the input would be a RelNode and shouldn't be matched again 
here.
+return false;
+  }
+}
 bindings.add(rel);
 @SuppressWarnings("unchecked")
 List childRels = (List) rel.getInputs();



[GitHub] [calcite] asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378515748
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378515748
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378515110
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378513665
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] vlsi commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378509609
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] vlsi commented on a change in pull request #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1798: [CALCITE-2442] Remove 
.toDelete cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#discussion_r378509030
 
 

 ##
 File path: 
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java
 ##
 @@ -0,0 +1,218 @@
+/*
+ * 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.test;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
+
+import org.apache.cassandra.concurrent.StageManager;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.WindowsFailedSnapshotTracker;
+import org.apache.cassandra.service.CassandraDaemon;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.thrift.transport.TTransportException;
+
+import com.datastax.driver.core.Cluster;
+import com.datastax.driver.core.Session;
+import com.google.common.collect.ImmutableMap;
+
+import org.cassandraunit.utils.EmbeddedCassandraServerHelper;
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionConfigurationException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Locale;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * JUnit5 extension to start and stop embedded cassandra server.
+ *
+ * Note that tests will be skipped if running on JDK11+
+ * (which is not yet supported by cassandra) see
+ *  https://issues.apache.org/jira/browse/CASSANDRA-9608;>CASSANDRA-9608.
+ */
+class CassandraExtension implements ParameterResolver, ExecutionCondition {
+
+  private static final ExtensionContext.Namespace NAMESPACE =
+  ExtensionContext.Namespace.create(CassandraExtension.class);
+
+  private static final String KEY = "cassandra";
+
+  @Override public boolean supportsParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+final Class type = parameterContext.getParameter().getType();
+return Session.class.isAssignableFrom(type) || 
Cluster.class.isAssignableFrom(type);
+  }
+
+  @Override public Object resolveParameter(final ParameterContext 
parameterContext,
+  final ExtensionContext extensionContext) throws 
ParameterResolutionException {
+
+Class type = parameterContext.getParameter().getType();
+if (Session.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).session;
+} else if (Cluster.class.isAssignableFrom(type)) {
+  return getOrCreate(extensionContext).cluster;
+}
+
+throw new ExtensionConfigurationException(
+String.format(Locale.ROOT, "%s supports only %s or %s but yours was 
%s",
+CassandraExtension.class.getSimpleName(),
+Session.class.getName(), Cluster.class.getName(), type.getName()));
+  }
+
+  static ImmutableMap getDataset(String resourcePath) {
+return ImmutableMap.of("model",
+Sources.of(CassandraExtension.class.getResource(resourcePath))
+.file().getAbsolutePath());
+  }
+
+  /**
+   * Register cassandra resource in root context so it can be shared with 
other tests
+   */
+  private static CassandraResource getOrCreate(ExtensionContext context) {
+// same cassandra instance should be shared across all extension instances
+return context.getRoot()
+.getStore(NAMESPACE)
+.getOrComputeIfAbsent(KEY, key -> new CassandraResource(), 
CassandraResource.class);
+  }
+
+  /**
+   * Whether to run this test.
+   * Enabled by default, unless explicitly disabled
+   * from command line ({@code -Dcalcite.test.cassandra=false}) or running on 

[GitHub] [calcite] vlsi commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
vlsi commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp 
folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585413865
 
 
   @asereda-gs , can you please remove this line 
https://github.com/apache/calcite/blob/10a5b8a89d319e6fed563e7e49518cfc960b93d6/cassandra/gradle.properties#L20
 so junit4 is not added to cassandra/../test classpath?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs closed pull request #775: [CALCITE-2442] Cassandra intermittent test failures

2020-02-12 Thread GitBox
asereda-gs closed pull request #775: [CALCITE-2442] Cassandra intermittent test 
failures 
URL: https://github.com/apache/calcite/pull/775
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585406047
 
 
   @DonnyZone  can you pls check now (unfortunately I don't have easy access to 
Windows OS runtime) ? 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs edited a comment on issue #1798: [CALCITE-2442] Remove .toDelete 
cassandra temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585375547
 
 
   I have registered CassandraResource (which implements 
[CloseableResource](https://junit.org/junit5/docs/5.1.0/api/org/junit/jupiter/api/extension/ExtensionContext.Store.CloseableResource.html))
 in root context.
   
   It seems to work.
   
   JUnit5 allows more flexibility with extensions and context store.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585375547
 
 
   I have registered CassandraResource (which implements 
[CloseableResource](https://junit.org/junit5/docs/5.1.0/api/org/junit/jupiter/api/extension/ExtensionContext.Store.CloseableResource.html))
 in root context.
   
   It seems to work.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
vlsi commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp 
folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585374414
 
 
   > What I need is a callback after all tests have been executed
   
   I don't think JUnit has a callback "after all tests".
   So there are multiple solutions:
   
   a) use TestContainers
   b) use "after each test class" callback
   
   
   A workaround might be to use a single class for all the tests, then 
"afterclass" would be the same ase "after all cassandra tests".


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra temp folder on Windows after tests

2020-02-12 Thread GitBox
asereda-gs commented on issue #1798: [CALCITE-2442] Remove .toDelete cassandra 
temp folder on Windows after tests
URL: https://github.com/apache/calcite/pull/1798#issuecomment-585370715
 
 
   @DonnyZone the issue is that cassandra daemon is not properly shutdown after 
tests so some files are locked by cassandra service.
   
   I have migrated Cassandra tests to JUnit5 which allows better control of 
resources (before/after callbacks). What I need is a callback after *all* tests 
have been executed. 
   
   Testing now


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] julianhyde commented on issue #1706: CALCITE-3770, minor change "EnumerableCalcRel" appears in code to "EnumerableCalc".

2020-02-12 Thread GitBox
julianhyde commented on issue #1706: CALCITE-3770, minor change 
"EnumerableCalcRel" appears in code to "EnumerableCalc".
URL: https://github.com/apache/calcite/pull/1706#issuecomment-585365757
 
 
   > And one thing I want to for sure, do you think it needs to be modified in 
history.md? IMHO, we shouldn't.
   
   I agree. Don't change history. It is (basically) a log of commit messages.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] dhirenda-gautam commented on a change in pull request #1785: [CALCITE-3771] TRIM Support for HIVE/SPARK Dialect

2020-02-12 Thread GitBox
dhirenda-gautam commented on a change in pull request #1785: [CALCITE-3771] 
TRIM Support for HIVE/SPARK Dialect
URL: https://github.com/apache/calcite/pull/1785#discussion_r378260941
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java
 ##
 @@ -102,10 +108,121 @@ public SparkSqlDialect(SqlDialect.Context context) {
 timeUnitNode.getParserPosition());
 SqlFloorFunction.unparseDatetimeFunction(writer, call2, "DATE_TRUNC", 
false);
 break;
-
+  case TRIM:
+unparseTrim(writer, call, leftPrec, rightPrec);
+break;
   default:
 super.unparseCall(writer, call, leftPrec, rightPrec);
   }
 }
   }
+
+  /**
+   * For usage of TRIM, LTRIM and RTRIM in Spark
+   */
+  private void unparseTrim(
+  SqlWriter writer, SqlCall call, int leftPrec,
+  int rightPrec) {
+SqlLiteral valueToBeTrim = call.operand(1);
+if (valueToBeTrim.toValue().matches("\\s+")) {
+  handleTrimWithSpace(writer, call, leftPrec, rightPrec);
+} else {
+  handleTrimWithChar(writer, call, leftPrec, rightPrec);
+}
+  }
+
+  /**
+   * This method will handle the TRIM function if the value to be trimmed is 
space
+   * Below is an example :
+   * INPUT : SELECT TRIM(both ' ' from "ABC")
+   * OUPUT : SELECT TRIM(ABC)
+   * @param writer Target SqlWriter to write the call
+   * @param call SqlCall
+   * @param leftPrec Indicate left precision
+   * @param rightPrec Indicate Right precision
+   */
+  private void handleTrimWithSpace(
+  SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
+final String operatorName;
+SqlLiteral trimFlag = call.operand(0);
+switch (trimFlag.getValueAs(SqlTrimFunction.Flag.class)) {
+case LEADING:
+  operatorName = "LTRIM";
+  break;
+case TRAILING:
+  operatorName = "RTRIM";
+  break;
+default:
+  operatorName = call.getOperator().getName();
+  break;
+}
+final SqlWriter.Frame trimFrame = writer.startFunCall(operatorName);
+call.operand(2).unparse(writer, leftPrec, rightPrec);
+writer.endFunCall(trimFrame);
+  }
+
+  /**
+   * This method will handle the TRIM function if the value to be trimmed is 
not space
+   * Below is an example :
+   * INPUT : SELECT TRIM(both 'A' from "ABC")
+   * OUPUT : SELECT REGEXP_REPLACE("ABC", '^(A)*', '')
+   * @param writer Target SqlWriter to write the call
+   * @param call SqlCall
+   * @param leftPrec Indicate left precision
+   * @param rightPrec Indicate Right precision
+   */
+  private void handleTrimWithChar(
+  SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
+SqlLiteral trimFlag = call.operand(0);
+SqlCharStringLiteral regexNode = makeRegexNodeFromCall(call.operand(1), 
trimFlag);
+SqlCharStringLiteral blankLiteral = SqlLiteral.createCharString("",
+call.getParserPosition());
+SqlNode[] trimOperands = new SqlNode[]{call.operand(2), regexNode, 
blankLiteral};
+SqlCall regexReplaceCall = new SqlBasicCall(REGEXP_REPLACE, trimOperands, 
SqlParserPos.ZERO);
+REGEXP_REPLACE.unparse(writer, regexReplaceCall, leftPrec, rightPrec);
+  }
+
+  /**
+   * This method will make regex pattern based on the TRIM flag
+   *
+   * @param call SqlCall contains the values that needs to be trimmed
+   * @param trimFlag It will contain the trimFlag either BOTH,LEADING or 
TRAILING
+   * @return It will return the regex pattern of the character to be trimmed.
+   */
+  private SqlCharStringLiteral makeRegexNodeFromCall(SqlNode call, SqlLiteral 
trimFlag) {
+String regexPattern = ((SqlCharStringLiteral) call).toValue();
+regexPattern = escapeSpecialChar(regexPattern);
+switch (trimFlag.getValueAs(SqlTrimFunction.Flag.class)) {
+case LEADING:
+  regexPattern = "^(".concat(regexPattern).concat(")*");
+  break;
+case TRAILING:
+  regexPattern = "(".concat(regexPattern).concat(")*$");
+  break;
+default:
 
 Review comment:
   @danny0405 I have moved the common code to a new class RelToSqlConverterUtil


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] Aaaaaaron edited a comment on issue #1706: CALCITE-3770, minor change "EnumerableCalcRel" appears in code to "EnumerableCalc".

2020-02-12 Thread GitBox
Aaron edited a comment on issue #1706: CALCITE-3770, minor change 
"EnumerableCalcRel" appears in code to "EnumerableCalc".
URL: https://github.com/apache/calcite/pull/1706#issuecomment-585187498
 
 
   @zabetak OK, thanks a lot, I'll give a look.
   And one thing I want to for sure, do you think it needs to be modified in 
history.md? IMHO, we shouldn't.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] Aaaaaaron edited a comment on issue #1706: CALCITE-3770, minor change "EnumerableCalcRel" appears in code to "EnumerableCalc".

2020-02-12 Thread GitBox
Aaron edited a comment on issue #1706: CALCITE-3770, minor change 
"EnumerableCalcRel" appears in code to "EnumerableCalc".
URL: https://github.com/apache/calcite/pull/1706#issuecomment-585187498
 
 
   @zabetak OK, thanks a lot, I'll give a look.
   And one thing I want to for sure, do you think it needs to be modified in 
history.md(IMHO, we shouldn't)?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] Aaaaaaron edited a comment on issue #1706: CALCITE-3770, minor change "EnumerableCalcRel" appears in code to "EnumerableCalc".

2020-02-12 Thread GitBox
Aaron edited a comment on issue #1706: CALCITE-3770, minor change 
"EnumerableCalcRel" appears in code to "EnumerableCalc".
URL: https://github.com/apache/calcite/pull/1706#issuecomment-585187498
 
 
   @zabetak OK, thanks a lot, I'll give a look.
   And one thing I want to for sure, do you think it needs to be modified in 
history.md?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1790: [CALCITE-3769] Deprecate TableScanRule

2020-02-12 Thread GitBox
danny0405 commented on a change in pull request #1790: [CALCITE-3769] Deprecate 
TableScanRule
URL: https://github.com/apache/calcite/pull/1790#discussion_r378245164
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/tools/RelRunners.java
 ##
 @@ -29,6 +35,19 @@ private RelRunners() {}
 
   /** Runs a relational expression by creating a JDBC connection. */
   public static PreparedStatement run(RelNode rel) {
+final RelShuttle shuttle = new RelHomogeneousShuttle() {
+  @Override public RelNode visit(TableScan scan) {
+final RelOptTable table = scan.getTable();
+if (scan instanceof LogicalTableScan
+&& Bindables.BindableTableScan.canHandle(table)) {
+  // Always replace the LogicalTableScan with BindableTableScan
+  // because it's implementation does not require a "schema" as 
context.
+  return Bindables.BindableTableScan.create(scan.getCluster(), table);
 
 Review comment:
   The logic is the same compared to the old one. The RelRunners always run a 
logical plan.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378244829
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   It seems to be difficult to achieve both efficiency and elegant stlye. Maybe 
some functional languages (e.g., Scala) provide sugar :).


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378237911
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   However, it looks like this case equally bad in both styles :(
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378237911
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   However, it looks like this case looks equally bad in both styles :(
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378236745
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   Ok, thanks for clarification.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378233204
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   The efficiency does not matter here.
   
   I think we should refrain from deep {.. {..} } structures.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378229570
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   Is the following code style more efficient than current implementation? If 
`xxx==null`, it also returns `null` fast willout doing extra work.
   ```
   if (schemaName2 == null) {
 return null;
   }
   final Schema s = schema.getSubSchema(schemaName2);
   if (s == null) {
 return null;
   }
   return new SimpleCalciteSchema(this, s, schemaName2);
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378229570
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   Is the following code style more efficient?
   ```
   if (schemaName2 == null) {
 return null;
   }
   final Schema s = schema.getSubSchema(schemaName2);
   if (s == null) {
 return null;
   }
   return new SimpleCalciteSchema(this, s, schemaName2);
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] Aaaaaaron commented on issue #1706: CALCITE-3770, minor change "EnumerableCalcRel" appears in code to "EnumerableCalc".

2020-02-12 Thread GitBox
Aaron commented on issue #1706: CALCITE-3770, minor change 
"EnumerableCalcRel" appears in code to "EnumerableCalc".
URL: https://github.com/apache/calcite/pull/1706#issuecomment-585187498
 
 
   @zabetak OK, thanks a lot, I'll give a look.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378214708
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,67 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+final String schemaName2 = caseSensitive ? schemaName : 
caseInsensitiveLookup(
+schema.getSubSchemaNames(), schemaName);
+if (schemaName2 != null) {
+  final Schema s = schema.getSubSchema(schemaName2);
 
 Review comment:
   Can you please use fast return here?
   That is `if (... == null) { return 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378211336
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,79 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+if (caseSensitive) {
+  Schema s = schema.getSubSchema(schemaName);
+  if (s != null) {
+return new SimpleCalciteSchema(this, s, schemaName);
+  }
+} else {
+  final String schemaName2 = caseInsensitiveLookup(
+  schema.getSubSchemaNames(), schemaName);
+  if (schemaName2 != null) {
+return new SimpleCalciteSchema(this,
+schema.getSubSchema(schemaName2), schemaName2);
 
 Review comment:
   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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-12 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-585166370
 
 
   I've created https://issues.apache.org/jira/browse/CALCITE-3786 for Digest.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378198948
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +69,79 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+// Exact string lookup
+if (candidates.contains(name)) {
+  return name;
+}
+// Upper case string lookup
+final String upperCaseName = name.toUpperCase(Locale.ROOT);
+if (candidates.contains(upperCaseName)) {
+  return upperCaseName;
+}
+// Lower case string lookup
+final String lowerCaseName = name.toLowerCase(Locale.ROOT);
+if (candidates.contains(lowerCaseName)) {
+  return lowerCaseName;
+}
+// Fall through: Set iteration
+for (String candidate: candidates) {
+  if (candidate.equalsIgnoreCase(name)) {
+return candidate;
+  }
+}
+return null;
+  }
+
   protected CalciteSchema getImplicitSubSchema(String schemaName,
   boolean caseSensitive) {
 // Check implicit schemas.
-Schema s = schema.getSubSchema(schemaName);
-if (s != null) {
-  return new SimpleCalciteSchema(this, s, schemaName);
+if (caseSensitive) {
+  Schema s = schema.getSubSchema(schemaName);
+  if (s != null) {
+return new SimpleCalciteSchema(this, s, schemaName);
+  }
+} else {
+  final String schemaName2 = caseInsensitiveLookup(
+  schema.getSubSchemaNames(), schemaName);
+  if (schemaName2 != null) {
+return new SimpleCalciteSchema(this,
+schema.getSubSchema(schemaName2), schemaName2);
 
 Review comment:
   This looks like code duplication.
   Can you please factor that out of `if` statement?
   
   For instance:
   1. determine schema name
   2. return candidate


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1790: [CALCITE-3769] Deprecate TableScanRule

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1790: [CALCITE-3769] Deprecate 
TableScanRule
URL: https://github.com/apache/calcite/pull/1790#discussion_r378197263
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/tools/RelRunners.java
 ##
 @@ -29,6 +35,19 @@ private RelRunners() {}
 
   /** Runs a relational expression by creating a JDBC connection. */
   public static PreparedStatement run(RelNode rel) {
+final RelShuttle shuttle = new RelHomogeneousShuttle() {
+  @Override public RelNode visit(TableScan scan) {
+final RelOptTable table = scan.getTable();
+if (scan instanceof LogicalTableScan
+&& Bindables.BindableTableScan.canHandle(table)) {
+  // Always replace the LogicalTableScan with BindableTableScan
+  // because it's implementation does not require a "schema" as 
context.
+  return Bindables.BindableTableScan.create(scan.getCluster(), table);
 
 Review comment:
   Can the clients of Relrunners do the appropriate conversion instead?
   
   org.apache.calcite.tools.RelRunners#run API says nothing on what changes 
would it make to the input rel node.
   It might be very unexpected for those who see just the call to 
`RelRunners.run(root))`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
DonnyZone commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378184129
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +68,64 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+for (String candidate: candidates) {
 
 Review comment:
   I agree. It makes sense for most frequent cases, i.e., upper case or lower 
case.
   I will add this optimization soon. Thanks!


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] danny0405 commented on a change in pull request #1790: [CALCITE-3769] Deprecate TableScanRule

2020-02-12 Thread GitBox
danny0405 commented on a change in pull request #1790: [CALCITE-3769] Deprecate 
TableScanRule
URL: https://github.com/apache/calcite/pull/1790#discussion_r378172547
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/tools/RelRunners.java
 ##
 @@ -29,6 +35,19 @@ private RelRunners() {}
 
   /** Runs a relational expression by creating a JDBC connection. */
   public static PreparedStatement run(RelNode rel) {
+final RelShuttle shuttle = new RelHomogeneousShuttle() {
+  @Override public RelNode visit(TableScan scan) {
+final RelOptTable table = scan.getTable();
+if (scan instanceof LogicalTableScan
+&& Bindables.BindableTableScan.canHandle(table)) {
+  // Always replace the LogicalTableScan with BindableTableScan
+  // because it's implementation does not require a "schema" as 
context.
+  return Bindables.BindableTableScan.create(scan.getCluster(), table);
 
 Review comment:
   Because the Enumerables implementation needs a "schema" to look up for the 
table. but the RelRunners does not actually have a schema, the Bindables 
actually can implement that.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
vlsi commented on a change in pull request #1792: [CALCITE-3775] Implicit 
lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#discussion_r378167122
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java
 ##
 @@ -67,31 +68,64 @@ public CalciteSchema add(String name, Schema schema) {
 return calciteSchema;
   }
 
+  private String caseInsensitiveLookup(Set candidates, String name) {
+for (String candidate: candidates) {
 
 Review comment:
   I suggest it should try:
   1) exact lookup
   2) name.toUpperCase() lookup
   3) name.toLowerCase() lookup
   
   otherwise => iterate over the set.
   
   @DonnyZone , WDYT?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] zabetak commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-12 Thread GitBox
zabetak commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-585133377
 
 
   Since https://issues.apache.org/jira/browse/CALCITE-3785 was logged for this 
purpose, I guess we should continue the discussion there. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] zabetak commented on issue #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter

2020-02-12 Thread GitBox
zabetak commented on issue #1792: [CALCITE-3775] Implicit lookup methods in 
SimpleCalciteSchema ignore case sensitivity parameter
URL: https://github.com/apache/calcite/pull/1792#issuecomment-585128426
 
 
   Although I do care a lot about performance the current implementation in 
`SimpleCalciteSchema` is broken and I think this is more important. I would 
suggest to discuss possible changes to the `Schema` API under a dedicated JIRA 
and not block this PR. Discussing this kind of things in a PR makes it more 
difficult to find the trace afterwards.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[calcite] branch master updated: [CALCITE-3783] PruneEmptyRules#JOIN_RIGHT_INSTANCE wrong behavior for JoinRelType.ANTI

2020-02-12 Thread rubenql
This is an automated email from the ASF dual-hosted git repository.

rubenql 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 10a5b8a  [CALCITE-3783] PruneEmptyRules#JOIN_RIGHT_INSTANCE wrong 
behavior for JoinRelType.ANTI
10a5b8a is described below

commit 10a5b8a89d319e6fed563e7e49518cfc960b93d6
Author: rubenada 
AuthorDate: Tue Feb 11 15:00:09 2020 +0100

[CALCITE-3783] PruneEmptyRules#JOIN_RIGHT_INSTANCE wrong behavior for 
JoinRelType.ANTI
---
 .../apache/calcite/rel/rules/PruneEmptyRules.java  |  15 ++
 .../org/apache/calcite/test/RelOptRulesTest.java   | 245 -
 .../org/apache/calcite/test/RelOptRulesTest.xml| 213 +-
 3 files changed, 464 insertions(+), 9 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java 
b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
index 5e68a6f..a8838d8 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
@@ -26,6 +26,7 @@ import org.apache.calcite.rel.SingleRel;
 import org.apache.calcite.rel.core.Aggregate;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.core.Project;
 import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.core.Sort;
@@ -286,6 +287,9 @@ public abstract class PruneEmptyRules {
*
* 
* Join(Empty, Scan(Dept), INNER) becomes Empty
+   * Join(Empty, Scan(Dept), LEFT) becomes Empty
+   * Join(Empty, Scan(Dept), SEMI) becomes Empty
+   * Join(Empty, Scan(Dept), ANTI) becomes Empty
* 
*/
   public static final RelOptRule JOIN_LEFT_INSTANCE =
@@ -314,6 +318,9 @@ public abstract class PruneEmptyRules {
*
* 
* Join(Scan(Emp), Empty, INNER) becomes Empty
+   * Join(Scan(Emp), Empty, RIGHT) becomes Empty
+   * Join(Scan(Emp), Empty, SEMI) becomes Empty
+   * Join(Scan(Emp), Empty, ANTI) becomes Scan(Emp)
* 
*/
   public static final RelOptRule JOIN_RIGHT_INSTANCE =
@@ -330,6 +337,14 @@ public abstract class PruneEmptyRules {
 // dept is empty
 return;
   }
+  if (join.getJoinType() == JoinRelType.ANTI) {
+// "select * from emp anti join dept" is not necessarily empty if 
dept is empty
+if (join.analyzeCondition().isEqui()) {
+  // In case of anti (equi) join: Join(X, Empty, ANTI) becomes X
+  call.transformTo(join.getLeft());
+}
+return;
+  }
   call.transformTo(call.builder().push(join).empty().build());
 }
   };
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 01bcff8..6b02ab4 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -3040,7 +3040,7 @@ public class RelOptRulesTest extends RelOptTestBase {
 sql(sql).with(program).check();
   }
 
-  @Test public void testEmptyJoin() {
+  @Test public void testLeftEmptyInnerJoin() {
 HepProgram program = new HepProgramBuilder()
 .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
 .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
@@ -3055,7 +3055,7 @@ public class RelOptRulesTest extends RelOptTestBase {
 sql(sql).with(program).check();
   }
 
-  @Test public void testEmptyJoinLeft() {
+  @Test public void testLeftEmptyLeftJoin() {
 HepProgram program = new HepProgramBuilder()
 .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
 .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
@@ -3070,7 +3070,7 @@ public class RelOptRulesTest extends RelOptTestBase {
 sql(sql).with(program).check();
   }
 
-  @Test public void testEmptyJoinRight() {
+  @Test public void testLeftEmptyRightJoin() {
 HepProgram program = new HepProgramBuilder()
 .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
 .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
@@ -3078,7 +3078,7 @@ public class RelOptRulesTest extends RelOptTestBase {
 .addRuleInstance(PruneEmptyRules.JOIN_RIGHT_INSTANCE)
 .build();
 
-// Plan should be equivalent to "select * from emp join dept".
+// Plan should be equivalent to "select * from emp right join dept".
 // Cannot optimize away the join because of RIGHT.
 final String sql = "select * from (\n"
 + "  select * from emp where false) e\n"
@@ -3086,6 +3086,243 @@ public class RelOptRulesTest extends RelOptTestBase {
 sql(sql).with(program).check();
   }
 
+  @Test public void testLeftEmptyFullJoin() {
+HepProgram program = new HepProgramBuilder()
+ 

[GitHub] [calcite] rubenada merged pull request #1796: [CALCITE-3783] PruneEmptyRules#JOIN_RIGHT_INSTANCE wrong behavior for JoinRelType.ANTI

2020-02-12 Thread GitBox
rubenada merged pull request #1796: [CALCITE-3783] 
PruneEmptyRules#JOIN_RIGHT_INSTANCE wrong behavior for JoinRelType.ANTI
URL: https://github.com/apache/calcite/pull/1796
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services