[GitHub] [calcite] amaliujia commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
amaliujia commented on a change in pull request #2035: URL: https://github.com/apache/calcite/pull/2035#discussion_r445213183 ## File path: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java ## @@ -817,6 +817,112 @@ public void remove() { resultSelector); } + /** + * Group keys are sorted already. Key values are compared by using a + * specified comparator. Groups the elements of a sequence according to a + * specified key selector function and initializing one accumulator at a time. + * Go over elements sequentially, adding to accumulator each time an element + * with the same key is seen. When key changes, creates a result value from the + * accumulator and then re-initializes the accumulator. + */ + public static Enumerable sortedGroupBy( + Enumerable enumerable, + Function1 keySelector, + Function0 accumulatorInitializer, + Function2 accumulatorAdder, + final Function2 resultSelector, + final Comparator comparator) { +return new AbstractEnumerable() { + public Enumerator enumerator() { +return new SortedAggregateEnumerator( + enumerable, keySelector, accumulatorInitializer, + accumulatorAdder, resultSelector, comparator); + } +}; + } + + private static class SortedAggregateEnumerator + implements Enumerator { +private final Enumerable enumerable; +private final Function1 keySelector; +private final Function0 accumulatorInitializer; +private final Function2 accumulatorAdder; +private final Function2 resultSelector; +private final Comparator comparator; +private boolean isInitialized; +private TAccumulate curAccumulator; +private Enumerator enumerator; + +SortedAggregateEnumerator( +Enumerable enumerable, +Function1 keySelector, +Function0 accumulatorInitializer, +Function2 accumulatorAdder, +final Function2 resultSelector, +final Comparator comparator) { + this.enumerable = enumerable; + this.keySelector = keySelector; + this.accumulatorInitializer = accumulatorInitializer; + this.accumulatorAdder = accumulatorAdder; + this.resultSelector = resultSelector; + this.comparator = comparator; + isInitialized = false; + curAccumulator = null; + enumerator = enumerable.enumerator(); +} + +@Override public TResult current() { + if (curAccumulator == null) { +curAccumulator = accumulatorInitializer.apply(); + } + TResult result = null; + TSource o = enumerator.current(); + TKey prevKey = keySelector.apply(o); + curAccumulator = accumulatorAdder.apply(curAccumulator, o); + while (enumerator.moveNext()) { +o = enumerator.current(); +TKey curKey = keySelector.apply(o); Review comment: Added a test to test null values. Based on the test, null is put at the last position (ASC) by comparator. 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
[GitHub] [calcite] amaliujia commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
amaliujia commented on a change in pull request #2035: URL: https://github.com/apache/calcite/pull/2035#discussion_r445212926 ## File path: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java ## @@ -817,6 +817,112 @@ public void remove() { resultSelector); } + /** + * Group keys are sorted already. Key values are compared by using a + * specified comparator. Groups the elements of a sequence according to a + * specified key selector function and initializing one accumulator at a time. + * Go over elements sequentially, adding to accumulator each time an element + * with the same key is seen. When key changes, creates a result value from the + * accumulator and then re-initializes the accumulator. + */ + public static Enumerable sortedGroupBy( + Enumerable enumerable, + Function1 keySelector, + Function0 accumulatorInitializer, + Function2 accumulatorAdder, + final Function2 resultSelector, + final Comparator comparator) { +return new AbstractEnumerable() { + public Enumerator enumerator() { +return new SortedAggregateEnumerator( + enumerable, keySelector, accumulatorInitializer, + accumulatorAdder, resultSelector, comparator); + } +}; + } + + private static class SortedAggregateEnumerator + implements Enumerator { +private final Enumerable enumerable; +private final Function1 keySelector; +private final Function0 accumulatorInitializer; +private final Function2 accumulatorAdder; +private final Function2 resultSelector; +private final Comparator comparator; +private boolean isInitialized; +private TAccumulate curAccumulator; +private Enumerator enumerator; + +SortedAggregateEnumerator( +Enumerable enumerable, +Function1 keySelector, +Function0 accumulatorInitializer, +Function2 accumulatorAdder, +final Function2 resultSelector, +final Comparator comparator) { + this.enumerable = enumerable; + this.keySelector = keySelector; + this.accumulatorInitializer = accumulatorInitializer; + this.accumulatorAdder = accumulatorAdder; + this.resultSelector = resultSelector; + this.comparator = comparator; + isInitialized = false; + curAccumulator = null; + enumerator = enumerable.enumerator(); +} + +@Override public TResult current() { Review comment: Thanks for the example! I doubled check the contract and have the following conclusion: ``` * After an enumerator is created or after the {@link #reset} method is * called, the {@link #moveNext} method must be called to advance the * enumerator to the first element of the collection before reading the * value of the {@code current} property; otherwise, {@code current} is * undefined. ``` So in this case, return `null` is right as the result is undefined. ``` * This method also throws {@link java.util.NoSuchElementException} if * the last call to {@code moveNext} returned {@code false}, which indicates * the end of the collection. ``` I updated the code to make sure it throws `NoSuchElementException` in this 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
[calcite] branch master updated: [CALCITE-4087] Hoist, a utility to replace literals in a SQL string with placeholders
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 The following commit(s) were added to refs/heads/master by this push: new 6f90aca [CALCITE-4087] Hoist, a utility to replace literals in a SQL string with placeholders 6f90aca is described below commit 6f90acaaac962f666741dae8fa20170e1d9a71e4 Author: Julian Hyde AuthorDate: Tue Jun 23 17:06:50 2020 -0700 [CALCITE-4087] Hoist, a utility to replace literals in a SQL string with placeholders --- .../org/apache/calcite/test/BabelParserTest.java | 39 + .../main/java/org/apache/calcite/tools/Hoist.java | 191 + .../apache/calcite/sql/parser/SqlParserTest.java | 51 ++ 3 files changed, 281 insertions(+) diff --git a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java index b5d185f..47d5500 100644 --- a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java +++ b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java @@ -16,11 +16,15 @@ */ package org.apache.calcite.test; +import org.apache.calcite.sql.SqlDialect; +import org.apache.calcite.sql.dialect.MysqlSqlDialect; import org.apache.calcite.sql.parser.SqlAbstractParserImpl; +import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql.parser.SqlParserImplFactory; import org.apache.calcite.sql.parser.SqlParserTest; import org.apache.calcite.sql.parser.SqlParserUtil; import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl; +import org.apache.calcite.tools.Hoist; import com.google.common.base.Throwables; @@ -281,4 +285,39 @@ class BabelParserTest extends SqlParserTest { + "(`BAR` INTEGER NOT NULL, `BAZ` VARCHAR(30))"; sql(sql).ok(expected); } + + /** Similar to {@link #testHoist()} but using custom parser. */ + @Test void testHoistMySql() { +// SQL contains back-ticks, which require MySQL's quoting, +// and DATEADD, which requires Babel. +final String sql = "select 1 as x,\n" ++ " 'ab' || 'c' as y\n" ++ "from `my emp` /* comment with 'quoted string'? */ as e\n" ++ "where deptno < 40\n" ++ "and DATEADD(day, 1, hiredate) > date '2010-05-06'"; +final SqlDialect dialect = MysqlSqlDialect.DEFAULT; +final Hoist.Hoisted hoisted = +Hoist.create(Hoist.config() +.withParserConfig( +dialect.configureParser(SqlParser.configBuilder()) +.setParserFactory(SqlBabelParserImpl::new) +.build())) +.hoist(sql); + +// Simple toString converts each variable to '?N' +final String expected = "select ?0 as x,\n" ++ " ?1 || ?2 as y\n" ++ "from `my emp` /* comment with 'quoted string'? */ as e\n" ++ "where deptno < ?3\n" ++ "and DATEADD(day, ?4, hiredate) > ?5"; +assertThat(hoisted.toString(), is(expected)); + +// Custom string converts variables to '[N:TYPE:VALUE]' +final String expected2 = "select [0:DECIMAL:1] as x,\n" ++ " [1:CHAR:ab] || [2:CHAR:c] as y\n" ++ "from `my emp` /* comment with 'quoted string'? */ as e\n" ++ "where deptno < [3:DECIMAL:40]\n" ++ "and DATEADD(day, [4:DECIMAL:1], hiredate) > [5:DATE:2010-05-06]"; +assertThat(hoisted.substitute(SqlParserTest::varToStr), is(expected2)); + } } diff --git a/core/src/main/java/org/apache/calcite/tools/Hoist.java b/core/src/main/java/org/apache/calcite/tools/Hoist.java new file mode 100644 index 000..c8e7f21 --- /dev/null +++ b/core/src/main/java/org/apache/calcite/tools/Hoist.java @@ -0,0 +1,191 @@ +/* + * 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.tools; + +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.parser.SqlParseException; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.parser.SqlParserUtil; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.sql.util.SqlShuttle; +import
[calcite] branch master updated: [CALCITE-3786] Make digestEquals and digestHash available to be overridden
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 af3bca3 [CALCITE-3786] Make digestEquals and digestHash available to be overridden af3bca3 is described below commit af3bca328a40d2d6515ea00e2094974cc725d4c3 Author: Haisheng Yuan AuthorDate: Tue Jun 23 17:20:37 2020 -0500 [CALCITE-3786] Make digestEquals and digestHash available to be overridden By default #digestEquals() and #digestHash() collect digest attributes from compute hash code. This should work well for most cases. If the method is a performance bottleneck for your project, or the default behavior can't handle your scenario properly, you can choose to override #digestEquals() and Only these operators are changed to override the default behavior, for performance and demonstration purposes. All the other operators remain unchanged. Close #2044 --- .../java/org/apache/calcite/plan/RelOptNode.java | 15 +-- .../org/apache/calcite/plan/hep/HepRelVertex.java | 11 +--- .../org/apache/calcite/rel/AbstractRelNode.java| 26 -- .../main/java/org/apache/calcite/rel/RelNode.java | 24 ++--- .../java/org/apache/calcite/rel/core/Filter.java | 19 + .../java/org/apache/calcite/rel/core/Join.java | 21 +++ .../java/org/apache/calcite/rel/core/Project.java | 20 ++ .../apache/calcite/rel/logical/LogicalFilter.java | 9 +++ .../apache/calcite/rel/logical/LogicalJoin.java| 13 + .../apache/calcite/rel/logical/LogicalProject.java | 8 ++ .../org/apache/calcite/rel/type/RelDataType.java | 31 ++ 11 files changed, 169 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java index 208a87c..c21f745 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java @@ -18,8 +18,6 @@ package org.apache.calcite.plan; import org.apache.calcite.rel.type.RelDataType; -import org.apiguardian.api.API; - import java.util.List; /** @@ -49,18 +47,7 @@ public interface RelOptNode { * * @return Digest string of this {@code RelNode} */ - default String getDigest() { -return getRelDigest().toString(); - } - - /** - * Digest of the {@code RelNode}, for planner internal use only. - * - * @return Digest of this {@code RelNode} - * @see #getDigest() - */ - @API(since = "1.24", status = API.Status.INTERNAL) - RelDigest getRelDigest(); + String getDigest(); /** * Retrieves this RelNode's traits. Note that although the RelTraitSet diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java index 1061a61..4ad6d28 100644 --- a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java +++ b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java @@ -91,9 +91,14 @@ public class HepRelVertex extends AbstractRelNode { return currentRel; } - @Override public RelWriter explainTerms(final RelWriter pw) { -return super.explainTerms(pw) -.item("currentRel", currentRel.getId()); + @Override protected boolean digestEquals(Object obj) { +return this == obj +|| (obj instanceof HepRelVertex +&& currentRel == ((HepRelVertex) obj).currentRel); + } + + @Override protected int digestHash() { +return currentRel.getId(); } @Override public String getDigest() { diff --git a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java index 4ab68a0..ec52c10 100644 --- a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java +++ b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java @@ -337,9 +337,8 @@ public abstract class AbstractRelNode implements RelNode { return r; } - public RelDigest recomputeDigest() { + public void recomputeDigest() { digest.clear(); -return digest; } public void replaceInput( @@ -394,9 +393,19 @@ public abstract class AbstractRelNode implements RelNode { } /** - * Equality check for RelNode digest + * Equality check for RelNode digest. + * + * By default this method collects digest attributes from + * {@link #explainTerms(RelWriter)}, then compares each attribute pair. + * This should work well for most cases. If this method is a performance + * bottleneck for your project, or the default behavior can't handle + * your scenario properly, you can choose to override this method and + * {@link #digestHash()}. See {@code LogicalJoin} as an example. + * + * @return Whether the 2 RelN
[GitHub] [calcite] hsyuan merged pull request #2044: [CALCITE-3786] Make digestEquals and digestHash available to be overridden
hsyuan merged pull request #2044: URL: https://github.com/apache/calcite/pull/2044 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
[calcite] branch master updated: [CALCITE-4086] Upgrade Avatica version to 1.17.0
This is an automated email from the ASF dual-hosted git repository. zabetak pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/master by this push: new b0b435c [CALCITE-4086] Upgrade Avatica version to 1.17.0 b0b435c is described below commit b0b435cae05d2d96326e3f7165bfaea4c2b28e52 Author: Stamatis Zampetakis AuthorDate: Wed Jun 24 00:40:11 2020 +0200 [CALCITE-4086] Upgrade Avatica version to 1.17.0 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index ea5b9aa..d67b6a2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -25,7 +25,7 @@ kotlin.parallel.tasks.in.project=true # Release version can be generated by using -Prelease or -Prc= arguments calcite.version=1.24.0 # This is a version to be used from Maven repository. It can be overridden by localAvatica below -calcite.avatica.version=1.16.0 +calcite.avatica.version=1.17.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=
[GitHub] [calcite] zabetak merged pull request #2043: [CALCITE-4086] Upgrade Avatica version to 1.17.0
zabetak merged pull request #2043: URL: https://github.com/apache/calcite/pull/2043 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
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035: URL: https://github.com/apache/calcite/pull/2035#discussion_r444744863 ## File path: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java ## @@ -817,6 +817,112 @@ public void remove() { resultSelector); } + /** + * Group keys are sorted already. Key values are compared by using a + * specified comparator. Groups the elements of a sequence according to a + * specified key selector function and initializing one accumulator at a time. + * Go over elements sequentially, adding to accumulator each time an element + * with the same key is seen. When key changes, creates a result value from the + * accumulator and then re-initializes the accumulator. + */ + public static Enumerable sortedGroupBy( + Enumerable enumerable, + Function1 keySelector, + Function0 accumulatorInitializer, + Function2 accumulatorAdder, + final Function2 resultSelector, + final Comparator comparator) { +return new AbstractEnumerable() { + public Enumerator enumerator() { +return new SortedAggregateEnumerator( + enumerable, keySelector, accumulatorInitializer, + accumulatorAdder, resultSelector, comparator); + } +}; + } + + private static class SortedAggregateEnumerator + implements Enumerator { +private final Enumerable enumerable; +private final Function1 keySelector; +private final Function0 accumulatorInitializer; +private final Function2 accumulatorAdder; +private final Function2 resultSelector; +private final Comparator comparator; +private boolean isInitialized; +private TAccumulate curAccumulator; +private Enumerator enumerator; + +SortedAggregateEnumerator( +Enumerable enumerable, +Function1 keySelector, +Function0 accumulatorInitializer, +Function2 accumulatorAdder, +final Function2 resultSelector, +final Comparator comparator) { + this.enumerable = enumerable; + this.keySelector = keySelector; + this.accumulatorInitializer = accumulatorInitializer; + this.accumulatorAdder = accumulatorAdder; + this.resultSelector = resultSelector; + this.comparator = comparator; + isInitialized = false; + curAccumulator = null; + enumerator = enumerable.enumerator(); +} + +@Override public TResult current() { Review comment: Also a minor detail, please notice that, per contract, `current` must throw `NoSuchElementException` if `moveNext` has not been called, in your case I think it would just return `null`. You can easily fix this by using a DUMMY object, see e.g. `lazyCollectionSpool` method 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
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035: URL: https://github.com/apache/calcite/pull/2035#discussion_r444738980 ## File path: linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java ## @@ -817,6 +817,112 @@ public void remove() { resultSelector); } + /** + * Group keys are sorted already. Key values are compared by using a + * specified comparator. Groups the elements of a sequence according to a + * specified key selector function and initializing one accumulator at a time. + * Go over elements sequentially, adding to accumulator each time an element + * with the same key is seen. When key changes, creates a result value from the + * accumulator and then re-initializes the accumulator. + */ + public static Enumerable sortedGroupBy( + Enumerable enumerable, + Function1 keySelector, + Function0 accumulatorInitializer, + Function2 accumulatorAdder, + final Function2 resultSelector, + final Comparator comparator) { +return new AbstractEnumerable() { + public Enumerator enumerator() { +return new SortedAggregateEnumerator( + enumerable, keySelector, accumulatorInitializer, + accumulatorAdder, resultSelector, comparator); + } +}; + } + + private static class SortedAggregateEnumerator + implements Enumerator { +private final Enumerable enumerable; +private final Function1 keySelector; +private final Function0 accumulatorInitializer; +private final Function2 accumulatorAdder; +private final Function2 resultSelector; +private final Comparator comparator; +private boolean isInitialized; +private TAccumulate curAccumulator; +private Enumerator enumerator; + +SortedAggregateEnumerator( +Enumerable enumerable, +Function1 keySelector, +Function0 accumulatorInitializer, +Function2 accumulatorAdder, +final Function2 resultSelector, +final Comparator comparator) { + this.enumerable = enumerable; + this.keySelector = keySelector; + this.accumulatorInitializer = accumulatorInitializer; + this.accumulatorAdder = accumulatorAdder; + this.resultSelector = resultSelector; + this.comparator = comparator; + isInitialized = false; + curAccumulator = null; + enumerator = enumerable.enumerator(); +} + +@Override public TResult current() { + if (curAccumulator == null) { +curAccumulator = accumulatorInitializer.apply(); + } + TResult result = null; + TSource o = enumerator.current(); + TKey prevKey = keySelector.apply(o); + curAccumulator = accumulatorAdder.apply(curAccumulator, o); + while (enumerator.moveNext()) { +o = enumerator.current(); +TKey curKey = keySelector.apply(o); Review comment: NULLs should always come at the beginning or at the end, so I assume the current algorithm would work... if the comparator is able to support nulls. Otherwise, as you say, we would need to add an extra check for null values. I guess this will be an good scenario for a unit 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