You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "olivrlee (via GitHub)" <gi...@apache.org> on 2023/11/09 19:39:10 UTC
[PR] [CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) serialization [calcite]
olivrlee opened a new pull request, #3513:
URL: https://github.com/apache/calcite/pull/3513
https://issues.apache.org/jira/browse/CALCITE-5607
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) serialization [calcite]
Posted by "tjbanghart (via GitHub)" <gi...@apache.org>.
tjbanghart commented on code in PR #3513:
URL: https://github.com/apache/calcite/pull/3513#discussion_r1391975445
##########
core/src/test/java/org/apache/calcite/plan/RelWriterTest.java:
##########
@@ -1052,6 +1053,41 @@ void testAggregateWithAlias(SqlExplainFormat format) {
assertThat(result, isLinux(expected));
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5607">[CALCITE-5607]</a>
+ *
+ * <p>Before the fix, RelJson.toRex would throw an ArrayIndexOutOfBounds error
+ * when deserialization.
+ * */
+ @Test void testDeserializeMinusDateOperator() {
Review Comment:
The commit message and test case don't seem to align. This checks that `MINUS` can be deserialized but I don't see anything to suggest we've changed how the return `type` is being serialized.
##########
core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java:
##########
@@ -622,6 +622,7 @@ public Object toJson(RexNode node) {
}
map.put("operands", list);
switch (node.getKind()) {
+ case MINUS:
Review Comment:
We should change the Jira issue to reflect that we are only changing the behavior of `MINUS`.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Error when deserializing TIMESTAMP_DIFF in RelJson#toJson [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3513:
URL: https://github.com/apache/calcite/pull/3513#issuecomment-1811565260
Please retry analysis of this Pull-Request directly on [SonarCloud](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513).
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) serialization [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3513:
URL: https://github.com/apache/calcite/pull/3513#issuecomment-1804701411
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3513)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Error when deserializing TIMESTAMP_DIFF in RelJson#toJson [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3513:
URL: https://github.com/apache/calcite/pull/3513#issuecomment-1811559374
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3513)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3513&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3513&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3513&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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Error when deserializing TIMESTAMP_DIFF in RelJson#toJson [calcite]
Posted by "olivrlee (via GitHub)" <gi...@apache.org>.
olivrlee merged PR #3513:
URL: https://github.com/apache/calcite/pull/3513
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) serialization [calcite]
Posted by "olivrlee (via GitHub)" <gi...@apache.org>.
olivrlee commented on code in PR #3513:
URL: https://github.com/apache/calcite/pull/3513#discussion_r1393361220
##########
core/src/test/java/org/apache/calcite/plan/RelWriterTest.java:
##########
@@ -1052,6 +1053,41 @@ void testAggregateWithAlias(SqlExplainFormat format) {
assertThat(result, isLinux(expected));
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5607">[CALCITE-5607]</a>
+ *
+ * <p>Before the fix, RelJson.toRex would throw an ArrayIndexOutOfBounds error
+ * when deserialization.
+ * */
+ @Test void testDeserializeMinusDateOperator() {
Review Comment:
Yeah let me update the JIRA summary and the ticket title for this.
We're changing it to only affect MINUS
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org