[GitHub] [calcite] danny0405 commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled
danny0405 commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584#discussion_r346695179 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -877,6 +877,10 @@ public RelSubset ensureRegistered(RelNode rel, RelNode equivRel) { * Checks internal consistency. */ protected boolean isValid(Litmus litmus) { +if (this.getRoot() == null) { + return true; Review comment: I have left the comment in the commit page. 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] amaliujia opened a new pull request #1588: [CALCITE-3381] Unparse to correct BigQuery interval syntax: "INTERVAL…
amaliujia opened a new pull request #1588: [CALCITE-3381] Unparse to correct BigQuery interval syntax: "INTERVAL… URL: https://github.com/apache/calcite/pull/1588 … int64 time_unit". Range time unit is not supported yet by BigQuery. 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] amaliujia commented on issue #1510: [CALCITE-3418] Grouped Window Function TUMBLE should return "$TUMBLE"…
amaliujia commented on issue #1510: [CALCITE-3418] Grouped Window Function TUMBLE should return "$TUMBLE"… URL: https://github.com/apache/calcite/pull/1510#issuecomment-554228355 This patch is a part of https://github.com/apache/calcite/pull/1587 already. 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] amaliujia closed pull request #1457: [WIP][Prototype][CALCITE-3340] TUMBLE as a TableFunction
amaliujia closed pull request #1457: [WIP][Prototype][CALCITE-3340] TUMBLE as a TableFunction URL: https://github.com/apache/calcite/pull/1457 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] amaliujia commented on issue #1457: [WIP][Prototype][CALCITE-3340] TUMBLE as a TableFunction
amaliujia commented on issue #1457: [WIP][Prototype][CALCITE-3340] TUMBLE as a TableFunction URL: https://github.com/apache/calcite/pull/1457#issuecomment-554228194 Will close this PR. The progress now is in https://github.com/apache/calcite/pull/1587. 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 commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled
hsyuan commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584#discussion_r346640964 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -877,6 +877,10 @@ public RelSubset ensureRegistered(RelNode rel, RelNode equivRel) { * Checks internal consistency. */ protected boolean isValid(Litmus litmus) { +if (this.getRoot() == null) { + return true; Review comment: Thanks for reviewing. We can revert the change and reopen the issue if you would like to. 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 a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if…
julianhyde commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if… URL: https://github.com/apache/calcite/pull/1586#discussion_r346639635 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -1081,60 +1072,6 @@ private RelNode changeTraitsUsingConverters( return converted; } - /** - * Converts traits using well-founded induction. We don't require that - * each conversion preserves all traits that have previously been converted, - * but if it changes "locked in" traits we'll try some other conversion. - * - * @param rel Relational expression - * @param allowInfiniteCostConverters Whether to allow infinite converters - * @param toTraitsTarget trait set - * @param usedTraits Traits that have been locked in - * @return Converted relational expression - */ - private RelNode completeConversion( - RelNode rel, - boolean allowInfiniteCostConverters, - RelTraitSet toTraits, - Expressions.FluentList usedTraits) { -if (true) { - return rel; Review comment: I already replied in the jira case 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 #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled
danny0405 commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584#discussion_r346616585 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -877,6 +877,10 @@ public RelSubset ensureRegistered(RelNode rel, RelNode equivRel) { * Checks internal consistency. */ protected boolean isValid(Litmus litmus) { +if (this.getRoot() == null) { + return true; Review comment: BTW, next time we fix the code, can we try to give some tests ? Without the test, no one can make sure how the code works and why it should be fixed like 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] danny0405 commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if…
danny0405 commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if… URL: https://github.com/apache/calcite/pull/1586#discussion_r346616014 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -1081,60 +1072,6 @@ private RelNode changeTraitsUsingConverters( return converted; } - /** - * Converts traits using well-founded induction. We don't require that - * each conversion preserves all traits that have previously been converted, - * but if it changes "locked in" traits we'll try some other conversion. - * - * @param rel Relational expression - * @param allowInfiniteCostConverters Whether to allow infinite converters - * @param toTraitsTarget trait set - * @param usedTraits Traits that have been locked in - * @return Converted relational expression - */ - private RelNode completeConversion( - RelNode rel, - boolean allowInfiniteCostConverters, - RelTraitSet toTraits, - Expressions.FluentList usedTraits) { -if (true) { - return rel; Review comment: Please hold on for the fix, i'm curious if this logic is left there intentionally. @julianhyde Can you please take a look at this code ? There are many places in Calcite with the special "useless" `if(true)` logic, for me, it makes the logic more clear because it illustrates how the logic evolves. 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 #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled
danny0405 commented on a change in pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584#discussion_r346615063 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -877,6 +877,10 @@ public RelSubset ensureRegistered(RelNode rel, RelNode equivRel) { * Checks internal consistency. */ protected boolean isValid(Litmus litmus) { +if (this.getRoot() == null) { + return true; Review comment: -1 for this fix 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] amaliujia opened a new pull request #1587: [CALCITE-3382] Support TUMBLE as Table Value Function
amaliujia opened a new pull request #1587: [CALCITE-3382] Support TUMBLE as Table Value Function URL: https://github.com/apache/calcite/pull/1587 This PR add TUMBLE as table value function and also add stream.iq along with Enumerable implementation. This is a big PR that actually is related to the following JIRAs: https://jira.apache.org/jira/browse/CALCITE-3340 https://jira.apache.org/jira/browse/CALCITE-3501 https://jira.apache.org/jira/browse/CALCITE-3499 https://jira.apache.org/jira/browse/CALCITE-3418 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 a change in pull request #1556: [CALCITE-3438] Validator should disallow use of the GROUPING function inside a FILTER clause
julianhyde commented on a change in pull request #1556: [CALCITE-3438] Validator should disallow use of the GROUPING function inside a FILTER clause URL: https://github.com/apache/calcite/pull/1556#discussion_r346602042 ## File path: core/src/main/java/org/apache/calcite/sql/validate/AggVisitor.java ## @@ -44,6 +44,9 @@ * or group auxiliary functions (e.g. {@code TUMBLE_START}). */ protected final boolean group; protected final SqlNameMatcher nameMatcher; + /** Whether to find grouping functions (i.e., {@code GROUP_ID}, + * {@code GROUPING_ID}, {@code GROUPING}).*/ + private boolean grouping = false; Review comment: I don't think the `grouping` field is necessary. If `AggFinder` treats grouping functions (e.g. GROUP_ID) as aggregate functions (e.g. SUM) by default, we get what we need. 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 merged pull request #1582: Bump geode-core from 1.9.2 to 1.10.0
asereda-gs merged pull request #1582: Bump geode-core from 1.9.2 to 1.10.0 URL: https://github.com/apache/calcite/pull/1582 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 (1667c0d -> 956905b)
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 1667c0d [CALCITE-3494] Support decimal type aggregate in Interpreter (Wang Yanlin) add 956905b Bump geode-core from 1.9.2 to 1.10.0 No new revisions were added by this update. Summary of changes: pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[GitHub] [calcite] hsyuan commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if…
hsyuan commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if… URL: https://github.com/apache/calcite/pull/1586#discussion_r346569044 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -1054,10 +1053,6 @@ private RelNode changeTraitsUsingConverters( allowInfiniteCostConverters); if (rel != null) { assert rel.getTraitSet().getTrait(traitDef).satisfies(toTrait); -rel = -completeConversion( -rel, allowInfiniteCostConverters, toTraits, -Expressions.list(traitDef)); if (rel != null) { register(rel, converted); } Review comment: https://github.com/apache/calcite/pull/1586/files#diff-70d5035797baebe157bab8cbf860e456R1061 can be changed to `else if` and remove `rel == null` check. 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 commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if…
hsyuan commented on a change in pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if… URL: https://github.com/apache/calcite/pull/1586#discussion_r346568160 ## File path: core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java ## @@ -1054,10 +1053,6 @@ private RelNode changeTraitsUsingConverters( allowInfiniteCostConverters); if (rel != null) { assert rel.getTraitSet().getTrait(traitDef).satisfies(toTrait); -rel = -completeConversion( -rel, allowInfiniteCostConverters, toTraits, -Expressions.list(traitDef)); if (rel != null) { Review comment: We don't need this check anymore. 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 (5999578 -> 1667c0d)
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 5999578 Improve folder detection logic in DocumentationTest add 1667c0d [CALCITE-3494] Support decimal type aggregate in Interpreter (Wang Yanlin) No new revisions were added by this update. Summary of changes: .../apache/calcite/interpreter/AggregateNode.java | 67 -- .../apache/calcite/interpreter/Interpreter.java| 5 +- .../org/apache/calcite/test/InterpreterTest.java | 7 +++ 3 files changed, 74 insertions(+), 5 deletions(-)
[GitHub] [calcite] hsyuan merged pull request #1572: [CALCITE-3494] Support decimal type aggregate in Interpreter
hsyuan merged pull request #1572: [CALCITE-3494] Support decimal type aggregate in Interpreter URL: https://github.com/apache/calcite/pull/1572 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] xndai commented on issue #1543: [CALCITE-3455] Redundant rule firing for both logical and physical nodes
xndai commented on issue #1543: [CALCITE-3455] Redundant rule firing for both logical and physical nodes URL: https://github.com/apache/calcite/pull/1543#issuecomment-554097058 > Have you seen https://issues.apache.org/jira/browse/CALCITE-2223 ? > I think 3455 duplicates 2223, and, unfortunately, there's no trivial solution for it. I don't think this is a duplication. This patch tries to improve the optimization performance by avoid firing redundant rules. It's not particularly related to cycles between RelSubset. 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] xndai commented on a change in pull request #1543: [CALCITE-3455] Redundant rule firing for both logical and physical nodes
xndai commented on a change in pull request #1543: [CALCITE-3455] Redundant rule firing for both logical and physical nodes URL: https://github.com/apache/calcite/pull/1543#discussion_r346563987 ## File path: core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java ## @@ -60,7 +62,7 @@ * {@link org.apache.calcite.rel.core.Correlate} from being de-correlated. */ public static final FilterProjectTransposeRule INSTANCE = - new FilterProjectTransposeRule(Filter.class, Project.class, true, true, + new FilterProjectTransposeRule(LogicalFilter.class, LogicalProject.class, true, true, Review comment: This is a good idea. I will try that. Thanks @rubenada 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] xndai opened a new pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if…
xndai opened a new pull request #1586: [CALCITE-3491] VolcanoPlanner.completeConversion() is bypassed by "if… URL: https://github.com/apache/calcite/pull/1586 … (true) return;" Remove unused function. 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 (7341708 -> 5999578)
This is an automated email from the ASF dual-hosted git repository. vladimirsitnikov pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 7341708 [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled (Xiening Dai) add 5999578 Improve folder detection logic in DocumentationTest No new revisions were added by this update. Summary of changes: .../apache/calcite/sql/test/DocumentationTest.java | 46 +++--- 1 file changed, 31 insertions(+), 15 deletions(-)
[GitHub] [calcite] julianhyde commented on issue #1582: Bump geode-core from 1.9.2 to 1.10.0
julianhyde commented on issue #1582: Bump geode-core from 1.9.2 to 1.10.0 URL: https://github.com/apache/calcite/pull/1582#issuecomment-554045544 I think these PRs from dependabot count as "small", therefore no JIRA is necessary. However, it is worth doing a build on each JDK version and generating javadoc. If a JIRA makes that easier, go ahead. 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 #1582: Bump geode-core from 1.9.2 to 1.10.0
asereda-gs commented on issue #1582: Bump geode-core from 1.9.2 to 1.10.0 URL: https://github.com/apache/calcite/pull/1582#issuecomment-554044223 @julianhyde will do. as a general approach should we create a JRIA ticket for this PR ? 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 #1582: Bump geode-core from 1.9.2 to 1.10.0
julianhyde commented on issue #1582: Bump geode-core from 1.9.2 to 1.10.0 URL: https://github.com/apache/calcite/pull/1582#issuecomment-554028976 @asereda-gs can you take a look at this? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[calcite] branch master updated (4e7f7a3 -> 7341708)
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. discard 4e7f7a3 [CALCITE-3530] NPE at VolcanoPlanner#isValid when DEBUG is enabled add 7341708 [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled (Xiening Dai) This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (4e7f7a3) \ N -- N -- N refs/heads/master (7341708) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes:
[calcite] branch master updated (472523c -> 4e7f7a3)
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 472523c Use proper ClassLoader in SparkHandlerImpl add 4e7f7a3 [CALCITE-3530] NPE at VolcanoPlanner#isValid when DEBUG is enabled No new revisions were added by this update. Summary of changes: .../src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java | 4 1 file changed, 4 insertions(+)
[GitHub] [calcite] hsyuan merged pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled
hsyuan merged pull request #1584: [CALCITE-3503] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584 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] betogontijo opened a new pull request #1585: Fixing function MOD
betogontijo opened a new pull request #1585: Fixing function MOD URL: https://github.com/apache/calcite/pull/1585 Returning int value if second operator is int 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 (47b70d2 -> 472523c)
This is an automated email from the ASF dual-hosted git repository. vladimirsitnikov pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 47b70d2 [CALCITE-3502] Upgrade Geode dependency 1.6.0 -> 1.9.2 add e6b2df1 Use Class#getResource in FileReaderTest instead of hard-coding file name add ac5ed01 Ignore TpcdsLatticeSuggesterTest because it does not work add 472523c Use proper ClassLoader in SparkHandlerImpl No new revisions were added by this update. Summary of changes: .../calcite/adapter/file/FileReaderTest.java | 50 -- .../org/apache/calcite/adapter/file/SqlTest.java | 3 +- .../materialize/TpcdsLatticeSuggesterTest.java | 3 ++ .../calcite/adapter/spark/SparkHandlerImpl.java| 7 ++- 4 files changed, 28 insertions(+), 35 deletions(-)
[calcite-avatica] branch master updated: Bump release plugin 1.44.0 -> 1.45.0: do not require GPG key for publishDist
This is an automated email from the ASF dual-hosted git repository. vladimirsitnikov pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git The following commit(s) were added to refs/heads/master by this push: new ed05329 Bump release plugin 1.44.0 -> 1.45.0: do not require GPG key for publishDist ed05329 is described below commit ed05329a1f54518256e1179b909170a00fa0b0b8 Author: Vladimir Sitnikov AuthorDate: Thu Nov 14 17:44:04 2019 +0300 Bump release plugin 1.44.0 -> 1.45.0: do not require GPG key for publishDist see https://github.com/vlsi/vlsi-release-plugins/issues/26 --- gradle.properties | 12 settings.gradle.kts | 19 --- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/gradle.properties b/gradle.properties index 362d895..ea5176a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -21,13 +21,17 @@ kotlin.parallel.tasks.in.project=true # Release version can be generated by using -Prelease or -Prc= arguments calcite.avatica.version=1.16.0 +# The options below configures the use of local clone (e.g. testing development versions) +# You can pass un-comment it, or pass option -PlocalReleasePlugins, or -PlocalReleasePlugins= +# localReleasePlugins=../vlsi-release-plugins + # Plugins com.github.johnrengelman.shadow.version=5.1.0 com.github.spotbugs.version=2.0.0 -com.github.vlsi.crlf.version=1.44.0 -com.github.vlsi.ide.version=1.44.0 -com.github.vlsi.license-gather.version=1.44.0 -com.github.vlsi.stage-vote-release.version=1.44.0 +com.github.vlsi.crlf.version=1.45.0 +com.github.vlsi.ide.version=1.45.0 +com.github.vlsi.license-gather.version=1.45.0 +com.github.vlsi.stage-vote-release.version=1.45.0 com.google.protobuf.version=0.8.10 de.thetaphi.forbiddenapis.version=2.7 org.jetbrains.gradle.plugin.idea-ext.version=0.5 diff --git a/settings.gradle.kts b/settings.gradle.kts index 17fd3ed..e67e548 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -62,17 +62,14 @@ project(":shaded:avatica").projectDir = file("shaded/core") org.apache.tools.ant.DirectoryScanner.removeDefaultExclude("**/.gitattributes") org.apache.tools.ant.DirectoryScanner.removeDefaultExclude("**/.gitignore") -fun String?.toBool(nullAs: Boolean, blankAs: Boolean, default: Boolean) = -when { -this == null -> nullAs -isBlank() -> blankAs -default -> !equals("false", ignoreCase = true) -else -> equals("true", ignoreCase = true) +fun property(name: String) = +when (extra.has(name)) { +true -> extra.get(name) as? String +else -> null } -if (startParameter.projectProperties["localReleasePlugins"] -.toBool(nullAs = false, blankAs = true, default = false) -) { -// This enables to use local clone of vlsi-release-plugins for debugging purposes -includeBuild("../vlsi-release-plugins") +// This enables to use local clone of vlsi-release-plugins for debugging purposes +property("localReleasePlugins")?.ifBlank { "../vlsi-release-plugins" }?.let { +println("Importing project '$it'") +includeBuild(it) }
[GitHub] [calcite] DonnyZone commented on issue #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
DonnyZone commented on issue #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#issuecomment-553844508 Thanks for review! 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] rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346228217 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { +CalciteAssert.that() +.query("select v, o\n" ++ "from unnest(array[100, 200]) with ordinality as t1(v, o)\n" ++ "where v > 1") +.returnsUnordered( Review comment: Thanks for taking care of it. 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 #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
DonnyZone commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346200285 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { +CalciteAssert.that() +.query("select v, o\n" ++ "from unnest(array[100, 200]) with ordinality as t1(v, o)\n" ++ "where v > 1") +.returnsUnordered( Review comment: Thanks for your clarification! I will use `returns` for the first one and returnsUnordered for the second one. 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 #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
DonnyZone commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346193979 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { +CalciteAssert.that() +.query("select v, o\n" ++ "from unnest(array[100, 200]) with ordinality as t1(v, o)\n" ++ "where v > 1") +.returnsUnordered( Review comment: Yes, I agree. We can check the results are ordered. I just wonder why other similar tests use `returnsUnorderd`. 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] rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346192777 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { +CalciteAssert.that() +.query("select v, o\n" ++ "from unnest(array[100, 200]) with ordinality as t1(v, o)\n" ++ "where v > 1") +.returnsUnordered( Review comment: Sorry @DonnyZone , I was not very specific on my previous comment. I think `returns` can only be used on the first test: `unnest(array[100, 200]) with ordinality as t1(v, o)`; in the second one, since there is a JOIN involved (with no ORDER BY), I believe order cannot be guaranteed, so in that case `returnsUnordered` would be safer. Do you agree? Sorry for the inconvenience. 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] rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346171376 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { +CalciteAssert.that() +.query("select v, o\n" ++ "from unnest(array[100, 200]) with ordinality as t1(v, o)\n" ++ "where v > 1") +.returnsUnordered( Review comment: Shouldn't we check here that the results are ordered, i.e. using `returns` instead of `returnsUnordered`? 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] rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic
rubenada commented on a change in pull request #1575: [CALCITE-3498] Unnest operation's ordinality should be deterministic URL: https://github.com/apache/calcite/pull/1575#discussion_r346171177 ## File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java ## @@ -2188,9 +2188,32 @@ private static void checkJoinNWay(int n) { + " UNNEST(d.\"employees\", array[1, 2]) with ordinality as e (ei, d, n, s, c, i, o)\n" + "where ei + i > 151") .returnsUnordered( -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=2", -"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=4", -"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=5"); +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=1; O=1", +"name=HR; EI=200; D=20; N=Eric; S=8000.0; C=500; I=2; O=2", +"name=Sales; EI=150; D=10; N=Sebastian; S=7000.0; C=null; I=2; O=4"); + } + + /** Test case for + * https://issues.apache.org/jira/browse/CALCITE-3498;>[CALCITE-3498] + * Unnest operation's ordinality should be deterministic. */ + @Test public void testUnnestArrayWithDeterministicOrdinality2() { Review comment: Minor detail: the test method name ends with "...2", but I think there is no testUnnestArrayWithDeterministicOrdinality**1**. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] xndai opened a new pull request #1584: [CALCITE-3530] NPE at VolcanoPlanner#isValid when DEBUG is enabled
xndai opened a new pull request #1584: [CALCITE-3530] NPE at VolcanoPlanner#isValid when DEBUG is enabled URL: https://github.com/apache/calcite/pull/1584 When planner root is null, we should skip the validation since the memo is empty at that moment. 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