Skip to content

[SPARK-11694][SQL] Parquet logical types are not being tested properly #9660

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.

Sign up for GitHub

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

Jump to bottom
Closed
HyukjinKwon wants to merge 7 commits into from
Closed

[SPARK-11694][SQL] Parquet logical types are not being tested properly #9660

HyukjinKwon wants to merge 7 commits into from

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Nov 12, 2015

All the physical types are properly tested at ParquetIOSuite but logical type mapping is not being tested.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 12, 2015

cc @liancheng

liancheng
liancheng reviewed Nov 12, 2015
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetIOSuite.scala Outdated
Copy link
Contributor

@liancheng liancheng Nov 12, 2015

Choose a reason for hiding this comment

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

What's the +?

Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 12, 2015

Choose a reason for hiding this comment

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

Hm. I should get rid of this. I ran this test several times and added a PR. This is weird. Sorry.

@SparkQA
Copy link

SparkQA commented Nov 12, 2015

Test build #45729 has finished for PR 9660 at commit 870a37a.

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

liancheng
liancheng reviewed Nov 12, 2015
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetIOSuite.scala Outdated
Copy link
Contributor

@liancheng liancheng Nov 12, 2015

Choose a reason for hiding this comment

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

Can we add all Parquet logical types that Spark supports here for completeness? For example:

fixed_len_byte_array f0(DECIMAL(d, s))
binary f1(UTF8);

@SparkQA
Copy link

SparkQA commented Nov 12, 2015

Test build #45733 has finished for PR 9660 at commit 9a5c2a3.

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

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45791 has finished for PR 9660 at commit 02f3ef9.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45798 has finished for PR 9660 at commit 22e1374.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

hm. I'm not using FileMetaData at line 234 in ParquetIOSuite.

BTW I will correct the other tests to use the overloaded function in other PRs in a followup PR for this issue.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45807 has finished for PR 9660 at commit 22e1374.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

I added an import FileMetaData at ParquetIOSuite.

This is weird, I am suing scala 2.10 and it compiles okay in my local computer.

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45812 has finished for PR 9660 at commit 8e3e470.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

Also added an import ParquetMetadata at ParquetIOSuite

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45817 has finished for PR 9660 at commit 3c7277a.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

Also added an import Collections at ParquetIOSuite.

Does this also compile the old version of all commits in this PR?

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45830 has finished for PR 9660 at commit ce36be0.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

restart this please

@liancheng
Copy link
Contributor

liancheng commented Nov 13, 2015

@HyukjinKwon The spell is "retest this please" :)

@liancheng
Copy link
Contributor

liancheng commented Nov 13, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Nov 13, 2015

Test build #45856 has finished for PR 9660 at commit ce36be0.

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

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 13, 2015

Finally I got all-pass! Pleace review the codes!

@liancheng
Copy link
Contributor

liancheng commented Nov 14, 2015

Thanks! Merging to master and branch-1.6.

@asfgit asfgit closed this in 139c15b Nov 14, 2015
asfgit pushed a commit that referenced this pull request Nov 14, 2015
@HyukjinKwon @liancheng
[SPARK-11694][SQL] Parquet logical types are not being tested properly
47a9186 
All the physical types are properly tested at `ParquetIOSuite` but logical type mapping is not being tested.

Author: hyukjinkwon <gurwls223@gmail.com>
Author: Hyukjin Kwon <gurwls223@gmail.com>

Closes #9660 from HyukjinKwon/SPARK-11694.

(cherry picked from commit 139c15b)
Signed-off-by: Cheng Lian <lian@databricks.com>
@HyukjinKwon HyukjinKwon mentioned this pull request Nov 17, 2015
Closed
asfgit pushed a commit that referenced this pull request Nov 17, 2015
@HyukjinKwon @liancheng
[SPARK-11694][FOLLOW-UP] Clean up imports, use a common function for …
75d2020 
…metadata and add a test for FIXED_LEN_BYTE_ARRAY

As discussed #9660 #9060, I cleaned up unused imports, added a test for fixed-length byte array and used a common function for writing metadata for Parquet.

For the test for fixed-length byte array, I have tested and checked the encoding types with [parquet-tools](https://github.com/Parquet/parquet-mr/tree/master/parquet-tools).

Author: hyukjinkwon <gurwls223@gmail.com>

Closes #9754 from HyukjinKwon/SPARK-11694-followup.
kiszk pushed a commit to kiszk/spark-gpu that referenced this pull request Dec 26, 2015
@HyukjinKwon @kiszk
[SPARK-11694][FOLLOW-UP] Clean up imports, use a common function for …
755a985 
…metadata and add a test for FIXED_LEN_BYTE_ARRAY

As discussed apache/spark#9660 apache/spark#9060, I cleaned up unused imports, added a test for fixed-length byte array and used a common function for writing metadata for Parquet.

For the test for fixed-length byte array, I have tested and checked the encoding types with [parquet-tools](https://github.com/Parquet/parquet-mr/tree/master/parquet-tools).

Author: hyukjinkwon <gurwls223@gmail.com>

Closes #9754 from HyukjinKwon/SPARK-11694-followup.
@HyukjinKwon HyukjinKwon deleted the SPARK-11694 branch September 23, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HyukjinKwon @SparkQA @liancheng