[GitHub] [calcite] birschick-bq closed pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
birschick-bq closed pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric URL: https://github.com/apache/calcite/pull/3031 -- 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] sonarcloud[bot] commented on pull request #3038: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
sonarcloud[bot] commented on PR #3038: URL: https://github.com/apache/calcite/pull/3038#issuecomment-1398008354 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3038) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3038&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3038&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3038&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3038&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3038&metric=new_coverage&view=list) [75.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3038&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3038&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3038&metric=new_duplicated_lines_density&view=list) -- 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] libenchao opened a new pull request, #3038: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
libenchao opened a new pull request, #3038: URL: https://github.com/apache/calcite/pull/3038 This PR is based on https://github.com/apache/calcite/pull/3031 from @birschick-bq -- 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] libenchao closed pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
libenchao closed pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr… URL: https://github.com/apache/calcite/pull/3035 -- 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] sonarcloud[bot] commented on pull request #2981: [CALCITE-5283] Add ARG_MIN, ARG_MAX aggregate functions
sonarcloud[bot] commented on PR #2981: URL: https://github.com/apache/calcite/pull/2981#issuecomment-1397858121 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2981) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2981&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2981&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2981&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2981&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2981&metric=new_coverage&view=list) [71.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2981&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2981&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2981&metric=new_duplicated_lines_density&view=list) -- 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] sonarcloud[bot] commented on pull request #3036: [CALCITE-5424] Custom literals
sonarcloud[bot] commented on PR #3036: URL: https://github.com/apache/calcite/pull/3036#issuecomment-1397828527 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3036) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [45 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_coverage&view=list) [98.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_duplicated_lines_density&view=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_duplicated_lines_density&view=list) -- 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] olivrlee commented on a diff in pull request #3036: [CALCITE-5424] Custom literals
olivrlee commented on code in PR #3036:
URL: https://github.com/apache/calcite/pull/3036#discussion_r1082033615
##
core/src/main/java/org/apache/calcite/sql/SqlUnknownLiteral.java:
##
@@ -0,0 +1,68 @@
+/*
+ * 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.sql;
+
+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.util.NlsString;
+import org.apache.calcite.util.Util;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Literal whose type is not yet known.
+ */
+public class SqlUnknownLiteral extends SqlLiteral {
+ public final String tag;
+
+ SqlUnknownLiteral(String tag, String value, SqlParserPos pos) {
+super(requireNonNull(value, "value"), SqlTypeName.UNKNOWN, pos);
+this.tag = requireNonNull(tag, "tag");
+ }
+
+ @Override public String getValue() {
+return (String) requireNonNull(super.getValue(), "value");
+ }
+
+ @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec)
{
+final NlsString nlsString = new NlsString(getValue(), null, null);
+writer.keyword(tag);
+writer.literal(nlsString.asSql(true, true, writer.getDialect()));
+ }
+
+
+ /** Converts this unknown literal to a literal of known type. */
+ public SqlLiteral resolve(SqlTypeName typeName) {
+switch (typeName) {
+case DATE:
+ return SqlParserUtil.parseDateLiteral(getValue(), pos);
+case TIME:
+ return SqlParserUtil.parseTimeLiteral(getValue(), pos);
+case TIMESTAMP:
+ return SqlParserUtil.parseTimestampLiteral(getValue(), pos);
+case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+ return SqlParserUtil.parseTimestampWithLocalTimeZoneLiteral(getValue(),
pos);
+default:
+ throw Util.unexpected(typeName);
+}
+ }
+
+ @Override public T getValueAs(Class clazz) {
Review Comment:
Q: Is this override necessary if it's just calling `super.getValueAs` ?
--
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] sonarcloud[bot] commented on pull request #3036: [CALCITE-5424] Custom literals
sonarcloud[bot] commented on PR #3036: URL: https://github.com/apache/calcite/pull/3036#issuecomment-1397815215 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3036) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3036&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [46 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3036&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_coverage&view=list) [98.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_duplicated_lines_density&view=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3036&metric=new_duplicated_lines_density&view=list) -- 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-avatica] julianhyde commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
julianhyde commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081992148
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
> This is a private method, and all existing invocations of it pass it a
null Calendar. If you open Avatica at head in IntelliJ, it suggests inlining
the constant parameter.
Ah, I missed that. I guess the calendar parameter had some use in the
distant past.
Still, don't overuse the 'no tests broke' or 'intellij suggested a
refactoring' argument. By that argument you can justify renaming a variable
called 'black' to 'white' - intellij and tests don't care about class and
variable names - but it doesn't account for the confusion you create when you
don't make the code self-explanatory.
> Note that both a regular TIMESTAMP and a TIMESTAMP WITH LOCAL TIME ZONE
are represented in JDBC as TIMESTAMP
There are two separate decisions to make: how to represent the type, and how
to represent the value.
It seems that there is a way to represent the type: type = TIMESTAMP and
typeName = "TIMESTAMP_WITH_LOCAL_TIME_ZONE" or something like that.
How have you decided to represent the value? It's worth calling out
explicitly.
> I chose to re-use the existing timestamp getter with the addition of this
new fixedInstant parameter since it seemed like a straightforward addition.
It's not totally straightforward. Difference in interpretation is subtle.
And adding a boolean field and an extra couple of `if`s to some very simple
classes does make them significantly more complex.
So I would actually create a `class TimestampLtzAccessor` (maybe it would
extend `TimestampAccessor`, maybe not) to make everything explicit.
--
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-avatica] julianhyde commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
julianhyde commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081983370
##
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
-super(timeZone);
+// Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
> So, there's certainly an existing bug in the implementation of
Array.getResultSet() that only manifests
> when the array contains timestamps and when the connection default time
zone is not GMT, and this
> was not covered by tests
I don't know of any such bugs. But your changes do seem to make things worse
- by ensuring that the nested `AvaticaResultSet` set does not get the right
`timeZone`.
If there's a bug, log 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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] julianhyde merged pull request #202: (do not merge) avatica staging
julianhyde merged PR #202: URL: https://github.com/apache/calcite-avatica/pull/202 -- 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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081880447
##
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
-super(timeZone);
+// Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
I can't refute that with 100% certainty, but I will point out:
- Currently, at head, if you just make this 1 line change to pass `UTC_ZONE`
to the super constructor (here, the superclass being `ArrayFactoryImpl`) and
run all the tests, none of them fail.
- Alternatively, if you keep this 1 line as is, but go into
`AvaticaResultSetConversionTest` and delete the [`timeZone` property for the
connection][1], which is currently set to `"GMT"`, several `getString()` and
`getArray()` tests start to fail. The `getString()` failures are
self-explanatory, but the `getArray()` failures are tricky. They're
double-adjusting the timestamps:
- Once when they call [`getObject()`][2] (which invokes `getTimestamp()`
with the connection's default calendar, which was previously GMT but is now the
system default).
- Then, in [`ArrayImpl.equalContents()`][3] the arrays are converted to
result sets for traversal via [`Array.getResultSet()` ][4], which uses the same
time zone as the "factory" result set and eventually invokes `getObject()`
again, thus applying the time zone offset twice.
So, there's certainly an existing bug in the implementation of
`Array.getResultSet()` that only manifests when the array contains timestamps
and when the connection default time zone is not GMT, and this was not covered
by tests. By removing the explicit GMT `timeZone` for these tests, I'm covering
this case, and this seemed like the best solution. It's just hardcoding a UTC
time zone for the `ArrayFactoryImpl` parent of a `ResultSet`.
[1]:
https://github.com/apache/calcite-avatica/blob/dbe9b1d8c2e53474eb40cfaf5721aceca3bdb57f/core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetConversionsTest.java#L1121
[2]:
https://github.com/apache/calcite-avatica/blob/810acf80771310431d7ef576f3404299ebb8eaf2/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java#L1432
[3]:
https://github.com/apache/calcite-avatica/blob/820edf6f653607afb5a2a280a32f315aff1f64cb/core/src/main/java/org/apache/calcite/avatica/util/ArrayImpl.java#L233
[4]:
https://docs.oracle.com/javase/8/docs/api/java/sql/Array.html#getResultSet--
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081852921
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
This is a private method, and all existing invocations of it pass it a null
`Calendar`. If you open Avatica at head in IntelliJ, it suggests inlining the
constant parameter.
With my change, a `TIMESTAMP WITH LOCAL TIME ZONE` getter might pass it a
non-null `Calendar`, and that's the only time it could be non-null. So,
existing behavior cannot possibly be impacted by switching that minus to a plus.
Note that both a regular `TIMESTAMP` and a `TIMESTAMP WITH LOCAL TIME ZONE`
are represented in JDBC as `TIMESTAMP` (since there is no JDBC type for the
latter). I chose to re-use the existing timestamp getter with the addition of
this new `fixedInstant` parameter since it seemed like a straightforward
addition.
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081852921
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
This is a private method, and all existing invocations of it pass it a null
`Calendar`. If you open Avatica at head in IntelliJ, it suggests inlining the
constant parameter.
With my change, a `TIMESTAMP WITH LOCAL TIME ZONE` getter might pass it a
non-null `Calendar`, and that's the only time it could be non-null. So,
existing behavior cannot possibly be impacted by switch that minus to a plus.
Note that both a regular `TIMESTAMP` and a `TIMESTAMP WITH LOCAL TIME ZONE`
are represented in JDBC as `TIMESTAMP` (since there is no JDBC type for the
latter). I chose to re-use the existing timestamp getter with the addition of
this new `fixedInstant` parameter since it seemed like a straightforward
addition.
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081880447
##
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
-super(timeZone);
+// Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
I can't refute that with 100% certainty, but I will point out:
- Currently, at head, if you just make this 1 line change to pass
`UTC_CALENDAR` to the super constructor (here, the superclass being
`ArrayFactoryImpl`) and run all the tests, none of them fail.
- Alternatively, if you keep this 1 line as is, but go into
`AvaticaResultSetConversionTest` and delete the [`timeZone` property for the
connection][1], which is currently set to `"GMT"`, several `getString()` and
`getArray()` tests start to fail. The `getString()` failures are
self-explanatory, but the `getArray()` failures are tricky. They're
double-adjusting the timestamps:
- Once when they call [`getObject()`][2] (which invokes `getTimestamp()`
with the connection's default calendar, which was previously GMT but is now the
system default).
- Then, in [`ArrayImpl.equalContents()`][3] the arrays are converted to
result sets for traversal via [`Array.getResultSet()` ][4], which uses the same
time zone as the "factory" result set and eventually invokes `getObject()`
again, thus applying the time zone offset twice.
So, there's certainly an existing bug in the implementation of
`Array.getResultSet()` that only manifests when the array contains timestamps
and when the connection default time zone is not GMT, and this was not covered
by tests. By removing the explicit GMT `timeZone` for these tests, I'm covering
this case, and this seemed like the best solution. It's just hardcoding a UTC
time zone for the `ArrayFactoryImpl` parent of a `ResultSet`.
[1]:
https://github.com/apache/calcite-avatica/blob/dbe9b1d8c2e53474eb40cfaf5721aceca3bdb57f/core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetConversionsTest.java#L1121
[2]:
https://github.com/apache/calcite-avatica/blob/810acf80771310431d7ef576f3404299ebb8eaf2/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java#L1432
[3]:
https://github.com/apache/calcite-avatica/blob/820edf6f653607afb5a2a280a32f315aff1f64cb/core/src/main/java/org/apache/calcite/avatica/util/ArrayImpl.java#L233
[4]:
https://docs.oracle.com/javase/8/docs/api/java/sql/Array.html#getResultSet--
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081880447
##
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
-super(timeZone);
+// Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
I can't refute that with 100% certainty, but I will point out:
- Currently, at head, if you just make this 1 line change to pass
`UTC_CALENDAR` to the super constructor (here, the superclass being
`ArrayFactoryImpl`) and run all the tests, none of them fail.
- Alternatively, if you keep this 1 line as is, but go into
`AvaticaResultSetConversionTest` and delete the [`timeZone` property for the
connection][1], which is currently set to `"GMT"`, several `getString()` and
`getArray()` tests start to fail. The `getString()` failures are
self-explanatory, but the `getArray()` failures are tricky. They're
double-adjusting the timestamps:
- Once when they call [`getObject()`][2] (which invokes `getTimestamp()`
with the connection's default calendar, which was previously GMT but is now the
system default).
- Then, in [`ArrayImpl.equalContents()`][3] the arrays are converted to
result sets for traversal via [`Array.getResultSet()` ][4], which uses the same
time zone as the "factory" result set and eventually invokes `getObject()`
again, thus applying the time zone offset twice.
So, there's certainly an existing bug in the implementation of
`Array.getResultSet()` that only manifests when the array contains timestamps
and when the connection default time zone is not GMT, and this was not covered
by tests. By removing the explicit GMT `timeZone` for these tests, I'm covering
this case, and this seemed like the best solution. It's just hardcoding a UTC
time zone for `ArrayFactoryImpl` parent of a `ResultSet`.
[1]:
https://github.com/apache/calcite-avatica/blob/dbe9b1d8c2e53474eb40cfaf5721aceca3bdb57f/core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetConversionsTest.java#L1121
[2]:
https://github.com/apache/calcite-avatica/blob/810acf80771310431d7ef576f3404299ebb8eaf2/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java#L1432
[3]:
https://github.com/apache/calcite-avatica/blob/820edf6f653607afb5a2a280a32f315aff1f64cb/core/src/main/java/org/apache/calcite/avatica/util/ArrayImpl.java#L233
[4]:
https://docs.oracle.com/javase/8/docs/api/java/sql/Array.html#getResultSet--
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081852921
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
This is a private method, and all existing invocations of it pass it a null
`Calendar`. If you open Avatica at head in IntelliJ, it suggests inlining the
constant parameter.
With my change, a `TIMESTAMP WITH LOCAL TIME ZONE` getter might pass it a
non-null `Calendar`, and that's the only time it could be non-null.
Note that both a regular `TIMESTAMP` and a `TIMESTAMP WITH LOCAL TIME ZONE`
are represented in JDBC as `TIMESTAMP` (since there is no JDBC type for the
latter). I chose to re-use the existing timestamp getter with the addition of
this new `fixedInstant` parameter since it seemed like a straightforward
addition.
--
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-avatica] wnob commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081852921
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
This is a private method, and all existing invocations of it pass it a null
`Calendar`. If you open Avatica at head in IntelliJ, it suggests inlining the
constant parameter.
With my change, a `TIMESTAMP WITH LOCAL TIME ZONE` getter might pass it a
non-null `Calendar`, and that's the only time it could be non-null.
Note that both a regular `TIMESTAMP` and a `TIMESTAMP WITH LOCAL TIME ZONE`
are represented in JDBC as `TIMESTAMP` (since there is no JDBC type for the
latter. I chose to re-use the existing timestamp getter with the addition of
this new `fixedInstant` parameter since it seemed like a straightforward
addition.
--
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 a diff in pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
julianhyde commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1081846240
##
core/src/main/java/org/apache/calcite/sql/fun/SqlBigQueryFormatDatetimeFunction.java:
##
@@ -0,0 +1,111 @@
+/*
+ * 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.sql.fun;
+
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlOperandTypeChecker;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import java.util.Locale;
+
+import static org.apache.calcite.sql.type.SqlTypeName.DATE;
+import static org.apache.calcite.sql.type.SqlTypeName.TIME;
+
+/**
+ * The Google BigQuery style datetime formatting functions. This is a
generic type representing
+ * one of the following:
+ *
+ *
+ * {@code FORMAT_TIME(format_string, time_object)}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/time_functions#format_time";>ref
+ * {@code FORMAT_DATE(format_string, date_expr)}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/date_functions#format_date";>ref
+ * {@code FORMAT_TIMESTAMP(format_string, timestamp[, time_zone])}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/timestamp_functions#format_timestamp";>ref
+ * {@code FORMAT_DATETIME(format_string, timestamp[, time_zone])}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/datetime_functions#format_datetime";>ref
+ *
+ */
+public class SqlBigQueryFormatDatetimeFunction extends SqlFunction {
Review Comment:
However I would make the constructor private now. A public constructor will
prevent you from subclassing later. Use a `create` method instead, and add
`withXxx` methods to change the value of parameters that are not frequently
used. (Same pattern as `SqlBasicFunction`, and a pattern that is a little
nearer to functional programming than object-oriented.)
--
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 a diff in pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
julianhyde commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1081843370
##
core/src/main/java/org/apache/calcite/sql/fun/SqlBigQueryFormatDatetimeFunction.java:
##
@@ -0,0 +1,111 @@
+/*
+ * 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.sql.fun;
+
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlOperandCountRanges;
+import org.apache.calcite.sql.type.SqlOperandTypeChecker;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+
+import java.util.Locale;
+
+import static org.apache.calcite.sql.type.SqlTypeName.DATE;
+import static org.apache.calcite.sql.type.SqlTypeName.TIME;
+
+/**
+ * The Google BigQuery style datetime formatting functions. This is a
generic type representing
+ * one of the following:
+ *
+ *
+ * {@code FORMAT_TIME(format_string, time_object)}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/time_functions#format_time";>ref
+ * {@code FORMAT_DATE(format_string, date_expr)}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/date_functions#format_date";>ref
+ * {@code FORMAT_TIMESTAMP(format_string, timestamp[, time_zone])}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/timestamp_functions#format_timestamp";>ref
+ * {@code FORMAT_DATETIME(format_string, timestamp[, time_zone])}
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/datetime_functions#format_datetime";>ref
+ *
+ */
+public class SqlBigQueryFormatDatetimeFunction extends SqlFunction {
Review Comment:
Parsing/formatting functions might have enough commonality to warrant a
common class - an extension to `SqlBasicFunction` with an extra field, a map
from strings to format elements.
I wouldn't change it now. But come back and refactor if that pattern emerges.
--
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 a diff in pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
julianhyde commented on code in PR #3034: URL: https://github.com/apache/calcite/pull/3034#discussion_r1081837725 ## core/src/main/java/org/apache/calcite/sql/SqlDialect.java: ## @@ -1002,6 +1004,18 @@ protected static void unparseOffset(SqlWriter writer, @Nullable SqlNode offset) } } + /** Review Comment: There's a complex mapping between format elements and dialects. But I suspect that SqlDialect is not the right place to do it, and functions is closer to the right place. Hypothetically, someone could be in MySQL dialect and want to enable MySQL's and Postgres's formatting functions. It's a useful hypothetical. SqlDialect is used only for a short part of the query life cycle - when going from AST nodes to SQL text. But functions are around for longer. I have in mind a map from strings to format elements that is part of the `SqlLibraryOperators.FORMAT_TIMESTAMP` function, and the same or similar map that is used by `FORMAT_TIME` and `PARSE_TIMESTAMP` functions. Parse functions might have an additional parser, because parsing format strings is not always trivial. Maybe the aforementioned 'map' and 'parser' would evolve into slightly larger classes. When we tackle the different issue for 'how do I translate a MySQL format string to a BigQuery format string?' - or 'how do I translate a call to MySQL_FORMAT_TIMESTAMP to a call to BigQuery_FORMAT_TIMESTAMP' - then those larger classes could be used to drive the proceedings. -- 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-avatica] julianhyde commented on a diff in pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
julianhyde commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081816343
##
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
-super(timeZone);
+// Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
I think you just tore down a Chesterton's fence.
##
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
after this change the method name is no longer appropriate.
the purpose of this method is to access a SQL `TIMESTAMP` value as a string
you have added logic to access a SQL `TIMESTAMP WITH LOCAL TIME ZONE` value
as a string. and paved over the old functionality, which we still 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.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite-avatica] wnob closed pull request #186: [CALCITE-5308] Add support for TIMESTAMP WITH LOCAL TIME ZONE and microsecond-precision
wnob closed pull request #186: [CALCITE-5308] Add support for TIMESTAMP WITH LOCAL TIME ZONE and microsecond-precision URL: https://github.com/apache/calcite-avatica/pull/186 -- 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-avatica] wnob commented on pull request #186: [CALCITE-5308] Add support for TIMESTAMP WITH LOCAL TIME ZONE and microsecond-precision
wnob commented on PR #186: URL: https://github.com/apache/calcite-avatica/pull/186#issuecomment-1397531597 Superceded by https://github.com/apache/calcite-avatica/pull/205 which focuses just on the type support and punts on precision for now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite-avatica] wnob opened a new pull request, #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE
wnob opened a new pull request, #205: URL: https://github.com/apache/calcite-avatica/pull/205 This is a work-in-progress as there are still some tests failing, but I believe the proper semantics are now codified in `AvaticaResultSetConversionsTest`. Was hoping to get some early feedback to see if I was barking up the wrong tree with these changes. -- 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] sonarcloud[bot] commented on pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
sonarcloud[bot] commented on PR #3035: URL: https://github.com/apache/calcite/pull/3035#issuecomment-1397056502 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3035) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_duplicated_lines_density&view=list) -- 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] Aitozi commented on a diff in pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
Aitozi commented on code in PR #3035:
URL: https://github.com/apache/calcite/pull/3035#discussion_r1081278261
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -3002,6 +3002,32 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.withProgram(program).check();
}
+ @Test void testReduceConstantsProject() {
Review Comment:
Thanks for your suggestion, updated. And I rebase to update the commit
message, please take a look again.
--
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] libenchao commented on a diff in pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
libenchao commented on code in PR #3035:
URL: https://github.com/apache/calcite/pull/3035#discussion_r1081180900
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -3002,6 +3002,32 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.withProgram(program).check();
}
+ @Test void testReduceConstantsProject() {
Review Comment:
how about name it more specifically, e.g.
`testReducingConstantsInferedFromCorrelate`?
--
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] sonarcloud[bot] commented on pull request #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
sonarcloud[bot] commented on PR #3035: URL: https://github.com/apache/calcite/pull/3035#issuecomment-1396589034 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3035) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3035&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3035&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3035&metric=new_duplicated_lines_density&view=list) -- 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] Aitozi opened a new pull request, #3035: [CALCITE-5466] Fix the constant condition can't be reduced after corr…
Aitozi opened a new pull request, #3035:
URL: https://github.com/apache/calcite/pull/3035
…elate
Before fix the sql in test will be optimize to:
```
LogicalProject(ENAME=[$0], EMPNO=[$1], EMPNO_R=[$3], TYPE=[CASE(=($2,
'bounded'), 1, 2)])
LogicalCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{1}])
LogicalProject(ENAME=[$1], EMPNO=[$0], __SOURCE__TYPE__=['bounded'])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalTableFunctionScan(invocation=[RAMP($cor0.EMPNO)],
rowType=[RecordType(INTEGER I)])
```
Now, the constant condition can be reduced:
```
LogicalProject(ENAME=[$0], EMPNO=[$1], EMPNO_R=[$3], TYPE=[1])
LogicalCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{1}])
LogicalProject(ENAME=[$1], EMPNO=[$0], __SOURCE__TYPE__=['bounded'])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalTableFunctionScan(invocation=[RAMP($cor0.EMPNO)],
rowType=[RecordType(INTEGER I)])
```
--
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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080688763
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -240,6 +281,17 @@ public MysqlSqlDialect(Context context) {
unparseListAggCall(writer, call, null, leftPrec, rightPrec);
break;
+case FORMAT_DATE:
+case FORMAT_TIME:
+case FORMAT_TIMESTAMP:
+case FORMAT_DATETIME:
+ writer.print("DATE_FORMAT(");
Review Comment:
Or better yet convert these BQ format functions into the standard `CAST(...
as FORMAT )` and unparse the `CAST` to `DATE_FORMAT`, `TO_CHAR`
accordingly.
--
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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080677876
##
core/src/main/java/org/apache/calcite/util/format/FormatModelElementAlias.java:
##
@@ -0,0 +1,76 @@
+/*
+ * 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.util.format;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.StringJoiner;
+
+/**
+ * Represents a format element comprised of one or more {@link
FormatElementEnum} entries.
+ */
+public class FormatModelElementAlias implements FormatModelElement {
Review Comment:
Yeah exactly - it maps to one or more FormatModelElementEnum entries.
For example in Google SQL `%R` is the same as `HH24:MI`. I'll add more
context/examples in the javadoc
--
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] olivrlee commented on a diff in pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
olivrlee commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080644292
##
core/src/main/java/org/apache/calcite/util/format/FormatModelElementAlias.java:
##
@@ -0,0 +1,76 @@
+/*
+ * 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.util.format;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.StringJoiner;
+
+/**
+ * Represents a format element comprised of one or more {@link
FormatElementEnum} entries.
+ */
+public class FormatModelElementAlias implements FormatModelElement {
Review Comment:
what does this `FormatModelElementAlias` actually represent? a collection of
`FormatModelElement`s?
--
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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080614602
##
core/src/main/java/org/apache/calcite/sql/FormatModel.java:
##
@@ -0,0 +1,154 @@
+/*
+ * 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.sql;
+
+import static org.apache.calcite.util.format.FormatElementEnum.D;
+import static org.apache.calcite.util.format.FormatElementEnum.DAY;
+import static org.apache.calcite.util.format.FormatElementEnum.DD;
+import static org.apache.calcite.util.format.FormatElementEnum.DDD;
+import static org.apache.calcite.util.format.FormatElementEnum.DY;
+import static org.apache.calcite.util.format.FormatElementEnum.HH24;
+import static org.apache.calcite.util.format.FormatElementEnum.IW;
+import static org.apache.calcite.util.format.FormatElementEnum.MI;
+import static org.apache.calcite.util.format.FormatElementEnum.MM;
+import static org.apache.calcite.util.format.FormatElementEnum.MON;
+import static org.apache.calcite.util.format.FormatElementEnum.MONTH;
+import static org.apache.calcite.util.format.FormatElementEnum.Q;
+import static org.apache.calcite.util.format.FormatElementEnum.SS;
+import static org.apache.calcite.util.format.FormatElementEnum.TZR;
+import static org.apache.calcite.util.format.FormatElementEnum.WW;
+import static org.apache.calcite.util.format.FormatElementEnum.;
+
+import org.apache.calcite.sql.fun.SqlLibrary;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeTransforms;
+import org.apache.calcite.util.format.FormatElementEnum;
+import org.apache.calcite.util.format.FormatModelElement;
+import org.apache.calcite.util.format.FormatModelElementAlias;
+import org.apache.calcite.util.format.FormatModelElementLiteral;
+import org.apache.calcite.util.format.FormatModelUtil;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import java.util.Arrays;
+import java.util.List;
+
+
+/** A https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlqr/Format-Models.html";>
+ * format model is a character literal that describes the format of {@code
DATETIME} or {@code
+ * NUMBER} data stored in a character string.
+ *
+ * {@link #unparse(SqlWriter, SqlCall, int, int)} calls
+ * {@link SqlDialect#getFormatElement(FormatElementEnum)} for known elements
and aliases. Consider
+ * overriding this method if a dialect's format elements differs from those in
{@link
+ * FormatElementEnum}
+ */
+public class FormatModel extends SqlInternalOperator {
+
+ private List elements;
+ private ImmutableMap fmtModelParseMap;
+
+ /**
+ * TODO(CALCITE-2980): This should live elsewhere and be associated with
{@link SqlLibrary}
+ * or {@link org.apache.calcite.config.Lex}.
+ */
+ public static final ImmutableMap
BIG_QUERY_FORMAT_ELEMENT_PARSE_MAP =
Review Comment:
For sure, I can open a Jira case to swap `GOOGLE_SQL` or `GSQL` with
`BIG_QUERY` references -- would have to a non-breaking change for config
settings etc.
--
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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080608342
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -240,6 +281,17 @@ public MysqlSqlDialect(Context context) {
unparseListAggCall(writer, call, null, leftPrec, rightPrec);
break;
+case FORMAT_DATE:
+case FORMAT_TIME:
+case FORMAT_TIMESTAMP:
+case FORMAT_DATETIME:
+ writer.print("DATE_FORMAT(");
Review Comment:
Yeah that's open for discussion -- how to best swap a `FORMAT_TIMESTAMP`
Google SQL style function call to any other dialect. Snowflake for example uses
`TO_CHAR` or `TO_VARCHAR`.
Maybe a generic entry in `SqlKind` called `GOOGLE_SQL_DATETIME_FORMAT`?
Could even be more generic like `DATETIME_FORMAT` and then each dialect
would know how to unparse.
--
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] mkou commented on a diff in pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
mkou commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1080572882
##
core/src/main/java/org/apache/calcite/sql/FormatModel.java:
##
@@ -0,0 +1,154 @@
+/*
+ * 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.sql;
+
+import static org.apache.calcite.util.format.FormatElementEnum.D;
+import static org.apache.calcite.util.format.FormatElementEnum.DAY;
+import static org.apache.calcite.util.format.FormatElementEnum.DD;
+import static org.apache.calcite.util.format.FormatElementEnum.DDD;
+import static org.apache.calcite.util.format.FormatElementEnum.DY;
+import static org.apache.calcite.util.format.FormatElementEnum.HH24;
+import static org.apache.calcite.util.format.FormatElementEnum.IW;
+import static org.apache.calcite.util.format.FormatElementEnum.MI;
+import static org.apache.calcite.util.format.FormatElementEnum.MM;
+import static org.apache.calcite.util.format.FormatElementEnum.MON;
+import static org.apache.calcite.util.format.FormatElementEnum.MONTH;
+import static org.apache.calcite.util.format.FormatElementEnum.Q;
+import static org.apache.calcite.util.format.FormatElementEnum.SS;
+import static org.apache.calcite.util.format.FormatElementEnum.TZR;
+import static org.apache.calcite.util.format.FormatElementEnum.WW;
+import static org.apache.calcite.util.format.FormatElementEnum.;
+
+import org.apache.calcite.sql.fun.SqlLibrary;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeTransforms;
+import org.apache.calcite.util.format.FormatElementEnum;
+import org.apache.calcite.util.format.FormatModelElement;
+import org.apache.calcite.util.format.FormatModelElementAlias;
+import org.apache.calcite.util.format.FormatModelElementLiteral;
+import org.apache.calcite.util.format.FormatModelUtil;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import java.util.Arrays;
+import java.util.List;
+
+
+/** A https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlqr/Format-Models.html";>
+ * format model is a character literal that describes the format of {@code
DATETIME} or {@code
+ * NUMBER} data stored in a character string.
+ *
+ * {@link #unparse(SqlWriter, SqlCall, int, int)} calls
+ * {@link SqlDialect#getFormatElement(FormatElementEnum)} for known elements
and aliases. Consider
+ * overriding this method if a dialect's format elements differs from those in
{@link
+ * FormatElementEnum}
+ */
+public class FormatModel extends SqlInternalOperator {
+
+ private List elements;
+ private ImmutableMap fmtModelParseMap;
+
+ /**
+ * TODO(CALCITE-2980): This should live elsewhere and be associated with
{@link SqlLibrary}
+ * or {@link org.apache.calcite.config.Lex}.
+ */
+ public static final ImmutableMap
BIG_QUERY_FORMAT_ELEMENT_PARSE_MAP =
Review Comment:
nit: I know it's everywhere so no need to change it but in general I hake a
bit when I see BigQuery since we really want to do "Google SQL"
##
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##
@@ -271,6 +272,50 @@ private static TimeUnit validate(TimeUnit timeUnit) {
}
}
+ /** {@inheritDoc}
+ *
+ * BigQuery format element reference:
+ * https://cloud.google.com/bigquery/docs/reference/standard-sql/format-elements";>
+ * BigQuery Standard SQL Format Elements.
+ */
+ @Override public String getFormatElement(FormatElementEnum fmtElement) {
Review Comment:
Yes I like this 🌷
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -240,6 +281,17 @@ public MysqlSqlDialect(Context context) {
unparseListAggCall(writer, call, null, leftPrec, rightPrec);
break;
+case FORMAT_DATE:
+case FORMAT_TIME:
+case FORMAT_TIMESTAMP:
+case FORMAT_DATETIME:
+ writer.print("DATE_FORMAT(");
Review Comment:
Like all of them should alias to the same canonical form so you can read the
canonical form in every language idk if that makes sense
##
core/src/main/java/org/apach
[GitHub] [calcite] wnob commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
wnob commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1074030646
##
core/src/main/java/org/apache/calcite/sql/SqlUnknownLiteral.java:
##
@@ -0,0 +1,68 @@
+/*
+ * 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.sql;
+
+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.util.NlsString;
+import org.apache.calcite.util.Util;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Literal whose type is not yet known.
+ */
+public class SqlUnknownLiteral extends SqlLiteral {
+ public final String tag;
+
+ SqlUnknownLiteral(String tag, String value, SqlParserPos pos) {
+super(requireNonNull(value, "value"), SqlTypeName.UNKNOWN, pos);
+this.tag = requireNonNull(tag, "tag");
+ }
+
+ @Override public String getValue() {
+return (String) requireNonNull(super.getValue(), "value");
+ }
+
+ @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec)
{
+final NlsString nlsString = new NlsString(getValue(), null, null);
+writer.keyword(tag);
+writer.literal(nlsString.asSql(true, true, writer.getDialect()));
+ }
+
+
+ /** Converts this unknown literal to a literal of known type. */
+ public SqlLiteral resolve(SqlTypeName typeName) {
+switch (typeName) {
+case DATE:
+ return SqlParserUtil.parseDateLiteral(getValue(), pos);
+case TIME:
+ return SqlParserUtil.parseTimeLiteral(getValue(), pos);
+case TIMESTAMP:
+ return SqlParserUtil.parseTimestampLiteral(getValue(), pos);
+case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+ return SqlParserUtil.parseTimestampWithLocalTimeZoneLiteral(getValue(),
pos);
+default:
Review Comment:
See in `SqlValidatorImpl.resolveLiteral` (which calls this function):
```
final SqlIdentifier identifier =
new SqlIdentifier(unknownLiteral.tag, SqlParserPos.ZERO);
final @Nullable RelDataType type =
catalogReader.getNamedType(identifier);
final SqlTypeName typeName;
if (type != null) {
typeName = type.getSqlTypeName();
} else {
typeName = SqlTypeName.lookup(unknownLiteral.tag);
}
return unknownLiteral.resolve(typeName);
```
So, it makes sense to call `SqlLiteral.createUnknown("DATETIME")`, passing
"DATETIME" as the `tag`. The tag just says "this is what the type called
itself", and in order to resolve it, we look up in `catalogReader` to see if
it's mapped to anything. In the case of BQ, it will map to `TIMESTAMP`. In the
case of everything else, it would be null, so validation would fail when it
invokes `SqlTypeName.lookup(unknownLiteral.tag)`.
--
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 a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
wnob commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1074027016
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4609,15 +4610,116 @@ SqlLiteral DateTimeLiteral() :
}
|
{ s = span(); } p = SimpleStringLiteral() {
- return SqlParserUtil.parseDateLiteral(p, s.end(this));
+ return SqlLiteral.createUnknown("DATE", p, s.end(this));
+}
+|
+ { s = span(); } p = SimpleStringLiteral() {
+return SqlLiteral.createUnknown("DATETIME", p, s.end(this));
}
|
{ s = span(); } p = SimpleStringLiteral() {
-return SqlParserUtil.parseTimeLiteral(p, s.end(this));
+ return SqlLiteral.createUnknown("TIME", p, s.end(this));
}
|
+LOOKAHEAD(2)
{ s = span(); } p = SimpleStringLiteral() {
-return SqlParserUtil.parseTimestampLiteral(p, s.end(this));
+return SqlLiteral.createUnknown("TIMESTAMP", p, s.end(this));
+}
+|
+ { s = span(); } p =
SimpleStringLiteral() {
+return SqlLiteral.createUnknown("TIMESTAMP WITH LOCAL TIME ZONE", p,
s.end(this));
+}
+}
+
+/**
+ * Parses BigQuery's built-in DATE() function.
+ */
+SqlNode DateFunctionCall() :
+{
+final SqlFunctionCategory funcType = SqlFunctionCategory.TIMEDATE;
+final SqlIdentifier qualifiedName;
+final Span s;
+final SqlLiteral quantifier;
+final List args;
+}
+{
+ {
+s = span();
+qualifiedName = new SqlIdentifier(unquotedIdentifier(), getPos());
+}
+args = FunctionParameterList(ExprContext.ACCEPT_SUB_QUERY) {
+quantifier = (SqlLiteral) args.get(0);
+args.remove(0);
Review Comment:
If you look at the production for `FunctionParameterList()` you'll see it
start like this:
```
{
(
qualifier = AllOrDistinct() { list.add(qualifier); }
|
{ list.add(null); }
)
...
```
Here, `list` is the return value (`args` here in `DateFunctionCall()`). So
the first argument is always either `null`, or either `ALL` or `DISTINCT`,
since some functions can take those qualifiers. Here, it should always be
`null`, although I suppose this means the parser would silently accept
something like `DATE(DISTINCT 2023, 01, 18)`, which it probably shouldn't.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tanclary commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1073952163
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -184,6 +185,46 @@ public MysqlSqlDialect(Context context) {
return super.getCastSpec(type);
}
+ /** {@inheritDoc}
+ *
+ * MySQL format element reference:
+ * https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_date-format";>
+ * MySQL Date and Time Functions.
+ */
+ @Override public String getFormatElement(FormatElementEnum fmtElement) {
Review Comment:
Makes sense! Looks great.
--
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 pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on PR #3034: URL: https://github.com/apache/calcite/pull/3034#issuecomment-1387591383 @tanclary yes I will add those. Thanks for reminding me! -- 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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1073950252
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -184,6 +185,46 @@ public MysqlSqlDialect(Context context) {
return super.getCastSpec(type);
}
+ /** {@inheritDoc}
+ *
+ * MySQL format element reference:
+ * https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_date-format";>
+ * MySQL Date and Time Functions.
+ */
+ @Override public String getFormatElement(FormatElementEnum fmtElement) {
Review Comment:
Each dialect might have a different set of elements to represent the same
type of datetime unit.
For example an abbreviated month name like `Jan` is `%b` in MySQL and `MON`
in Snowflake. Each `SqlDialect` will have to override this method to supply the
correct string for the element type. I only did MySQL, BigQuery, and Snowflake
in the first commit because I wanted to get some feedback before working on
each dialect.
Oracle's format elements were chosen as the "base" set since they map
closely to the SQL:2016 standard as described in
[CALCITE-2980](https://issues.apache.org/jira/browse/CALCITE-2980).
--
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] birschick-bq commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
birschick-bq commented on PR #3031: URL: https://github.com/apache/calcite/pull/3031#issuecomment-1387586998 cc: @CassieLyu / @sunnie629 -- 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 pull request #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tanclary commented on PR #3034: URL: https://github.com/apache/calcite/pull/3034#issuecomment-1387559274 Should you add descriptions for the functions to reference.md? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. 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 #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tanclary commented on code in PR #3034:
URL: https://github.com/apache/calcite/pull/3034#discussion_r1073936097
##
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##
@@ -184,6 +185,46 @@ public MysqlSqlDialect(Context context) {
return super.getCastSpec(type);
}
+ /** {@inheritDoc}
+ *
+ * MySQL format element reference:
+ * https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_date-format";>
+ * MySQL Date and Time Functions.
+ */
+ @Override public String getFormatElement(FormatElementEnum fmtElement) {
Review Comment:
Why does this logic go in MySqlSqlDialect?
--
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 opened a new pull request, #3034: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart opened a new pull request, #3034: URL: https://github.com/apache/calcite/pull/3034 Lays the groundwork for [CALCITE-2980](https://issues.apache.org/jira/browse/CALCITE-2980) -- 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 closed pull request #3033: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart closed pull request #3033: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME URL: https://github.com/apache/calcite/pull/3033 -- 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 opened a new pull request, #3033: [CALCITE-5357] Implement BigQuery FORMAT_TIMESTAMP, FORMAT_DATE, FORMAT_TIME, and FORMAT_DATETIME
tjbanghart opened a new pull request, #3033: URL: https://github.com/apache/calcite/pull/3033 Lays the groundwork for [CALCITE-2980](https://issues.apache.org/jira/browse/CALCITE-2980) -- 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, #3032: [CALCITE-5484] Implement BigQuery DATETIME_SUB()
tanclary opened a new pull request, #3032: URL: https://github.com/apache/calcite/pull/3032 Add support for BigQuery's DATETIME_SUB() 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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
sonarcloud[bot] commented on PR #3031: URL: https://github.com/apache/calcite/pull/3031#issuecomment-1386252119 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3031) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=list) -- 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] olivrlee commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
olivrlee commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1072911432
##
babel/src/test/resources/sql/big-query.iq:
##
@@ -1097,12 +1097,74 @@ select unix_date(datetime '2008-12-25') as d;
#
# Return Data Type: DATE
-select date(2022, 11, 15) as d;
-++
-| d |
-++
-| 2022-11-15 |
-++
+select date(2022, 11, 15) as d1,
+ date(datetime "2008-01-01 01:03:05") as d2,
+ date(datetime(2008, 1, 1, 1, 3, 5)) as d3;
+++++
+| d1 | d2 | d3 |
+++++
+| 2022-11-15 | 2008-01-01 | 2008-01-01 |
+++++
+(1 row)
+
+!ok
+
+# Test timezone conversion when converting TIMESTAMP to DATE.
+# Denver observes DST whereas Phoenix does not.
+# Both cities have a -07:00 offset in winter, but Denver has -06:00 in summer.
+select date(timestamp("2008-06-21 06:30:00")) as sum_utc,
+ date(timestamp("2008-06-21 06:30:00"), "America/Denver") as sum_dst,
+ date(timestamp("2008-06-21 06:30:00"), "America/Phoenix") as sum_std,
+ date(timestamp("2008-12-21 06:30:00")) as win_utc,
+ date(timestamp("2008-12-21 06:30:00"), "America/Denver") as win_dst,
+ date(timestamp("2008-12-21 06:30:00"), "America/Phoenix") as win_std;
++++++++
+| sum_utc| sum_dst| sum_std| win_utc| win_dst| win_std|
++++++++
+| 2008-06-21 | 2008-06-21 | 2008-06-20 | 2008-12-21 | 2008-12-20 | 2008-12-20 |
++++++++
+(1 row)
+
+!ok
+
+#
+# DATETIME
Review Comment:
there's a set of DATETIME tests currently disabled around line 650 btw
https://github.com/apache/calcite/blob/6e5b2f22414b1c1617d0e721d6dcf93f1b82d673/babel/src/test/resources/sql/big-query.iq#L649
--
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] birschick-bq commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
birschick-bq commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072879759
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
&& operands.get(1).getKind() == SqlKind.CAST
&& ((RexCall) operands.get(1)).operands.get(0).getKind()
== SqlKind.INPUT_REF
- && operands.get(2).getKind() == SqlKind.LITERAL) {
+ && operands.get(2).getKind() == SqlKind.LITERAL
+ && operands.get(2).getType().getFamily() ==
SqlTypeFamily.NUMERIC) {
Review Comment:
@clesaec
I agree with you on this. I believe there could be a conversion for
`BigDecimal` from `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] clesaec commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
clesaec commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072284872
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
&& operands.get(1).getKind() == SqlKind.CAST
&& ((RexCall) operands.get(1)).operands.get(0).getKind()
== SqlKind.INPUT_REF
- && operands.get(2).getKind() == SqlKind.LITERAL) {
+ && operands.get(2).getKind() == SqlKind.LITERAL
+ && operands.get(2).getType().getFamily() ==
SqlTypeFamily.NUMERIC) {
Review Comment:
I wonder where the code `literal.getValueAs(BigDecimal.class)` (line 111)
could work as [getValueAs
method](https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L1029)
has no case for "clazz == BigDecimal" ... May be not related to this specific
case, but it seems there is also an issue on RexLiteral class.
--
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] zabetak closed pull request #3027: [CALCITE-5475] Improve test coverage accuracy by aggregating modules
zabetak closed pull request #3027: [CALCITE-5475] Improve test coverage accuracy by aggregating modules URL: https://github.com/apache/calcite/pull/3027 -- 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] sonarcloud[bot] commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
sonarcloud[bot] commented on PR #3031: URL: https://github.com/apache/calcite/pull/3031#issuecomment-1384684549 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3031) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=list) -- 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 closed pull request #3013: CALCITE-5451 / Implement LPAD() and RPAD()
julianhyde closed pull request #3013: CALCITE-5451 / Implement LPAD() and RPAD() URL: https://github.com/apache/calcite/pull/3013 -- 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] sonarcloud[bot] commented on pull request #2942: (do not merge)
sonarcloud[bot] commented on PR #2942: URL: https://github.com/apache/calcite/pull/2942#issuecomment-1384677595 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2942) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [66.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) [21.2% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) -- 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] birschick-bq opened a new pull request, #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
birschick-bq opened a new pull request, #3031: URL: https://github.com/apache/calcite/pull/3031 Resolve issue with thrown exception if literal is non-numeric. https://issues.apache.org/jira/browse/CALCITE-5483 -- 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 #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
julianhyde commented on PR #3023: URL: https://github.com/apache/calcite/pull/3023#issuecomment-1384675118 There's too much going on here for me to review. Can you separate it out into pieces that can be understood by an end user as adding some useful functionality? If there refactorings that don't change functionality, keep those separate. It's confusing that there are type aliases, and there are also functions that are named after types. Basically you need to craft commit messages that are suitable for the release notes. (And each such commit should be a jira case that explains the new functionality.) -- 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 closed pull request #2992: [CALCITE-5404] Implement BigQuery's POW() and TRUNC() math functions
julianhyde closed pull request #2992: [CALCITE-5404] Implement BigQuery's POW() and TRUNC() math functions URL: https://github.com/apache/calcite/pull/2992 -- 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] sonarcloud[bot] commented on pull request #2942: (do not merge)
sonarcloud[bot] commented on PR #2942: URL: https://github.com/apache/calcite/pull/2942#issuecomment-1384596408 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2942) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [19.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) [54.2% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) -- 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] rubenada merged pull request #3026: [CALCITE-5471] RelSupplier.SqlRelSupplier#apply should use .project(), not .rel
rubenada merged PR #3026: URL: https://github.com/apache/calcite/pull/3026 -- 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] sonarcloud[bot] commented on pull request #3026: [CALCITE-5471] RelSupplier.SqlRelSupplier#apply should use .project(), not .rel
sonarcloud[bot] commented on PR #3026: URL: https://github.com/apache/calcite/pull/3026#issuecomment-1383852769 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3026) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_duplicated_lines_density&view=list) -- 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] sonarcloud[bot] commented on pull request #3027: [CALCITE-5475] Improve test coverage accuracy by aggregating modules
sonarcloud[bot] commented on PR #3027: URL: https://github.com/apache/calcite/pull/3027#issuecomment-1383666026 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3027) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3027&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3027&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3027&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3027&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3027&metric=coverage&view=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3027&metric=duplicated_lines_density&view=list) No Duplication information -- 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] zoudan commented on a diff in pull request #2981: [CALCITE-5283] Add ARG_MIN, ARG_MAX aggregate functions
zoudan commented on code in PR #2981:
URL: https://github.com/apache/calcite/pull/2981#discussion_r1070765927
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1849,6 +1849,17 @@ public static > T greatest(T b0,
T b1) {
return b0 == null || b1 != null && b0.compareTo(b1) < 0 ? b1 : b0;
}
+ /** Less than. */
+ public static > boolean lessThan(T b0, T b1) {
+return b1 == null || b0 != null && b0.compareTo(b1) < 0;
+ }
+
+ /** Grater than. */
+ public static > boolean greaterThan(T b0, T b1) {
+return b1 == null || b0 != null && b0.compareTo(b1) > 0;
Review Comment:
`greaterThan` return wether b0 is greater than b1 while `greatest` return
the larger one between b0 and b1
--
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-avatica] julianhyde closed pull request #179: [CALCITE-3557] ClassCastException for using nested multiset or array …
julianhyde closed pull request #179: [CALCITE-3557] ClassCastException for using nested multiset or array … URL: https://github.com/apache/calcite-avatica/pull/179 -- 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-avatica] julianhyde closed pull request #201: [CALCITE-5443] Reset update count when checking for more results
julianhyde closed pull request #201: [CALCITE-5443] Reset update count when checking for more results URL: https://github.com/apache/calcite-avatica/pull/201 -- 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] dssysolyatin merged pull request #2974: [CALCITE-5209] Proper sub-query handling if it is used inside select list and group by
dssysolyatin merged PR #2974: URL: https://github.com/apache/calcite/pull/2974 -- 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] libenchao commented on pull request #2974: [CALCITE-5209] Proper sub-query handling if it is used inside select list and group by
libenchao commented on PR #2974: URL: https://github.com/apache/calcite/pull/2974#issuecomment-1383136327 LGTM~ -- 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] sonarcloud[bot] commented on pull request #2974: [CALCITE-5209] Proper sub-query handling if it is used inside select list and group by
sonarcloud[bot] commented on PR #2974: URL: https://github.com/apache/calcite/pull/2974#issuecomment-1383109216 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2974) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_coverage&view=list) [96.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_duplicated_lines_density&view=list) -- 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] dssysolyatin commented on a diff in pull request #2974: [CALCITE-5209] Proper sub-query handling if it is used inside select list and group by
dssysolyatin commented on code in PR #2974: URL: https://github.com/apache/calcite/pull/2974#discussion_r1070548010 ## core/src/test/resources/sql/agg.iq: ## @@ -3467,4 +3467,38 @@ order by ename, deptno; !ok +# Test cases for [CALCITE-5209] ArrayIndexOutOfBoundsException during SqlToRelConverter for group-by on `case` +# having `in` expression predicates exceeding SqlRelConverter.Config InSubQueryThreshold +!use scott +select +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end +from emp +group by +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end; + +++ +| EXPR$0 | +++ +| 0 | +++ +(1 row) + +!ok + +!set insubquerythreshold 5 +select +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end Review Comment: @libenchao Done. I also rebased branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #2974: [CALCITE-5209] Proper sub-query handling if it is used inside select list and group by
sonarcloud[bot] commented on PR #2974: URL: https://github.com/apache/calcite/pull/2974#issuecomment-1383106352 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2974) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2974&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2974&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_coverage&view=list) [96.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2974&metric=new_duplicated_lines_density&view=list) -- 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] libenchao commented on a diff in pull request #2974: [CALCITE-5209] ArrayIndexOutOfBoundsException during SqlToRelConverter for group-by on `case` having `in` expression predicates
libenchao commented on code in PR #2974: URL: https://github.com/apache/calcite/pull/2974#discussion_r1070491944 ## core/src/test/resources/sql/agg.iq: ## @@ -3467,4 +3467,38 @@ order by ename, deptno; !ok +# Test cases for [CALCITE-5209] ArrayIndexOutOfBoundsException during SqlToRelConverter for group-by on `case` +# having `in` expression predicates exceeding SqlRelConverter.Config InSubQueryThreshold +!use scott +select +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end +from emp +group by +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end; + +++ +| EXPR$0 | +++ +| 0 | +++ +(1 row) + +!ok + +!set insubquerythreshold 5 +select +case when deptno in (1, 2, 3, 4, 5) THEN 1 else 0 end Review Comment: This looks better! -- 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] libenchao commented on a diff in pull request #2932: [CALCITE-5304] RelJson should support RexSubQuery
libenchao commented on code in PR #2932:
URL: https://github.com/apache/calcite/pull/2932#discussion_r1070270379
##
core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java:
##
@@ -122,10 +127,32 @@ public RelJson withInputTranslator(InputTranslator
inputTranslator) {
return new RelJson(jsonBuilder, inputTranslator);
}
+ /** Returns a RelJson with a given RelJsonReader. */
+ @SuppressWarnings("initialization.invalid.field.write.initialized")
+ public RelJson withRelJsonReader(@UnknownInitialization RelJsonReader
relJsonReader) {
+this.relJsonReader = relJsonReader;
+return this;
+ }
+
+ /** Returns a RelJson with a given RelJsonWriter. */
+ @SuppressWarnings("initialization.invalid.field.write.initialized")
+ public RelJson withRelJsonWriter(@UnknownInitialization RelJsonWriter
relJsonWriter) {
+this.relJsonWriter = relJsonWriter;
+return this;
+ }
+
private JsonBuilder jsonBuilder() {
return requireNonNull(jsonBuilder, "jsonBuilder");
}
+ private RelJsonWriter relJsonWriter() {
+return requireNonNull(relJsonWriter, "relJsonWriter");
+ }
+
+ private RelJsonReader relJsonReader() {
Review Comment:
Because there is a `RelNode` in `RexSubQuery`, to serialize/deserialize a
`RelNode`, we need to use `RelJsonWriter` and `RelJsonReader`.
--
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] sonarcloud[bot] commented on pull request #3030: [CALCITE-5479] Adjust handling of iFormalOperand in sequence and interval checkers.
sonarcloud[bot] commented on PR #3030: URL: https://github.com/apache/calcite/pull/3030#issuecomment-1382697697 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3030) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3030&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3030&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3030&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3030&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3030&metric=new_coverage&view=list) [70.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3030&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3030&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3030&metric=new_duplicated_lines_density&view=list) -- 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] sonarcloud[bot] commented on pull request #3029: [CALCITE-5478] Use highest input precision for datetimes in SqlTypeFactoryImpl.leastRestrictive.
sonarcloud[bot] commented on PR #3029: URL: https://github.com/apache/calcite/pull/3029#issuecomment-1382695349 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3029) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3029&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3029&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3029&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3029&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3029&metric=new_coverage&view=list) [92.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3029&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3029&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3029&metric=new_duplicated_lines_density&view=list) -- 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] gianm opened a new pull request, #3030: [CALCITE-5479] Adjust handling of iFormalOperand in sequence and interval checkers.
gianm opened a new pull request, #3030: URL: https://github.com/apache/calcite/pull/3030 1) Revert change from 33f4ab40bbee26e06209061c35a422f2f1e05371 that sent nonzero formal operands to any checkers in a sequence that are not FamilyOperandTypeChecker. 2) Adjust IntervalOperandTypeChecker to always expect iFormalOperand as zero. Based on my understanding of how `iFormalOperand` and sequence checkers are supposed to work, these two changes are both bug fixes. -- 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] sonarcloud[bot] commented on pull request #3028: [CALCITE-5477] Prefer Util.checkArgument over Preconditions.checkArgument
sonarcloud[bot] commented on PR #3028: URL: https://github.com/apache/calcite/pull/3028#issuecomment-1382693927 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3028) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3028&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3028&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3028&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3028&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3028&metric=new_coverage&view=list) [67.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3028&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3028&metric=new_duplicated_lines_density&view=list) [0.7% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3028&metric=new_duplicated_lines_density&view=list) -- 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] gianm opened a new pull request, #3029: [CALCITE-5478] Use highest input precision for datetimes in SqlTypeFactoryImpl.leastRestrictive.
gianm opened a new pull request, #3029: URL: https://github.com/apache/calcite/pull/3029 Some code exists to do this for other types, but not datetimes. -- 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 #3013: CALCITE-5451 / Implement LPAD() and RPAD()
julianhyde commented on PR #3013: URL: https://github.com/apache/calcite/pull/3013#issuecomment-1382623925 Oops, I wrongly closed this. I meant to close #3003 instead. Reopening. -- 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 #3003: 5430/Add IFNULL() function for BIG_QUERY dialect
julianhyde commented on PR #3003: URL: https://github.com/apache/calcite/pull/3003#issuecomment-1382622571 Merged as e426bd26. (The PR link in that commit is incorrect; it should point to this PR, 3003.) -- 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 closed pull request #3003: 5430/Add IFNULL() function for BIG_QUERY dialect
julianhyde closed pull request #3003: 5430/Add IFNULL() function for BIG_QUERY dialect URL: https://github.com/apache/calcite/pull/3003 -- 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] olivrlee commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
olivrlee commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1070146181
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4609,15 +4610,116 @@ SqlLiteral DateTimeLiteral() :
}
|
{ s = span(); } p = SimpleStringLiteral() {
- return SqlParserUtil.parseDateLiteral(p, s.end(this));
+ return SqlLiteral.createUnknown("DATE", p, s.end(this));
+}
+|
+ { s = span(); } p = SimpleStringLiteral() {
+return SqlLiteral.createUnknown("DATETIME", p, s.end(this));
}
|
{ s = span(); } p = SimpleStringLiteral() {
-return SqlParserUtil.parseTimeLiteral(p, s.end(this));
+ return SqlLiteral.createUnknown("TIME", p, s.end(this));
}
|
+LOOKAHEAD(2)
{ s = span(); } p = SimpleStringLiteral() {
-return SqlParserUtil.parseTimestampLiteral(p, s.end(this));
+return SqlLiteral.createUnknown("TIMESTAMP", p, s.end(this));
+}
+|
+ { s = span(); } p =
SimpleStringLiteral() {
+return SqlLiteral.createUnknown("TIMESTAMP WITH LOCAL TIME ZONE", p,
s.end(this));
+}
+}
+
+/**
+ * Parses BigQuery's built-in DATE() function.
+ */
+SqlNode DateFunctionCall() :
+{
+final SqlFunctionCategory funcType = SqlFunctionCategory.TIMEDATE;
+final SqlIdentifier qualifiedName;
+final Span s;
+final SqlLiteral quantifier;
+final List args;
+}
+{
+ {
+s = span();
+qualifiedName = new SqlIdentifier(unquotedIdentifier(), getPos());
+}
+args = FunctionParameterList(ExprContext.ACCEPT_SUB_QUERY) {
+quantifier = (SqlLiteral) args.get(0);
+args.remove(0);
Review Comment:
what is this line doing?
--
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] sonarcloud[bot] commented on pull request #2942: (do not merge)
sonarcloud[bot] commented on PR #2942: URL: https://github.com/apache/calcite/pull/2942#issuecomment-1382468564 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2942) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [37.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) -- 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 closed pull request #3012: [CALCITE-5454] Update BigQuery Conformance for != and % operators
julianhyde closed pull request #3012: [CALCITE-5454] Update BigQuery Conformance for != and % operators URL: https://github.com/apache/calcite/pull/3012 -- 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 closed pull request #3013: CALCITE-5451 / Implement LPAD() and RPAD()
julianhyde closed pull request #3013: CALCITE-5451 / Implement LPAD() and RPAD() URL: https://github.com/apache/calcite/pull/3013 -- 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 closed pull request #3008: [CALCITE-5436] Implement DATE_SUB, TIME_SUB, TIMESTAMP_SUB (compatible w/ BigQuery)
julianhyde closed pull request #3008: [CALCITE-5436] Implement DATE_SUB, TIME_SUB, TIMESTAMP_SUB (compatible w/ BigQuery) URL: https://github.com/apache/calcite/pull/3008 -- 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 closed pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
julianhyde closed pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH() URL: https://github.com/apache/calcite/pull/3011 -- 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 closed pull request #3014: [CALCITE-5432] Implement BigQuery TIME_ADD() and TIME_DIFF()
julianhyde closed pull request #3014: [CALCITE-5432] Implement BigQuery TIME_ADD() and TIME_DIFF() URL: https://github.com/apache/calcite/pull/3014 -- 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 closed pull request #3009: CALCITE-5447/ DATE_TRUNC() function for BIG_QUERY
julianhyde closed pull request #3009: CALCITE-5447/ DATE_TRUNC() function for BIG_QUERY URL: https://github.com/apache/calcite/pull/3009 -- 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] olivrlee commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
olivrlee commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1070115107
##
core/src/main/java/org/apache/calcite/sql/SqlUnknownLiteral.java:
##
@@ -0,0 +1,68 @@
+/*
+ * 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.sql;
+
+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.util.NlsString;
+import org.apache.calcite.util.Util;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Literal whose type is not yet known.
+ */
+public class SqlUnknownLiteral extends SqlLiteral {
+ public final String tag;
+
+ SqlUnknownLiteral(String tag, String value, SqlParserPos pos) {
+super(requireNonNull(value, "value"), SqlTypeName.UNKNOWN, pos);
+this.tag = requireNonNull(tag, "tag");
+ }
+
+ @Override public String getValue() {
+return (String) requireNonNull(super.getValue(), "value");
+ }
+
+ @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec)
{
+final NlsString nlsString = new NlsString(getValue(), null, null);
+writer.keyword(tag);
+writer.literal(nlsString.asSql(true, true, writer.getDialect()));
+ }
+
+
+ /** Converts this unknown literal to a literal of known type. */
+ public SqlLiteral resolve(SqlTypeName typeName) {
+switch (typeName) {
+case DATE:
+ return SqlParserUtil.parseDateLiteral(getValue(), pos);
+case TIME:
+ return SqlParserUtil.parseTimeLiteral(getValue(), pos);
+case TIMESTAMP:
+ return SqlParserUtil.parseTimestampLiteral(getValue(), pos);
+case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+ return SqlParserUtil.parseTimestampWithLocalTimeZoneLiteral(getValue(),
pos);
+default:
Review Comment:
nvm discussed offline, I think that location should be `return
SqlLiteral.createUnknown("TIMESTAMP")` instead
--
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] olivrlee commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
olivrlee commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1069975023
##
core/src/main/java/org/apache/calcite/sql/SqlUnknownLiteral.java:
##
@@ -0,0 +1,68 @@
+/*
+ * 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.sql;
+
+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.util.NlsString;
+import org.apache.calcite.util.Util;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Literal whose type is not yet known.
+ */
+public class SqlUnknownLiteral extends SqlLiteral {
+ public final String tag;
+
+ SqlUnknownLiteral(String tag, String value, SqlParserPos pos) {
+super(requireNonNull(value, "value"), SqlTypeName.UNKNOWN, pos);
+this.tag = requireNonNull(tag, "tag");
+ }
+
+ @Override public String getValue() {
+return (String) requireNonNull(super.getValue(), "value");
+ }
+
+ @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec)
{
+final NlsString nlsString = new NlsString(getValue(), null, null);
+writer.keyword(tag);
+writer.literal(nlsString.asSql(true, true, writer.getDialect()));
+ }
+
+
+ /** Converts this unknown literal to a literal of known type. */
+ public SqlLiteral resolve(SqlTypeName typeName) {
+switch (typeName) {
+case DATE:
+ return SqlParserUtil.parseDateLiteral(getValue(), pos);
+case TIME:
+ return SqlParserUtil.parseTimeLiteral(getValue(), pos);
+case TIMESTAMP:
+ return SqlParserUtil.parseTimestampLiteral(getValue(), pos);
+case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+ return SqlParserUtil.parseTimestampWithLocalTimeZoneLiteral(getValue(),
pos);
+default:
Review Comment:
did you miss a case for DATETIME?
https://github.com/apache/calcite/pull/3023/files#diff-e873041549333502af52ece8a1b34301ae5a059ff4719e9bddbaef48929e7047R4617
--
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] sonarcloud[bot] commented on pull request #2942: (do not merge)
sonarcloud[bot] commented on PR #2942: URL: https://github.com/apache/calcite/pull/2942#issuecomment-1382259823 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2942) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [27.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) -- 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] rubenada commented on pull request #3026: CALCITE-5471 RelSupplier.SqlRelSupplier#apply should use .project(), not .rel
rubenada commented on PR #3026: URL: https://github.com/apache/calcite/pull/3026#issuecomment-1381655727 LGTM -- 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] sonarcloud[bot] commented on pull request #3026: CALCITE-5471 RelSupplier.SqlRelSupplier#apply should use .project(), not .rel
sonarcloud[bot] commented on PR #3026: URL: https://github.com/apache/calcite/pull/3026#issuecomment-1381653736 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3026) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3026&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3026&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3026&metric=new_duplicated_lines_density&view=list) -- 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] chunweilei commented on a diff in pull request #2932: [CALCITE-5304] RelJson should support RexSubQuery
chunweilei commented on code in PR #2932:
URL: https://github.com/apache/calcite/pull/2932#discussion_r1069024265
##
core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java:
##
@@ -122,10 +127,32 @@ public RelJson withInputTranslator(InputTranslator
inputTranslator) {
return new RelJson(jsonBuilder, inputTranslator);
}
+ /** Returns a RelJson with a given RelJsonReader. */
+ @SuppressWarnings("initialization.invalid.field.write.initialized")
+ public RelJson withRelJsonReader(@UnknownInitialization RelJsonReader
relJsonReader) {
+this.relJsonReader = relJsonReader;
+return this;
+ }
+
+ /** Returns a RelJson with a given RelJsonWriter. */
+ @SuppressWarnings("initialization.invalid.field.write.initialized")
+ public RelJson withRelJsonWriter(@UnknownInitialization RelJsonWriter
relJsonWriter) {
+this.relJsonWriter = relJsonWriter;
+return this;
+ }
+
private JsonBuilder jsonBuilder() {
return requireNonNull(jsonBuilder, "jsonBuilder");
}
+ private RelJsonWriter relJsonWriter() {
+return requireNonNull(relJsonWriter, "relJsonWriter");
+ }
+
+ private RelJsonReader relJsonReader() {
Review Comment:
It's a little weird that you add RelJsonWriter and RelJsonReader to RelJson.
Why RelJson has to use them?
--
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] chunweilei commented on a diff in pull request #2981: [CALCITE-5283] Add ARG_MIN, ARG_MAX aggregate functions
chunweilei commented on code in PR #2981:
URL: https://github.com/apache/calcite/pull/2981#discussion_r1069003834
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1849,6 +1849,17 @@ public static > T greatest(T b0,
T b1) {
return b0 == null || b1 != null && b0.compareTo(b1) < 0 ? b1 : b0;
}
+ /** Less than. */
+ public static > boolean lessThan(T b0, T b1) {
+return b1 == null || b0 != null && b0.compareTo(b1) < 0;
+ }
+
+ /** Grater than. */
+ public static > boolean greaterThan(T b0, T b1) {
+return b1 == null || b0 != null && b0.compareTo(b1) > 0;
Review Comment:
What's the difference between `greatest` and `greaterThan`?
--
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] sonarcloud[bot] commented on pull request #2942: (do not merge)
sonarcloud[bot] commented on PR #2942: URL: https://github.com/apache/calcite/pull/2942#issuecomment-1381202178 SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2942) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2942&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2942&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [29.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2942&metric=new_duplicated_lines_density&view=list) -- 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, #3025: [CALCITE-5464] Implement BigQuery DATE_ADD/DATE_DIFF
tanclary opened a new pull request, #3025: URL: https://github.com/apache/calcite/pull/3025 This is a draft PR for beginning work on BigQuery DATE_ADD() and DATE_DIFF() functions. DATE_ADD() is currently functioning as expected. DATE_DIFF() is facing a small issue where 'week' is not working as expected because BigQuery considers a 'week' beginning on Sunday (so the difference between dates on a Saturday and Sunday would return 1 despite only being a day apart). If anyone has any comments about this issue or any other changes I would highly appreciate it! More info about these functions is below. DATE_ADD(date_expression, interval) adds the interval to the date expression. Example: DATE_ADD(DATE '2008-12-25', INTERVAL 3 DAY) would return '2008-12-28'. BigQuery docs: https://cloud.google.com/bigquery/docs/reference/standard-sql/date_functions#date_add DATE_DIFF(date_expression, date_expression2, time_unit) returns the whole number of time_unit between the first and second date expressions, with the result being negative if the first date is earlier than the second. Example: DATE_DIFF(DATE '2008-12-25', DATE '2008-12-28', DAY) would return -3. BigQuery docs: https://cloud.google.com/bigquery/docs/reference/standard-sql/date_functions#date_diff -- 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, #3024: [CALCITE-5469] Implement BigQuery DATETIME_ADD/DATETIME_DIFF
tanclary opened a new pull request, #3024: URL: https://github.com/apache/calcite/pull/3024 This is a beginning draft for the work to implement BigQuery DATETIME_ADD/DATETIME_DIFF. Currently, DATETIME_ADD is functioning as expected and is passing all operator and quidem tests. DATETIME_DIFF is not currently passing quidem tests for a couple of different reasons: the 'week' time unit is only counting whole number of weeks rather than demarcating each Sunday, as well as 'isoyear' for a similar reason. Any comments on these issues or the rest of the changes would be highly appreciated! More information about the functions may be found below. DATETIME_ADD(datetime, interval) can accept a timestamp (or a datetime, which is an alias for timestamp) for its first argument and an interval for its second. The output is the datetime that occurs interval after the provided datetime. DATETIME_DIFF(datetime, datetime2, timeUnit) returns the whole number of timeUnit between datetime and datetime2, with the result being negative if datetime occurs before datetime2 Examples: DATETIME_ADD(TIMESTAMP '2008-12-25 15:30:00', INTERVAL 5 MINUTE) would return: '2008-12-25 15:35:00'. DATETIME_DIFF(TIMESTAMP '2008-12-25 15:30:00', TIMESTAMP '2008-12-26 15:30:00', DAY) would return: -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. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #2991: [CALCITE-5407] Fix ARRAY conversion in MongoDB adapter
sonarcloud[bot] commented on PR #2991: URL: https://github.com/apache/calcite/pull/2991#issuecomment-1381192981 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=2991) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2991&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2991&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=2991&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=2991&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2991&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2991&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2991&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=2991&metric=new_duplicated_lines_density&view=list) -- 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
