You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "MasseGuillaume (via GitHub)" <gi...@apache.org> on 2023/01/31 22:59:16 UTC
[GitHub] [calcite] MasseGuillaume opened a new pull request, #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
MasseGuillaume opened a new pull request, #3056:
URL: https://github.com/apache/calcite/pull/3056
Spark's arrays are built calling a builtin function array: https://spark.apache.org/docs/latest/api/sql/#array
--
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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1412523770
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3056)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&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
[GitHub] [calcite] NobiGo commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1093949489
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
The java doc should stay the same as the Jira summary, Others are good to 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1094175517
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
change "Array literals are unparsed incorrectly for the spark dialect" to "Array literals are unparsed incorrectly for the spark dialect" and rebase the commit info
--
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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by sonarcloud.
sonarcloud[bot] commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1411208813
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3056)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&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
[GitHub] [calcite] NobiGo commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1093114069
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -5774,6 +5774,12 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
.ok("(ARRAY[(ROW(1, 'a')), (ROW(2, 'b'))])");
}
+ @Test void testArrayValueConstructorSpark() {
Review Comment:
+1 for removing the test to RelToSqlConverterTest
--
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
[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1094810171
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
```
change
"Array literals are unparsed incorrectly for the spark dialect" to
"Array literals are unparsed incorrectly for the spark dialect" and rebase the commit info
```
Did I miss something? it's exactly the same.
Rebasing commits 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1094010384
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
Do you mean the Description field?
```
They should be in the form: array(1, 2, 3)
However, they are unparsed as ARRAY[1, 2, 3]
https://spark.apache.org/docs/latest/api/sql/#array
```
--
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
[GitHub] [calcite] MasseGuillaume commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1411190050
While implementing, I noticed:
ArrayQueryConstructor are not valid in Spark:
`spark-shell`
```scala
spark.sql("select array(select 1)")
org.apache.spark.sql.catalyst.parser.ParseException:
extraneous input '1' expecting ')'(line 1, pos 20)
== SQL ==
select array(select 1)
--------------------^^^
at org.apache.spark.sql.catalyst.parser.ParseException.withCommand(ParseDriver.scala:266)
at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parse(ParseDriver.scala:127)
at org.apache.spark.sql.execution.SparkSqlParser.parse(SparkSqlParser.scala:51)
at org.apache.spark.sql.catalyst.parser.AbstractSqlParser.parsePlan(ParseDriver.scala:77)
at org.apache.spark.sql.SparkSession.$anonfun$sql$2(SparkSession.scala:616)
at org.apache.spark.sql.catalyst.QueryPlanningTracker.measurePhase(QueryPlanningTracker.scala:111)
at org.apache.spark.sql.SparkSession.$anonfun$sql$1(SparkSession.scala:616)
at org.apache.spark.sql.SparkSession.withActive(SparkSession.scala:775)
at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:613)
... 47 elided
```
Array literals are not uniform accross dialects,
for example Google's BigQuery uses this syntax:
```sql
SELECT [1, 2, 3]
```
https://cloud.google.com/bigquery/docs/reference/standard-sql/arrays#constructing_arrays
--
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
[GitHub] [calcite] zoudan commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1449216804
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao closed pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao closed pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
URL: https://github.com/apache/calcite/pull/3056
--
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
[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1094812010
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
ah you mean the commit title:
```
[CALCITE-5504] Unparse array value constructor for Apache Spark
```
vs jira
```
[CALCITE-5504] Array literals are unparsed incorrectly for the spark dialect
```
--
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
[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1094815119
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -3579,6 +3579,16 @@ private SqlDialect nonOrdinalDialect() {
sql(retainLimitQuery).ok(retainLimitResult);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5504">[CALCITE-5504]
+ * Array literals are unparsed incorrectly for the spark dialect</a>.*/
Review Comment:
here you go: https://github.com/apache/calcite/pull/3056/commits/8ba4534838688636d4c537e3867b99a04e297a88
--
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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1414193042
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3056)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3056&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3056&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3056&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
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1092709375
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -5774,6 +5774,12 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
.ok("(ARRAY[(ROW(1, 'a')), (ROW(2, 'b'))])");
}
+ @Test void testArrayValueConstructorSpark() {
Review Comment:
In my opinion, `RelToSqlConverterTest` maybe better than `SqlPraserTest`.
(Just a tip for you)
--
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
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3056:
URL: https://github.com/apache/calcite/pull/3056#discussion_r1092709375
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -5774,6 +5774,12 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
.ok("(ARRAY[(ROW(1, 'a')), (ROW(2, 'b'))])");
}
+ @Test void testArrayValueConstructorSpark() {
Review Comment:
In my opinion, RelToSqlConverterTest maybe better than SqlPraserTest.
(Just a tip for you)
--
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
[GitHub] [calcite] MasseGuillaume commented on pull request #3056: [CALCITE-5504] Unparse array value constructor for Apache Spark
Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.
MasseGuillaume commented on PR #3056:
URL: https://github.com/apache/calcite/pull/3056#issuecomment-1434149171
I think this PR is good to go. Is there anything remaining?
--
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