Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-21786][SQL][FOLLOWUP] Add compressionCodec test for CTAS #22302

Closed
wants to merge 1 commit into from

Conversation

fjh100456
Copy link
Contributor

@fjh100456 fjh100456 commented Aug 31, 2018

What changes were proposed in this pull request?

Before Apache Spark 2.3, table properties were ignored when writing data to a hive table(created with STORED AS PARQUET/ORC syntax), because the compression configurations were not passed to the FileFormatWriter in hadoopConf. Then it was fixed in #20087. But actually for CTAS with USING PARQUET/ORC syntax, table properties were ignored too when convertMastore, so the test case for CTAS not supported.

Now it has been fixed in #20522 , the test case should be enabled too.

How was this patch tested?

This only re-enables the test cases of previous PR.

What changes were proposed in this pull request?
Since resolved by @DongJoon in 20522, compressionCodec test for CTAS has been able to support, the scenario of CTAS suggested by @gatorsmile in 20087 should be enabled.

How was this patch tested?
Add test.
@maropu
Copy link
Member

maropu commented Sep 1, 2018

@gatorsmile @ueshin Can you trigger this? I checked the related jira and code and, then I think these tests should be passed in master when usingCTAS=true.

@maropu
Copy link
Member

maropu commented Sep 1, 2018

@fjh100456 Can you format the PR description cleanly to make others more understood?

@ueshin
Copy link
Member

ueshin commented Sep 1, 2018

ok to test.

@SparkQA
Copy link

SparkQA commented Sep 1, 2018

Test build #95572 has finished for PR 22302 at commit c3c5af2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@fjh100456
Copy link
Contributor Author

@maropu I'd update the PR description, thank you!

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Sep 6, 2018

@fjh100456 . I'm @dongjoon-hyun . I think you put a wrong person in the the PR description. :)
BTW, you don't need to mention people (including me). It would be enough to mention SPARK JIRA IDs.

@maropu
Copy link
Member

maropu commented Sep 6, 2018

@fjh100456 please do not break the PR format, and can you clean up, too?

@fjh100456
Copy link
Contributor Author

@dongjoon-hyun @maropu I'm so sorry, and I have changed the description. Is it ok now ?

@dongjoon-hyun
Copy link
Member

nit. Maybe, it seems that you removed ##. You may want to simplify the PR description. This only re-enables the test cases of previous your PR.

## What changes were proposed in this pull request?
...

## How was this patch tested?
...

@maropu
Copy link
Member

maropu commented Sep 6, 2018

btw, the behaivour differences of TABLEPROPERTIES and OPTIONS have already been documented somewhere? #20120 (comment)

@fjh100456
Copy link
Contributor Author

fjh100456 commented Sep 6, 2018

The pr #20120 was closed, instead SPARK-23355 had resolved it.
Does it mean the same thing? #21269

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. cc @gatorsmile for final sign-off

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Sep 6, 2018

Test build #95736 has finished for PR 22302 at commit c3c5af2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Merged to master/branch-2.4.

asfgit pushed a commit that referenced this pull request Sep 7, 2018
## What changes were proposed in this pull request?
Before Apache Spark 2.3, table properties were ignored when writing data to a hive table(created with STORED AS PARQUET/ORC syntax), because the compression configurations were not passed to the FileFormatWriter in hadoopConf. Then it was fixed in #20087. But actually for CTAS with USING PARQUET/ORC syntax, table properties were ignored too when convertMastore, so the test case for CTAS not supported.

Now it has been fixed  in #20522 , the test case should be enabled too.

## How was this patch tested?
This only re-enables the test cases of previous PR.

Closes #22302 from fjh100456/compressionCodec.

Authored-by: fjh100456 <fu.jinhua6@zte.com.cn>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 473f2fb)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@asfgit asfgit closed this in 473f2fb Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants