[calcite] branch main updated: [CALCITE-5336] Infer constants from predicates with IS NOT DISTINCT FROM operator
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 5a9cd9f259 [CALCITE-5336] Infer constants from predicates with IS NOT
DISTINCT FROM operator
5a9cd9f259 is described below
commit 5a9cd9f259c4dcc34a1bc1291dfcd188b3128156
Author: jtrada168
AuthorDate: Wed Nov 2 18:30:43 2022 -0700
[CALCITE-5336] Infer constants from predicates with IS NOT DISTINCT FROM
operator
Support DateString in RelBuilder.literal(Object).
Close apache/calcite#2961
---
.../apache/calcite/prepare/CalcitePrepareImpl.java | 3 ++
.../main/java/org/apache/calcite/rex/RexUtil.java | 25 --
.../java/org/apache/calcite/tools/RelBuilder.java | 3 ++
.../org/apache/calcite/rex/RexProgramTest.java | 27 ++-
.../org/apache/calcite/test/RelOptRulesTest.java | 40 ++
.../org/apache/calcite/test/RelOptRulesTest.xml| 16 +
6 files changed, 102 insertions(+), 12 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
index 6d06705677..ad0fff75f5 100644
--- a/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.prepare;
+import org.apache.calcite.DataContexts;
import org.apache.calcite.adapter.enumerable.EnumerableCalc;
import org.apache.calcite.adapter.enumerable.EnumerableConvention;
import org.apache.calcite.adapter.enumerable.EnumerableInterpretable;
@@ -72,6 +73,7 @@ import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexExecutorImpl;
import org.apache.calcite.rex.RexInputRef;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexProgram;
@@ -442,6 +444,7 @@ public class CalcitePrepareImpl implements CalcitePrepare {
}
final VolcanoPlanner planner =
new VolcanoPlanner(costFactory, externalContext);
+planner.setExecutor(new RexExecutorImpl(DataContexts.EMPTY));
planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
if (CalciteSystemProperty.ENABLE_COLLATION_TRAIT.value()) {
planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index b68eb43831..9fb321a905 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -400,24 +400,27 @@ public class RexUtil {
private static void gatherConstraints(Class clazz,
RexNode predicate, Map map, Set excludeSet,
RexBuilder rexBuilder) {
-if (predicate.getKind() != SqlKind.EQUALS
-&& predicate.getKind() != SqlKind.IS_NULL) {
- decompose(excludeSet, predicate);
- return;
-}
-final List operands = ((RexCall) predicate).getOperands();
final RexNode left;
final RexNode right;
-if (predicate.getKind() == SqlKind.EQUALS) {
- left = operands.get(0);
- right = operands.get(1);
-} else { // is null
- left = operands.get(0);
+switch (predicate.getKind()) {
+case EQUALS:
+case IS_NOT_DISTINCT_FROM:
+ left = ((RexCall) predicate).getOperands().get(0);
+ right = ((RexCall) predicate).getOperands().get(1);
+ break;
+
+case IS_NULL:
+ left = ((RexCall) predicate).getOperands().get(0);
if (!left.getType().isNullable()) {
// There's no sense in inferring $0=null when $0 is not nullable
return;
}
right = rexBuilder.makeNullLiteral(left.getType());
+ break;
+
+default:
+ decompose(excludeSet, predicate);
+ return;
}
// Note that literals are immutable too, and they can only be compared
// through values.
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 fdf6f0ab62..f48b14272c 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -99,6 +99,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.TableFunctionReturnTypeInference;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.sql2rel.SqlToRelConverter;
+import org.apache.calcite.util.DateString;
import org.apache.calcite.util.Holder;
import org.apache.calcite.util.ImmutableBitSet;
import org.apache.calcite.util.ImmutableIntList;
@@ -475,6 +476,8 @@ public class RelBuilder
[GitHub] [calcite] julianhyde closed pull request #2961: [CALCITE-5336] Infer constants from predicates with IS NOT DISTINCT FROM operator
julianhyde closed pull request #2961: [CALCITE-5336] Infer constants from predicates with IS NOT DISTINCT FROM operator URL: https://github.com/apache/calcite/pull/2961 -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
julianhyde commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322750893 The precision issue really is orthogonal, so at best it muddies the waters. See https://issues.apache.org/jira/browse/CALCITE-5266. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
julianhyde commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322748744 Let's move design discussions to 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] wnob commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
wnob commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322745325 I would also note that both BQ types have microsecond precision, so either way the existing Calcite `TIMESTAMP` is not a perfect 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] wnob commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
wnob commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322739188 Of course, we could simply re-interpret those bits as `DATETIME`, as I mentioned. We don't need a new internal representation; we could just make a new type that happens to have the same internal representation. UTC is not exactly relevant for a BQ timestamp. Sure, the epoch is 1970-01-01 00:00:00 UTC, but you could also think of it as 1969-01-01 16:00:00 PST. It's just a point in time. On the other hand, a time zone would be very relevant if we decided to represent BQ `DATETIME` as a Calcite `TIMESTAMP`, since we would have to interpret Calcite's offset-from-epoch in a particular time zone, UTC being the obvious choice. Alternatively, we could pick a new internal representation for BQ `DATETIME` -- e.g. as a Java `LocalDateTime`, which is internally represented as clock/calendar parameters with no time zone -- and avoid the implementation detail of converting from offset to clock/calendar parameters in a particular time zone. It doesn't really matter from an engineering standpoint. Basically, we have 2 equally possible options: 1. Map BQ `TIMESTAMP` to Calcite `TIMESTAMP`, and map BQ `DATETIME` to something else (probably a whole new type that's BQ-specific, call it `BIGQUERY_DATETIME`) 2. Map BQ `DATETIME` to Calcite `TIMESTAMP`, and map BQ `TIMESTAMP` to something else (also probably a whole new type that's BQ-specific, call it `BIGQUERY_TIMESTAMP`). I just don't see any advantages to option 1 over 2. Option 1 would cause confusion, as well as requiring a slightly awkward internal representation for `DATETIME` because it requires a time zone to convert from internal representation to semantic representation. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
julianhyde commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322709533 No, BigQuery, a timestamp is represented internally as an offset in microseconds since epoch *UTC*. The "UTC" is a crucial difference. Sure, they're both a bunch of bits. But so is a 64 bit IEEE floating point number. The interpretation of those bits is crucial. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] wnob commented on pull request #2973: [CALCITE-5180] Implement some of the overloads for BigQuery's DATE and TIMESTAMP
wnob commented on PR #2973: URL: https://github.com/apache/calcite/pull/2973#issuecomment-1322705468 > BQ `DATETIME` = Calcite `TIMESTAMP` I don't see the connection. In Calcite, a timestamp is represented internally as an offset in milliseconds since epoch. In BigQuery, a timestamp is represented internally as an offset in microseconds since epoch. Apart from the difference in resolution, they're essentially the same. Of course, this can easily be re-interpreted as a `DATETIME` by assuming a default time zone, but I struggle to see how a Calcite `TIMESTAMP` is better mapped to a BQ `DATETIME` than to a BQ `TIMESTAMP`. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tanclary commented on a diff in pull request #2978: [CALCITE-5385] Add BigQuery as support library for implemented functions
tanclary commented on code in PR #2978: URL: https://github.com/apache/calcite/pull/2978#discussion_r1028364707 ## gradle.properties: ## @@ -27,7 +27,7 @@ systemProp.org.gradle.internal.publish.checksums.insecure=true # This is version for Calcite itself # Note: it should not include "-SNAPSHOT" as it is automatically added by build.gradle.kts # Release version can be generated by using -Prelease or -Prc= arguments -calcite.version=1.33.0 +calcite.version=1.33.0-tannerclary Review Comment: Whoops, good catch. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tjbanghart commented on a diff in pull request #2978: [CALCITE-5385] Add BigQuery as support library for implemented functions
tjbanghart commented on code in PR #2978: URL: https://github.com/apache/calcite/pull/2978#discussion_r1028359636 ## gradle.properties: ## @@ -27,7 +27,7 @@ systemProp.org.gradle.internal.publish.checksums.insecure=true # This is version for Calcite itself # Note: it should not include "-SNAPSHOT" as it is automatically added by build.gradle.kts # Release version can be generated by using -Prelease or -Prc= arguments -calcite.version=1.33.0 +calcite.version=1.33.0-tannerclary Review Comment: Wouldn't want that slipping in! -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tanclary opened a new pull request, #2978: [Calcite-5385] Add BigQuery as support library for implemented functions
tanclary opened a new pull request, #2978: URL: https://github.com/apache/calcite/pull/2978 Add BigQuery as a supported library for functions that have already been implemented for other libraries. This ticket overrides Calcite-5384 to include other functions that need BQ added as a supported library. Note: This is my first Calcite PR so any feedback on the changes, the formatting of the PR, or any other aspect is highly appreciated. Thank you! List of functions: MD5() SHA1() GREATEST() LEAST() COSH() SINH() TANH() FROM_BASE64() LEFT() LTRIM() REPEAT() REVERSE() RIGHT() RTRIM() SOUNDEX() TRANSLATE() -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
asolimando commented on code in PR #2976:
URL: https://github.com/apache/calcite/pull/2976#discussion_r1028128040
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java:
##
@@ -391,6 +392,14 @@ protected RelMdExpressionLineage() {}
return mq.getExpressionLineage(rel.getInput(), outputExpression);
}
+ /**
+ * Expression lineage from Snapshot.
+ */
Review Comment:
It's better to keep the PR focused on the topic I guess, but thanks for
volunteering
--
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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
asolimando commented on code in PR #2976:
URL: https://github.com/apache/calcite/pull/2976#discussion_r1028126808
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java:
##
@@ -391,6 +392,18 @@ protected RelMdExpressionLineage() {}
return mq.getExpressionLineage(rel.getInput(), outputExpression);
}
+ /**
+ * Expression lineage from Snapshot.
+ * @param rel Relational expression which is Snapshot
+ * @param mq Metadata query
+ * @param outputExpression Expression which need be inferred
+ * @return If the lineage cannot be inferred, we return null.
+ */
Review Comment:
Thanks for adding the javadoc, in general the sentence starts with
lower-case letters, I have taken the liberty to also rephrase a couple of
sentences. No problem if you want to keep the original sentences, but please
fix the first letter casing if you do so.
```suggestion
/**
* Expression lineage from Snapshot.
* @param rel Snapshot relational expression
* @param mq metadata query
* @param outputExpression expression which needs to be inferred
* @return the inferred lineage, possibly 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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
JiajunBernoulli commented on code in PR #2976:
URL: https://github.com/apache/calcite/pull/2976#discussion_r1028107022
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java:
##
@@ -391,6 +392,14 @@ protected RelMdExpressionLineage() {}
return mq.getExpressionLineage(rel.getInput(), outputExpression);
}
+ /**
+ * Expression lineage from Snapshot.
+ */
Review Comment:
Thanks for your review, I added javadoc.
If you think that they is reasonable, I can add for other methods.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
JiajunBernoulli commented on PR #2976: URL: https://github.com/apache/calcite/pull/2976#issuecomment-1322142351 Sorry, I'll pay attention later. -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
asolimando commented on PR #2976: URL: https://github.com/apache/calcite/pull/2976#issuecomment-1322134305 @JiajunBernoulli, until the review process is over please don't force-push, it destroys the "link" to the old comments and it makes incremental reviewing impossible (checking just the delta from the latest review). In this case the PR is small-sized, but let's avoid it altogether -- 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2976: [CALCITE-5392] Support Snapshot in RelMdExpressionLineage
asolimando commented on code in PR #2976:
URL: https://github.com/apache/calcite/pull/2976#discussion_r1027756107
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdExpressionLineage.java:
##
@@ -391,6 +392,14 @@ protected RelMdExpressionLineage() {}
return mq.getExpressionLineage(rel.getInput(), outputExpression);
}
+ /**
+ * Expression lineage from Snapshot.
+ */
Review Comment:
I know that none of the other methods in the class have it, but I think it's
important to have proper javadoc for public methods of this kind, something
along this format:
```
/**
* Expression lineage from Snapshot.
* @param rel
* @param mq
* @param outputExpression
* @return
*/
```
What do you think Jiajun?
--
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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
