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/12/28 03:43:09 UTC

[GitHub] [spark] AngersZhuuuu opened a new pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

AngersZhuuuu opened a new pull request #30942:
URL: https://github.com/apache/spark/pull/30942


   ### What changes were proposed in this pull request?
   In hive script transform, when we use specified serde, the `filed.delim` is '\t'
   ![image](https://user-images.githubusercontent.com/46485123/103187960-7dd77800-4901-11eb-8241-f4636e66fbc8.png)
   And change to other serde and explain query plan, `filed.delim` is same.
   
   In spark current code, the result is as below:
   ![image](https://user-images.githubusercontent.com/46485123/103187999-95aefc00-4901-11eb-9850-5c385000b78c.png)
   
   We should keep same as hive.
   
   Notic:
   the result's NULL value is   different is another issue  https://issues.apache.org/jira/browse/SPARK-32684
   
   ### Why are the changes needed?
   Keep same with hive serde
   
   ### Does this PR introduce _any_ user-facing change?
   In script transform, is not specified,  `field.delim` keep same with hive as `\t`
   
   ### How was this patch tested?
   UT added
   


----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751578426


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38012/
   


----------------------------------------------------------------
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] cloud-fan commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751629073


   thanks, 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] cloud-fan commented on a change in pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30942:
URL: https://github.com/apache/spark/pull/30942#discussion_r549251997



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -505,7 +505,12 @@ class SparkSqlAstBuilder extends AstBuilder {
           } else {
             None
           }
-          (Seq.empty, Option(name), props.toSeq, recordHandler)
+          val finalProps = if (!props.contains("field.delim")) {

Review comment:
       is this case sensitive?




----------------------------------------------------------------
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] cloud-fan commented on a change in pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30942:
URL: https://github.com/apache/spark/pull/30942#discussion_r549220932



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -505,7 +505,12 @@ class SparkSqlAstBuilder extends AstBuilder {
           } else {
             None
           }
-          (Seq.empty, Option(name), props.toSeq, recordHandler)
+          val finalProps = if (!props.contains("field.delim")) {
+            props.toSeq ++ Seq("field.delim" -> "\t")

Review comment:
       what's the behavior now? which delimiter do we use by default before this PR?




----------------------------------------------------------------
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] AngersZhuuuu commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751628889


   The code has been modified to make it easier for it to add other possible default parameters in specified Hive serde mode.
   FYI @cloud-fan 


----------------------------------------------------------------
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] gatorsmile edited a comment on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
gatorsmile edited a comment on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-755054562


   @AngersZhuuuu Please update the migration guide 


----------------------------------------------------------------
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] AngersZhuuuu commented on a change in pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30942:
URL: https://github.com/apache/spark/pull/30942#discussion_r549253481



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -505,7 +505,12 @@ class SparkSqlAstBuilder extends AstBuilder {
           } else {
             None
           }
-          (Seq.empty, Option(name), props.toSeq, recordHandler)
+          val finalProps = if (!props.contains("field.delim")) {

Review comment:
       > is this case sensitive?
   
   Yes, case sensitive
   ![image](https://user-images.githubusercontent.com/46485123/103199858-d4a37880-4926-11eb-9d58-dae7b809e2c7.png)
   




----------------------------------------------------------------
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] AngersZhuuuu commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751565576


   Add UT 
   ```
   val query4 = sql(
         """
           |SELECT split(value, "&") FROM (
           |SELECT TRANSFORM(a, b, c)
           |  ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe'
           |USING 'cat'
           |  ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe'
           |  WITH SERDEPROPERTIES (
           |   'serialization.last.column.takes.rest' = 'true',
           |   'field.delim' = '&'
           |  )
           |FROM (SELECT 1 AS a, 2 AS b, 3 AS c) t
           |) temp;
         """.stripMargin)
       checkAnswer(query4, identity, Row(null) :: Nil)
   ```
   Since in Hive
   ![image](https://user-images.githubusercontent.com/46485123/103188394-3fdb5380-4903-11eb-8c26-bbbf79f71cb1.png)
   


----------------------------------------------------------------
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] cloud-fan closed pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30942:
URL: https://github.com/apache/spark/pull/30942


   


----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751565754


   **[Test build #133421 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133421/testReport)** for PR 30942 at commit [`79e78f3`](https://github.com/apache/spark/commit/79e78f34b31a3ccdfab40b0a0e95ff9d8fd70f14).


----------------------------------------------------------------
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] AngersZhuuuu commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-755057572


   > @AngersZhuuuu Please update the migration guide
   
   Yea


----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751625971


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133421/
   


----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751565754


   **[Test build #133421 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133421/testReport)** for PR 30942 at commit [`79e78f3`](https://github.com/apache/spark/commit/79e78f34b31a3ccdfab40b0a0e95ff9d8fd70f14).


----------------------------------------------------------------
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] gatorsmile commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-755054562


   @AngersZhuuuu Please upgrade the migration guide 


----------------------------------------------------------------
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] AngersZhuuuu commented on a change in pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on a change in pull request #30942:
URL: https://github.com/apache/spark/pull/30942#discussion_r549225754



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
##########
@@ -505,7 +505,12 @@ class SparkSqlAstBuilder extends AstBuilder {
           } else {
             None
           }
-          (Seq.empty, Option(name), props.toSeq, recordHandler)
+          val finalProps = if (!props.contains("field.delim")) {
+            props.toSeq ++ Seq("field.delim" -> "\t")

Review comment:
       > what's the behavior now? which delimiter do we use by default before this PR?
   
   No default we use '\u0001'




----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751625398


   **[Test build #133421 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133421/testReport)** for PR 30942 at commit [`79e78f3`](https://github.com/apache/spark/commit/79e78f34b31a3ccdfab40b0a0e95ff9d8fd70f14).
    * 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] AmplabJenkins removed a comment on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751625971


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133421/
   


----------------------------------------------------------------
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] AngersZhuuuu commented on pull request #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751565681


   FYI @maropu @cloud-fan 


----------------------------------------------------------------
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 #30942: [SPARK-32685][SQL] When specify serde, default filed.delim is '\t'

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30942:
URL: https://github.com/apache/spark/pull/30942#issuecomment-751578426


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38012/
   


----------------------------------------------------------------
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