[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=3038) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3038=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3038=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3038=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3038=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3038=new_coverage=list) [75.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3038=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3038=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3038=new_duplicated_lines_density=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
[calcite] branch main updated: [CALCITE-5466] The constant condition can't be inferred from correlate node
This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new fb340ece8c [CALCITE-5466] The constant condition can't be inferred
from correlate node
fb340ece8c is described below
commit fb340ece8c5e60ecfd6371950f8dcb665c85a712
Author: Aitozi
AuthorDate: Thu Jan 19 15:06:42 2023 +0800
[CALCITE-5466] The constant condition can't be inferred from correlate node
Close #3035
---
.../calcite/rel/metadata/RelMdPredicates.java | 8 +
.../org/apache/calcite/test/RelOptRulesTest.java | 26 +++
.../GeneratedMetadata_PredicatesHandler.java | 2 ++
.../org/apache/calcite/test/RelOptRulesTest.xml| 37 ++
4 files changed, 73 insertions(+)
diff --git
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
index da71912046..969e8a5e5d 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java
@@ -25,6 +25,7 @@ import org.apache.calcite.plan.Strong;
import org.apache.calcite.plan.volcano.RelSubset;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Correlate;
import org.apache.calcite.rel.core.Exchange;
import org.apache.calcite.rel.core.Filter;
import org.apache.calcite.rel.core.Intersect;
@@ -275,6 +276,13 @@ public class RelMdPredicates
return rexBuilder.makeLiteral(true);
}
+ /**
+ * Infers predicates for a correlate node.
+ */
+ public RelOptPredicateList getPredicates(Correlate correlate,
RelMetadataQuery mq) {
+return mq.getPulledUpPredicates(correlate.getLeft());
+ }
+
/**
* Add the Filter condition to the pulledPredicates list from the input.
*/
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index c588197333..7f5dca60b4 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -3002,6 +3002,32 @@ class RelOptRulesTest extends RelOptTestBase {
.withProgram(program).check();
}
+ @Test void testReducingConstantsInferredFromCorrelate() {
+HepProgram program = new HepProgramBuilder()
+.addRuleInstance(CoreRules.PROJECT_REDUCE_EXPRESSIONS)
+.build();
+
+final String sql = "SELECT ename,\n"
++ " empno,\n"
++ " T.empno_r,\n"
++ " CASE\n"
++ "WHEN __source__type__ = 'bounded'\n"
++ " THEN 1\n"
++ "ELSE 2\n"
++ "END AS type\n"
++ "FROM (\n"
++ " SELECT ename,\n"
++ "empno,\n"
++ "'bounded' AS __source__type__\n"
++ " FROM emp\n"
++ " ) a,\n"
++ " lateral TABLE (ramp(empno)) AS T(empno_r)";
+sql(sql)
+.withRelBuilderConfig(c -> c.withSimplifyValues(false))
+.withProgram(program).check();
+ }
+
+
@Test void testRemoveSemiJoin() {
final String sql = "select e.ename from emp e, dept d\n"
+ "where e.deptno = d.deptno";
diff --git
a/core/src/test/resources/org/apache/calcite/rel/metadata/janino/GeneratedMetadata_PredicatesHandler.java
b/core/src/test/resources/org/apache/calcite/rel/metadata/janino/GeneratedMetadata_PredicatesHandler.java
index 14149082a5..9ecec20edf 100644
---
a/core/src/test/resources/org/apache/calcite/rel/metadata/janino/GeneratedMetadata_PredicatesHandler.java
+++
b/core/src/test/resources/org/apache/calcite/rel/metadata/janino/GeneratedMetadata_PredicatesHandler.java
@@ -64,6 +64,8 @@ public final class GeneratedMetadata_PredicatesHandler
return
provider0.getPredicates((org.apache.calcite.plan.volcano.RelSubset) r, mq);
} else if (r instanceof org.apache.calcite.rel.core.Aggregate) {
return provider0.getPredicates((org.apache.calcite.rel.core.Aggregate)
r, mq);
+} else if (r instanceof org.apache.calcite.rel.core.Correlate) {
+ return provider0.getPredicates((org.apache.calcite.rel.core.Correlate)
r, mq);
} else if (r instanceof org.apache.calcite.rel.core.Exchange) {
return provider0.getPredicates((org.apache.calcite.rel.core.Exchange) r,
mq);
} else if (r instanceof org.apache.calcite.rel.core.Filter) {
diff --git
a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 321eebfb36..21dcf77334 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -11751,6 +11751,43 @@ LogicalProject(EXPR$0=[$0],
[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=2981) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=2981=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=2981=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=2981=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=2981=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=2981=new_coverage=list) [71.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=2981=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=2981=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=2981=new_duplicated_lines_density=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=3036) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [45 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_coverage=list) [98.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_duplicated_lines_density=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_duplicated_lines_density=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=3036) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3036=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [46 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3036=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_coverage=list) [98.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_duplicated_lines_density=list) [1.5% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3036=new_duplicated_lines_density=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
[calcite-site] branch main updated: Website deployed from calcite-avatica@aace9126b954c4adc36e9b1eb0d4ab8b8951c576
This is an automated email from the ASF dual-hosted git repository. asf-ci-deploy pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite-site.git The following commit(s) were added to refs/heads/main by this push: new caca68a9 Website deployed from calcite-avatica@aace9126b954c4adc36e9b1eb0d4ab8b8951c576 caca68a9 is described below commit caca68a9f74677abe476bf8292fafb54a616c434 Author: julianhyde AuthorDate: Thu Jan 19 21:46:36 2023 + Website deployed from calcite-avatica@aace9126b954c4adc36e9b1eb0d4ab8b8951c576 --- avatica/docs/client_reference.html | 2 +- avatica/docs/compatibility.html| 2 +- avatica/docs/custom_client_artifacts.html | 2 +- avatica/docs/docker.html | 2 +- avatica/docs/go_client_reference.html | 2 +- avatica/docs/go_history.html | 2 +- avatica/docs/go_howto.html | 2 +- avatica/docs/history.html | 6 +- avatica/docs/howto.html| 6 +- avatica/docs/index.html| 2 +- avatica/docs/json_reference.html | 2 +- avatica/docs/protobuf_reference.html | 2 +- avatica/docs/protocol_testing.html | 2 +- avatica/docs/roadmap.html | 2 +- avatica/docs/security.html | 2 +- avatica/downloads/avatica-go.html | 2 +- avatica/downloads/avatica.html | 12 +++- .../news/2016/03/04/separate-project/index.html| 10 +-- avatica/news/2016/03/18/release-1.7.1/index.html | 10 +-- avatica/news/2016/06/04/release-1.8.0/index.html | 10 +-- avatica/news/2016/11/01/release-1.9.0/index.html | 10 +-- .../2017/03/31/new-avatica-repository/index.html | 10 +-- avatica/news/2017/05/30/release-1.10.0/index.html | 10 +-- avatica/news/2018/03/09/release-1.11.0/index.html | 10 +-- .../2018/04/27/release-avatica-go-3.0.0/index.html | 10 +-- avatica/news/2018/06/24/release-1.12.0/index.html | 10 +-- .../2018/09/10/release-avatica-go-3.1.0/index.html | 10 +-- .../2018/09/18/release-avatica-go-3.2.0/index.html | 10 +-- avatica/news/2018/12/04/release-1.13.0/index.html | 10 +-- avatica/news/2019/04/29/release-1.14.0/index.html | 10 +-- avatica/news/2019/05/13/release-1.15.0/index.html | 10 +-- .../2019/05/16/release-avatica-go-4.0.0/index.html | 10 +-- avatica/news/2019/12/19/release-1.16.0/index.html | 10 +-- avatica/news/2020/06/22/release-1.17.0/index.html | 10 +-- .../2020/07/16/release-avatica-go-5.0.0/index.html | 10 +-- avatica/news/2021/05/18/release-1.18.0/index.html | 10 +-- avatica/news/2021/10/11/release-1.19.0/index.html | 10 +-- avatica/news/2021/12/13/release-1.20.0/index.html | 10 +-- .../2022/03/27/release-avatica-go-5.1.0/index.html | 10 +-- avatica/news/2022/05/08/release-1.21.0/index.html | 10 +-- avatica/news/2022/07/28/release-1.22.0/index.html | 10 +-- .../2022/10/13/release-avatica-go-5.2.0/index.html | 10 +-- .../01/19/release-1.23.0}/index.html | 49 +- avatica/news/avatica-go-releases/index.html| 12 ++-- avatica/news/avatica-releases/index.html | 76 -- avatica/news/index.html| 74 +++-- 46 files changed, 361 insertions(+), 152 deletions(-) diff --git a/avatica/docs/client_reference.html b/avatica/docs/client_reference.html index 9f2efb74..2cccdc6b 100644 --- a/avatica/docs/client_reference.html +++ b/avatica/docs/client_reference.html @@ -1002,7 +1002,7 @@ in 1.21.0. Setting this property to HOWTO + Avatica HOWTO diff --git a/avatica/docs/compatibility.html b/avatica/docs/compatibility.html index b71398b5..c3f41d0f 100644 --- a/avatica/docs/compatibility.html +++ b/avatica/docs/compatibility.html @@ -903,7 +903,7 @@ running the TCK, reference the provided https://github.com/apache/calci - HOWTO + Avatica HOWTO diff --git a/avatica/docs/custom_client_artifacts.html b/avatica/docs/custom_client_artifacts.html index 111b6c75..d3a57133 100644 --- a/avatica/docs/custom_client_artifacts.html +++ b/avatica/docs/custom_client_artifacts.html @@ -936,7 +936,7 @@ a brief pom.xml which - HOWTO + Avatica HOWTO diff --git a/avatica/docs/docker.html b/avatica/docs/docker.html index 345dc8dd..394f9625 100644 --- a/avatica/docs/docker.html +++ b/avatica/docs/docker.html @@ -958,7 +958,7 @@ launch a custom Avatica server against our database with this JDBC driver. - HOWTO + Avatica HOWTO diff --git a/avatica/docs/go_client_reference.html b/avatica/docs/go_client_reference.html index cc94c6b2..59942ed7 100644 --- a/avatica/docs/go_client_reference.html +++ b/avatica/docs/go_client_reference.html @@ -1070,7 +1070,7 @@ Apache Phoenix error code: - HOWTO + Avatica HOWTO
[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
[calcite-avatica] branch main updated (02c5fa153 -> aace9126b)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git from 02c5fa153 [CALCITE-5443] After Statement.getMoreResults() has returned false, Statement.getUpdateCount() should return -1 add fab03540d [CALCITE-5480] Release Avatica 1.23.0 new aace9126b Prepare for next development iteration The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: NOTICE | 2 +- README | 2 +- gradle.properties| 2 +- site/_docs/docker_images.md | 28 ++--- site/_docs/history.md| 183 +++ site/_docs/howto.md | 6 +- site/_posts/2023-01-19-release-1.23.0.md | 51 + 7 files changed, 230 insertions(+), 44 deletions(-) create mode 100644 site/_posts/2023-01-19-release-1.23.0.md
[calcite-avatica] 01/01: Prepare for next development iteration
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git
commit aace9126b954c4adc36e9b1eb0d4ab8b8951c576
Author: Julian Hyde
AuthorDate: Thu Jan 19 13:45:22 2023 -0800
Prepare for next development iteration
---
gradle.properties| 2 +-
site/_docs/history.md| 4 +--
site/_docs/howto.md | 2 +-
site/_posts/2023-01-19-release-1.23.0.md | 51
4 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/gradle.properties b/gradle.properties
index e1b6a5b38..749667d8d 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -23,7 +23,7 @@ kotlin.parallel.tasks.in.project=true
# This is version for Calcite Avatica itself
# Note: it should not include "-SNAPSHOT" as it is automatically added by
build.gradle.kts
# Release version can be generated by using -Prelease or -Prc= arguments
-calcite.avatica.version=1.23.0
+calcite.avatica.version=1.24.0
# The options below configures the use of local clone (e.g. testing
development versions)
# You can pass un-comment it, or pass option -PlocalReleasePlugins, or
-PlocalReleasePlugins=
diff --git a/site/_docs/history.md b/site/_docs/history.md
index b0b8d0984..69693aa35 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -59,6 +59,7 @@ other software versions as specified in `gradle.properties`.
Contributors to this release:
Aleksey Stavrov,
+Dan Zou,
Francis Chuang,
Greg Hart,
Hongbin Ma,
@@ -70,8 +71,7 @@ Oliver Lee,
Richard Antal,
Sergey Nuyanzin,
Stamatis Zampetakis,
-Zhe Hu,
-zoudan
+Zhe Hu
Features and bug fixes
diff --git a/site/_docs/howto.md b/site/_docs/howto.md
index 4af69b404..e36e20c39 100644
--- a/site/_docs/howto.md
+++ b/site/_docs/howto.md
@@ -1,6 +1,6 @@
---
layout: docs
-title: HOWTO
+title: Avatica HOWTO
permalink: /docs/howto.html
---
diff --git a/site/_posts/2023-01-19-release-1.23.0.md
b/site/_posts/2023-01-19-release-1.23.0.md
new file mode 100644
index 0..5de251951
--- /dev/null
+++ b/site/_posts/2023-01-19-release-1.23.0.md
@@ -0,0 +1,51 @@
+---
+layout: news_item
+date: "2023-01-19 08:30:00 +"
+author: jhyde
+version: 1.23.0
+categories: [release]
+tag: v1-23-0
+sha: fab0354
+component: avatica
+---
+
+
+Apache Calcite Avatica 1.23.0
+fixes bugs in
+https://issues.apache.org/jira/browse/CALCITE-5443;>`Statement.getUpdateCount()`,
+https://issues.apache.org/jira/browse/CALCITE-3557;>`ResultSet.getObject`;
+and supports
+https://issues.apache.org/jira/browse/CALCITE-5358;>`HTTP_BAD_REQUEST`
+and configuring
+https://issues.apache.org/jira/browse/CALCITE-2322;>fetch size
+and
+https://issues.apache.org/jira/browse/CALCITE-5327;>SSL key-store
type.
+Also,
+https://issues.apache.org/jira/browse/CALCITE-5338;>there
+https://issues.apache.org/jira/browse/CALCITE-3078;>are
+https://issues.apache.org/jira/browse/CALCITE-5369;>various
+https://issues.apache.org/jira/browse/CALCITE-2989;>improvements
+https://issues.apache.org/jira/browse/CALCITE-1639;>to
+`DateTimeUtils` and
+https://issues.apache.org/jira/browse/CALCITE-5415;>`ByteString`.
+
+See the list of
+[bug fixes and new features]({{ site.baseurl }}/docs/history.html#v1-23-0)
+for more information.
[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
[calcite-avatica] annotated tag rel/avatica-1.23.0 updated (fab03540d -> 9b9f8f9cb)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to annotated tag rel/avatica-1.23.0 in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git *** WARNING: tag rel/avatica-1.23.0 was modified! *** from fab03540d (commit) to 9b9f8f9cb (tag) tagging fab03540de866faa783194ed58beb4319f3066c7 (commit) replaces rel/avatica-1.22.0 by Julian Hyde on Thu Jan 19 10:43:00 2023 -0800 - Log - --- No new revisions were added by this update. Summary of changes:
svn commit: r59447 - /dev/calcite/apache-calcite-avatica-1.23.0-rc0/ /release/calcite/apache-calcite-avatica-1.23.0/
Author: jhyde Date: Thu Jan 19 18:42:31 2023 New Revision: 59447 Log: Promoting Apache Calcite Avatica avatica-1.23.0-rc0 -> rel/avatica-1.23.0 to release area Added: release/calcite/apache-calcite-avatica-1.23.0/ release/calcite/apache-calcite-avatica-1.23.0/apache-calcite-avatica-1.23.0-src.tar.gz - copied unchanged from r59365, dev/calcite/apache-calcite-avatica-1.23.0-rc0/apache-calcite-avatica-1.23.0-src.tar.gz release/calcite/apache-calcite-avatica-1.23.0/apache-calcite-avatica-1.23.0-src.tar.gz.asc - copied unchanged from r59365, dev/calcite/apache-calcite-avatica-1.23.0-rc0/apache-calcite-avatica-1.23.0-src.tar.gz.asc release/calcite/apache-calcite-avatica-1.23.0/apache-calcite-avatica-1.23.0-src.tar.gz.sha512 - copied unchanged from r59365, dev/calcite/apache-calcite-avatica-1.23.0-rc0/apache-calcite-avatica-1.23.0-src.tar.gz.sha512 Removed: dev/calcite/apache-calcite-avatica-1.23.0-rc0/
[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=3035) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_coverage=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_duplicated_lines_density=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=3035) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3035=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3035=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_coverage=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3035=new_duplicated_lines_density=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
