[GitHub] [calcite] danny0405 commented on issue #1803: [CALCITE-3793] AssertionError throws for planner digest cache key whe…
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…
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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…
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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".
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
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".
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".
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".
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
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
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
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
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
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
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
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
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".
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
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
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
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
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
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
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
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
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
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
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
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
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