You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/10/29 21:40:46 UTC
[GitHub] [spark] stwhit opened a new pull request #30189: Improve DataFrame.show for arrays, maps, and structs
stwhit opened a new pull request #30189:
URL: https://github.com/apache/spark/pull/30189
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
The changes in [SPARK-32501 Inconsistent NULL conversions to strings](https://issues.apache.org/jira/browse/SPARK-32501) introduced some behavior that I'd like to clean up a bit.
Here's sample code to illustrate the behavior I'd like to clean up:
```
val rows = Seq[String](null)
.toDF("value")
.withColumn("struct1", struct('value as "value1"))
.withColumn("struct2", struct('value as "value1", 'value as "value2"))
.withColumn("array1", array('value))
.withColumn("array2", array('value, 'value))
.withColumn("map1", map(lit("value1"), 'value))
.withColumn("map2", map(lit("value1"), 'value, lit("value2"), 'value))
// Show the DataFrame using the "first" codepath.
rows.show(truncate=false)
+-----+-------+-------------+------+--------+----------------+--------------------------------+
|value|struct1|struct2 |array1|array2 |map1 |map2 |
+-----+-------+-------------+------+--------+----------------+--------------------------------+
|null |{ null}|{ null, null}|[] |[, null]|{value1 -> null}|{value1 -> null, value2 -> null}|
+-----+-------+-------------+------+--------+----------------+--------------------------------+
// Write the DataFrame to disk, then read it back and show it to trigger the "codegen" code path:
rows.write.parquet("rows")
spark.read.parquet("rows").show(truncate=false)
+-----+-------+-------------+-------+-------------+----------------+--------------------------------+
|value|struct1|struct2 |array1 |array2 |map1 |map2 |
+-----+-------+-------------+-------+-------------+----------------+--------------------------------+
|null |{ null}|{ null, null}|[ null]|[ null, null]|{value1 -> null}|{value1 -> null, value2 -> null}|
+-----+-------+-------------+-------+-------------+----------------+--------------------------------+
```
Notice:
1. If the first element of a struct is null, it is printed with a leading space (e.g. "\{ null\}"). I think it's preferable to print it without the leading space (e.g. "\{null\}"). This is consistent with how non-null values are printed inside a struct.
2. If the first element of an array is null, it is not printed at all in the first code path, and the "codegen" code path prints it with a leading space. I think both code paths should be consistent and print it without a leading space (e.g. "[null]").
The desired result of this PR is to product the following output via both code paths:
```
+-----+-------+------------+------+------------+----------------+--------------------------------+
|value|struct1|struct2 |array1|array2 |map1 |map2 |
+-----+-------+------------+------+------------+----------------+--------------------------------+
|null |{null} |{null, null}|[null]|[null, null]|{value1 -> null}|{value1 -> null, value2 -> null}|
+-----+-------+------------+------+------------+----------------+--------------------------------+
```
This contribution is my original work and I license the work to the project under the project’s open source license.
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
To correct errors and inconsistencies in how DataFrame.show() displays nulls inside structs, arrays, and maps.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
Yes. This PR changes what is printed out by DataFrame.show().
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
I modified the existing test cases in CastSuite.scala to cover the cases addressed by this PR.
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723127387
**[Test build #130729 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130729/testReport)** for PR 30189 at commit [`601af5e`](https://github.com/apache/spark/commit/601af5e9fb034703f002aa39ce3e695118ab9ffb).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723262613
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722678961
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518884821
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
Oh, so map doesn't have the same a leading space? If so then it is okay. As you actually change map's code, it would be nice to add a test like array/struct to prevent anything wrong.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723160604
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723301659
Thanks for working on this! Merging to master.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518935367
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
Okay it sounds good to me if we already have test for it. Thanks for clarifying.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays, maps, and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719041916
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] stwhit commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
stwhit commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518956204
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
Sounds good. Thanks for the feedback.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723160604
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719326378
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719150057
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35033/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723127387
**[Test build #130729 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130729/testReport)** for PR 30189 at commit [`601af5e`](https://github.com/apache/spark/commit/601af5e9fb034703f002aa39ce3e695118ab9ffb).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719150069
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722818372
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518418496
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
Is this for map type? I saw you only added tests for array and struct.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518522799
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -898,8 +900,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
"""
}
- private def outNullElem(buffer: ExprValue): Block = {
- if (legacyCastToStr) code"" else code"""$buffer.append(" null");"""
+ private def appendIfNotLegacyCastToStr(buffer: ExprValue, s: String): Block = {
+ if (!legacyCastToStr) code"""$buffer.append("$s");"""
+ else EmptyBlock
Review comment:
nit: ` if (!legacyCastToStr) code"""$buffer.append("$s");""" else EmptyBlock`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722744086
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719041362
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719323870
**[Test build #130428 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130428/testReport)** for PR 30189 at commit [`cc7ccd2`](https://github.com/apache/spark/commit/cc7ccd2c3ba5b4069448d72c572d8ab6e91fbd74).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya closed pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya closed pull request #30189:
URL: https://github.com/apache/spark/pull/30189
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722939198
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719128904
**[Test build #130428 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130428/testReport)** for PR 30189 at commit [`cc7ccd2`](https://github.com/apache/spark/commit/cc7ccd2c3ba5b4069448d72c572d8ab6e91fbd74).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722796199
**[Test build #130685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130685/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719150069
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719326406
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130428/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719128031
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722744086
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722938529
**[Test build #130685 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130685/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays, maps, and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719041362
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722647065
**[Test build #130667 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130667/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] stwhit commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
stwhit commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518902804
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
map's behavior is already correct. For example, when printing a map, it looks like this:
{A -> B, C -> D}
So, the values are already printed with a leading space, as they should be. Therefore, I didn't need to change map's behavior.
Regarding a unit test, one already exists. In CastSuite.scala, beginning on line 731 (in my branch), there's a test named "SPARK-22973 Cast map to string" which already exists. So, yes, I changed map's code, but no, I didn't change map's behavior, and the correct map behavior is already being unit tested. My code changes didn't change the behavior, and the existing unit test passes successfully after my changes.
I could add another unit test, but it will literally just be the same as the existing unit test (because I didn't change any behavior). Do you want me to go ahead and add another unit test?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723261641
**[Test build #130729 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130729/testReport)** for PR 30189 at commit [`601af5e`](https://github.com/apache/spark/commit/601af5e9fb034703f002aa39ce3e695118ab9ffb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722815332
LGTM except for the @viirya comments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] stwhit commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
stwhit commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518806342
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -717,6 +717,17 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
checkEvaluation(ret8, "[[[a], [b, c]], [[d]]]")
}
+ test("SPARK-33291 Cast array with null elements to string") {
Review comment:
I've modified the code per your suggestion.
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -773,6 +784,19 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
}
}
+ test("SPARK-33291 Cast struct with null elements to string") {
Review comment:
I've modified the code per your suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722644295
Retest this please.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722744093
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130667/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719128904
**[Test build #130428 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130428/testReport)** for PR 30189 at commit [`cc7ccd2`](https://github.com/apache/spark/commit/cc7ccd2c3ba5b4069448d72c572d8ab6e91fbd74).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722818357
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35296/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] stwhit commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
stwhit commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518806137
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -898,8 +900,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
"""
}
- private def outNullElem(buffer: ExprValue): Block = {
- if (legacyCastToStr) code"" else code"""$buffer.append(" null");"""
+ private def appendIfNotLegacyCastToStr(buffer: ExprValue, s: String): Block = {
+ if (!legacyCastToStr) code"""$buffer.append("$s");"""
+ else EmptyBlock
Review comment:
I've modified the code per your suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722678948
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35278/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722818372
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722644100
cc @viirya
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719041916
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723262613
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722796199
**[Test build #130685 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130685/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722939201
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130685/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722669412
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35278/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722678961
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] stwhit commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
stwhit commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518813843
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -982,7 +985,7 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
| $buffer.append($keyToStringFunc($getMapFirstKey));
| $buffer.append(" ->");
| if ($map.valueArray().isNullAt(0)) {
- | ${outNullElem(buffer)}
+ | ${appendIfNotLegacyCastToStr(buffer, " null")}
Review comment:
I replaced outNullElem (which always appended " null") with appendIfNotLegacyCastToStr (which allows you to pass in the string to be appended).
Because the map, struct, and array code all called the outNullElem function, I changed them all to call appendIfNotLegacyCastToStr, even though, in the case of map, I'm not actually changing its behavior. IOW, the map code was already correct, so I'm not making any changes to its behavior, I only changed that function call because that function is shared with struct and array.
This is also why I didn't add any unit tests for map. I didn't change its behavior.
How would you like me to resolve this? I can put the outNullElem function back, and revert the map code to call it, which will revert all of my changes to the map code. However, doing that will create what could be considered redundant code (both outNullElem and appendIfNotLegacyCastToStr do almost the same thing, and appendIfNotLegacyCastToStr supercedes outNullElem, rendering outNullElem redundant).
I'm happy to make whatever change you recommend. Please advise.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723238876
LGTM pending test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723149292
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35338/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722939198
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518522909
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -717,6 +717,17 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
checkEvaluation(ret8, "[[[a], [b, c]], [[d]]]")
}
+ test("SPARK-33291 Cast array with null elements to string") {
Review comment:
nit: `SPARK-33291: `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30189:
URL: https://github.com/apache/spark/pull/30189#discussion_r518522939
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
##########
@@ -773,6 +784,19 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
}
}
+ test("SPARK-33291 Cast struct with null elements to string") {
Review comment:
ditto:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-723160584
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35338/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722818377
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35296/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722743474
**[Test build #130667 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130667/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722810231
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35296/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722647065
**[Test build #130667 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130667/testReport)** for PR 30189 at commit [`b4a14aa`](https://github.com/apache/spark/commit/b4a14aacb4efbd2240025d876fffb18b412b6045).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-722795546
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719326378
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30189: [SPARK-33291][SQL] Improve DataFrame.show for nulls in arrays and structs
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30189:
URL: https://github.com/apache/spark/pull/30189#issuecomment-719141425
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35033/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org