[GitHub] [calcite] danny0405 opened a new pull request #1812: [CALCITE-3801] Deprecate SqlToRelConverter.Config#isConvertTableAccess
danny0405 opened a new pull request #1812: [CALCITE-3801] Deprecate SqlToRelConverter.Config#isConvertTableAccess URL: https://github.com/apache/calcite/pull/1812 Because of CALCITE-3769, this config option is actually useless now, we always return logical rel with method RelOptTable#toRel. 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_r379978508 ## File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java ## @@ -67,33 +69,72 @@ 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) { + return null; } -return null; +final Schema s = schema.getSubSchema(schemaName2); +if (s == null) { + return null; +} +return new SimpleCalciteSchema(this, s, schemaName2); } protected TableEntry getImplicitTable(String tableName, boolean caseSensitive) { // Check implicit tables. -Table table = schema.getTable(tableName); -if (table != null) { - return tableEntry(tableName, table); +final String tableName2 = caseSensitive ? tableName : caseInsensitiveLookup( +schema.getTableNames(), tableName); +if (tableName2 == null) { Review comment: I ever thought about abstracting an interface. But it may bring complexity. (1) Different with name matcher, case (in)sensitive lookup is not widely used. Even for `CalciteSchema`, there is already a more general and efficient interface `NamedMap`. This piece of code is just for `SimpleCalciteSchema`, without maintaining cache. (2) For performance, we should prevent to call `getTableNames`/`getSubSchemas`/`getTypeNames` when case sensitive. Therefore, if we abstract the logic, the interface arguments need to contain functions. It may be too complex. Do you have any suggestions? 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 #1792: [CALCITE-3775] Implicit lookup methods in SimpleCalciteSchema ignore case sensitivity parameter
danny0405 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_r379970055 ## File path: core/src/main/java/org/apache/calcite/jdbc/SimpleCalciteSchema.java ## @@ -67,33 +69,72 @@ 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) { + return null; } -return null; +final Schema s = schema.getSubSchema(schemaName2); +if (s == null) { + return null; +} +return new SimpleCalciteSchema(this, s, schemaName2); } protected TableEntry getImplicitTable(String tableName, boolean caseSensitive) { // Check implicit tables. -Table table = schema.getTable(tableName); -if (table != null) { - return tableEntry(tableName, table); +final String tableName2 = caseSensitive ? tableName : caseInsensitiveLookup( +schema.getTableNames(), tableName); +if (tableName2 == null) { Review comment: Can we abstract a matcher like `SqlNameMatcher` for name matching ? 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 #1807: [CALCITE-3796] Support Array operator <, <=, >, >=
danny0405 commented on a change in pull request #1807: [CALCITE-3796] Support Array operator <, <=, >, >= URL: https://github.com/apache/calcite/pull/1807#discussion_r379969482 ## File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java ## @@ -693,6 +693,22 @@ public static boolean lt(BigDecimal b0, BigDecimal b1) { return b0.compareTo(b1) < 0; } + /** SQL operator applied to List values. */ + public static boolean lt(List b0, List b1) { +for (int i = 0; i < b0.size(); i++) { + // b0 is greater because b0 is longer than b1. Review comment: From your changes, it seems that the array comparison is allowed in the logic layer, we just do not have a right implementation ~ 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 #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto
danny0405 commented on a change in pull request #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto URL: https://github.com/apache/calcite/pull/1811#discussion_r379969084 ## File path: core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java ## @@ -73,7 +73,9 @@ /** Conformance value that instructs Calcite to use SQL semantics * consistent with Microsoft SQL Server version 2008. */ - SQL_SERVER_2008; + SQL_SERVER_2008, + + PRESTO; Review comment: PRESTO is not really a db, it behaves more like a computation engine, i'm not sure if we should add a new conformance for 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] danny0405 commented on a change in pull request #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto
danny0405 commented on a change in pull request #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto URL: https://github.com/apache/calcite/pull/1811#discussion_r379968866 ## File path: core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java ## @@ -298,7 +298,11 @@ public SqlQualified fullyQualify(SqlIdentifier identifier) { for (; i > 0; i--) { final SqlIdentifier prefix = identifier.getComponent(0, i); resolved.clear(); -resolve(prefix.names, nameMatcher, false, resolved); +resolve( +prefix.names, +nameMatcher, +validator.getConformance().allowAliasUnnestArrayColumns(), +resolved); Review comment: The code may works for your case, but you have changed a common sql identifier resolving logic with a special conformance, which seems not right way to go ~ The conformance switch logic should always happen in SqlValidator, we can pass along some switch flags into the scope if necessary. 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 #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto
danny0405 commented on a change in pull request #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto URL: https://github.com/apache/calcite/pull/1811#discussion_r379968267 ## File path: core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java ## @@ -97,6 +97,17 @@ public SqlUnnestOperator(boolean withOrdinality) { return builder.build(); } + private boolean allowFlattenStruct(SqlOperatorBinding operatorBinding) { +if (!(operatorBinding instanceof SqlCallBinding)) { Review comment: Another class type for operator binding is `RexCallBinding`, but why we return true for 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 #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto
danny0405 commented on a change in pull request #1811: [CALCITE-3789] Support validation of UNNEST multiple array columns like Presto URL: https://github.com/apache/calcite/pull/1811#discussion_r379968141 ## File path: core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java ## @@ -97,6 +97,17 @@ public SqlUnnestOperator(boolean withOrdinality) { return builder.build(); } + private boolean allowFlattenStruct(SqlOperatorBinding operatorBinding) { +if (!(operatorBinding instanceof SqlCallBinding)) { Review comment: `allowFlattenStruct` -> `allowAliasUnnestArrayColumns` ? 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 (6687925 -> 8cf7b44)
This is an automated email from the ASF dual-hosted git repository. danny0405 pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 6687925 [CALCITE-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens add 8cf7b44 Following [CALCITE-3763] Fix slow test failure (DonnyZone) No new revisions were added by this update. Summary of changes: core/src/test/java/org/apache/calcite/test/LatticeTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
[GitHub] [calcite] danny0405 closed pull request #1806: Following [CALCITE-3763] Fix slow test failure
danny0405 closed pull request #1806: Following [CALCITE-3763] Fix slow test failure URL: https://github.com/apache/calcite/pull/1806 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-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens
This is an automated email from the ASF dual-hosted git repository. chunwei pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/master by this push: new 6687925 [CALCITE-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens 6687925 is described below commit 668792554cf7097d668bf794ef2ba5b1fd4956ee Author: Chunwei Lei AuthorDate: Thu Feb 13 14:39:39 2020 +0800 [CALCITE-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens --- .../org/apache/calcite/plan/hep/HepPlanner.java| 26 +- .../org/apache/calcite/rel/metadata/RelMdUtil.java | 6 ++--- .../calcite/rel/metadata/RelMetadataQueryBase.java | 12 -- 3 files changed, 38 insertions(+), 6 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 3f9dd35..0ffebf2 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 @@ -59,8 +59,10 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.Set; /** @@ -874,7 +876,7 @@ public class HepPlanner extends AbstractRelOptPlanner { } parentRel.replaceInput(i, preservedVertex); } - RelMdUtil.clearCache(parentRel); + clearCache(parent); graph.removeEdge(parent, discardedVertex); graph.addEdge(parent, preservedVertex); updateVertex(parent, parentRel); @@ -889,6 +891,28 @@ public class HepPlanner extends AbstractRelOptPlanner { } } + /** + * Clears metadata cache for the RelNode and its ancestors. + * + * @param vertex relnode + */ + private void clearCache(HepRelVertex vertex) { +RelMdUtil.clearCache(vertex.getCurrentRel()); +if (!RelMdUtil.clearCache(vertex)) { + return; +} +Queue queue = +new LinkedList<>(graph.getInwardEdges(vertex)); +while (!queue.isEmpty()) { + DefaultEdge edge = queue.remove(); + HepRelVertex source = (HepRelVertex) edge.source; + RelMdUtil.clearCache(source.getCurrentRel()); + if (RelMdUtil.clearCache(source)) { +queue.addAll(graph.getInwardEdges(source)); + } +} + } + private void updateVertex(HepRelVertex vertex, RelNode rel) { if (rel != vertex.getCurrentRel()) { // REVIEW jvs 5-Apr-2006: We'll do this again later diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java index e7f9484..2a28588 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdUtil.java @@ -943,9 +943,9 @@ public class RelMdUtil { * Removes cached metadata values for specified RelNode. * * @param rel RelNode whose cached metadata should be removed + * @return true if cache for the provided RelNode was not empty */ - public static void clearCache(RelNode rel) { -rel.getCluster().getMetadataQuery().clearCache(rel); + public static boolean clearCache(RelNode rel) { +return rel.getCluster().getMetadataQuery().clearCache(rel); } - } diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java index 6bceb26..0de898c 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java @@ -23,6 +23,7 @@ import com.google.common.collect.Table; import java.lang.reflect.Proxy; import java.util.List; +import java.util.Map; import java.util.function.Supplier; /** @@ -98,8 +99,15 @@ public class RelMetadataQueryBase { * Removes cached metadata values for specified RelNode. * * @param rel RelNode whose cached metadata should be removed + * @return true if cache for the provided RelNode was not empty */ - public void clearCache(RelNode rel) { -map.row(rel).clear(); + public boolean clearCache(RelNode rel) { +Map row = map.row(rel); +if (row.isEmpty()) { + return false; +} + +row.clear(); +return true; } }
[GitHub] [calcite] chunweilei merged pull request #1801: [CALCITE-3791] HepPlanner does not clear metadata cache for the ancestors of discarded node when a transformation happens
chunweilei merged 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 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 (3e256e1 -> 21c8168)
This is an automated email from the ASF dual-hosted git repository. chunwei pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. discard 3e256e1 [CALCITE-3794] Return directly if there is no pulled up predicate add 21c8168 [CALCITE-3794] RexSimplify should return early if there is no pulled up predicate when simplifying using predicates 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 (3e256e1) \ N -- N -- N refs/heads/master (21c8168) 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: [CALCITE-3794] Return directly if there is no pulled up predicate
This is an automated email from the ASF dual-hosted git repository. chunwei pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/master by this push: new 3e256e1 [CALCITE-3794] Return directly if there is no pulled up predicate 3e256e1 is described below commit 3e256e12ff91ab26f68ec86498bd0af57f91a03c Author: Chunwei Lei AuthorDate: Thu Feb 13 16:25:03 2020 +0800 [CALCITE-3794] Return directly if there is no pulled up predicate Also some cosmetic changes. --- .../main/java/org/apache/calcite/rex/RexSimplify.java | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java index ce37244..4a1fee7 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -629,8 +629,8 @@ public class RexSimplify { // UnknownAs.FALSE corresponds to x IS TRUE evaluation // UnknownAs.TRUE to x IS NOT FALSE // Note that both UnknownAs.TRUE and UnknownAs.FALSE only changes the meaning of Unknown -// (1) if we are already in UnknownAs.FALSE mode; x IS TRUE can be simiplified to x -// (2) similarily in UnknownAs.TRUE mode ; x IS NOT FALSE can be simplified to x +// (1) if we are already in UnknownAs.FALSE mode; x IS TRUE can be simplified to x +// (2) similarily in UnknownAs.TRUE mode; x IS NOT FALSE can be simplified to x // (3) x IS FALSE could be rewritten to (NOT x) IS TRUE and from there the 1. rule applies // (4) x IS NOT TRUE can be rewritten to (NOT x) IS NOT FALSE and from there the 2. rule applies if (kind == SqlKind.IS_TRUE && unknownAs == RexUnknownAs.FALSE) { @@ -1571,6 +1571,9 @@ public class RexSimplify { private > RexNode simplifyUsingPredicates(RexNode e, Class clazz) { +if (predicates.pulledUpPredicates.isEmpty()) { + return e; +} final Comparison comparison = Comparison.of(e); // Check for comparison with null values if (comparison == null @@ -1738,7 +1741,6 @@ public class RexSimplify { // - if it is not an IS_NOT_NULL (i.e. it is a NOT_EQUALS): check comparison values if (prevNotEquals.getKind() != SqlKind.IS_NOT_NULL) { final Comparable comparable1 = notEqualsComparison.literal.getValue(); - //noinspection ConstantConditions final Comparable comparable2 = Comparison.of(prevNotEquals).literal.getValue(); //noinspection unchecked if (comparable1.compareTo(comparable2) != 0) { @@ -1978,7 +1980,7 @@ public class RexSimplify { case MILLISECOND: case MICROSECOND: if (inner == TimeUnit.QUARTER) { - return outer == TimeUnit.YEAR || outer == TimeUnit.QUARTER; + return outer == TimeUnit.YEAR; } return outer.ordinal() <= inner.ordinal(); } @@ -2014,7 +2016,7 @@ public class RexSimplify { if (p == null) { rangeTerms.put(ref, Pair.of(range(comparison, v0), - (List) ImmutableList.of(term))); + ImmutableList.of(term))); } else { // Exists boolean removeUpperBound = false; @@ -2029,7 +2031,7 @@ public class RexSimplify { } rangeTerms.put(ref, Pair.of(Range.singleton(v0), -(List) ImmutableList.of(term))); +ImmutableList.of(term))); // remove for (RexNode e : p.right) { replaceLast(terms, e, trueLiteral); @@ -2187,7 +2189,7 @@ public class RexSimplify { } newBounds.add(term); rangeTerms.put(ref, -Pair.of(r, (List) newBounds.build())); +Pair.of(r, newBounds.build())); } else if (removeLowerBound) { ImmutableList.Builder newBounds = ImmutableList.builder(); for (RexNode e : p.right) { @@ -2199,7 +2201,7 @@ public class RexSimplify { } newBounds.add(term); rangeTerms.put(ref, -Pair.of(r, (List) newBounds.build())); +Pair.of(r, newBounds.build())); } } // Default
[GitHub] [calcite] chunweilei merged pull request #1804: [CALCITE-3794] Stop simplifying using predicates if the pulled up predicates is empty
chunweilei merged pull request #1804: [CALCITE-3794] Stop simplifying using predicates if the pulled up predicates is empty URL: https://github.com/apache/calcite/pull/1804 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 #1806: Following [CALCITE-3763] Fix slow test failure
DonnyZone commented on issue #1806: Following [CALCITE-3763] Fix slow test failure URL: https://github.com/apache/calcite/pull/1806#issuecomment-586687362 Thanks, updated. It seems that ES test fails easily due to timeout. 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