[SPARK-2927][SQL] Add a conf to configure if we always read Binary columns stored in Parquet as String columns by yhuai · Pull Request #1855 · apache/spark

yhuai · 2014-08-08T17:24:07Z

This PR adds a new conf flag spark.sql.parquet.binaryAsString. When it is true, if there is no parquet metadata file available to provide the schema of the data, we will always treat binary fields stored in parquet as string fields. This conf is used to provide a way to read string fields generated without UTF8 decoration.

JIRA: https://issues.apache.org/jira/browse/SPARK-2927

…n Parquet as string columns.

SparkQA · 2014-08-08T17:29:31Z

QA tests have started for PR 1855. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18208/consoleFull

marmbrus · 2014-08-08T18:16:39Z

Nit: the if here is redundant.

SparkQA · 2014-08-08T18:40:14Z

QA results for PR 1855:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18208/consoleFull

marmbrus · 2014-08-13T21:46:38Z

@yhuai can you maybe fix the if and then remove WIP? Is this ready to go then?

…ring

yhuai · 2014-08-13T22:57:01Z

Actually, it needs a unit test. Let me take a look at how to add one.

SparkQA · 2014-08-13T23:00:20Z

QA tests have started for PR 1855. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18491/consoleFull

SparkQA · 2014-08-14T00:08:12Z

QA results for PR 1855:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18491/consoleFull

yhuai · 2014-08-14T00:53:04Z

@marmbrus Can you take a look at the unit test? If it is ok, I think this PR is good to go.

marmbrus · 2014-08-14T00:54:45Z

Nice test. Will merge once jenkins is happy.

marmbrus · 2014-08-14T00:54:52Z

Jenkins, test this please.

SparkQA · 2014-08-14T00:55:05Z

QA tests have started for PR 1855. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18503/consoleFull

SparkQA · 2014-08-14T01:00:11Z

QA tests have started for PR 1855. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18504/consoleFull

SparkQA · 2014-08-14T02:08:47Z

QA results for PR 1855:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18503/consoleFull

SparkQA · 2014-08-14T02:14:18Z

QA results for PR 1855:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18504/consoleFull

marmbrus · 2014-08-14T17:47:03Z

Thanks! I've merged this to master and 1.1.

…lumns stored in Parquet as String columns This PR adds a new conf flag `spark.sql.parquet.binaryAsString`. When it is `true`, if there is no parquet metadata file available to provide the schema of the data, we will always treat binary fields stored in parquet as string fields. This conf is used to provide a way to read string fields generated without UTF8 decoration. JIRA: https://issues.apache.org/jira/browse/SPARK-2927 Author: Yin Huai <huai@cse.ohio-state.edu> Closes#1855 from yhuai/parquetBinaryAsString and squashes the following commits: 689ffa9 [Yin Huai] Add missing "=". 80827de [Yin Huai] Unit test. 1765ca4 [Yin Huai] Use .toBoolean. 9d3f199 [Yin Huai] Merge remote-tracking branch 'upstream/master' into parquetBinaryAsString 5d436a1 [Yin Huai] The initial support of adding a conf to treat binary columns stored in Parquet as string columns. (cherry picked from commit add75d4) Signed-off-by: Michael Armbrust <michael@databricks.com>

chutium · 2014-08-15T14:25:00Z

this patch will be great for impala users like us :) thanks, moreover, there is a getCreatedBy method in readMetaData(origPath, conf).getFileMetaData, and impala creates parquet files always with its own CreatedBy information (always contains string "impala"), so, maybe we can do some auto-detection like (https://github.com/apache/spark/pull/1599/files)
if (fileMetaData.getCreatedBy.contains("impala")) { isBinaryAsString = true log.info(s"Impala parquet file found, blabla...") }
does this auto-detection make sense?

My only concern with auto detection like this is, what happens when impala starts adding the correct annotation and supporting byte arrays?

good question, such a auto detection brings confusion, this is a problem of impala, not spark sql, we are not going to make a impala file format corrector :)

…lumns stored in Parquet as String columns This PR adds a new conf flag `spark.sql.parquet.binaryAsString`. When it is `true`, if there is no parquet metadata file available to provide the schema of the data, we will always treat binary fields stored in parquet as string fields. This conf is used to provide a way to read string fields generated without UTF8 decoration. JIRA: https://issues.apache.org/jira/browse/SPARK-2927 Author: Yin Huai <huai@cse.ohio-state.edu> Closesapache#1855 from yhuai/parquetBinaryAsString and squashes the following commits: 689ffa9 [Yin Huai] Add missing "=". 80827de [Yin Huai] Unit test. 1765ca4 [Yin Huai] Use .toBoolean. 9d3f199 [Yin Huai] Merge remote-tracking branch 'upstream/master' into parquetBinaryAsString 5d436a1 [Yin Huai] The initial support of adding a conf to treat binary columns stored in Parquet as string columns.

The initial support of adding a conf to treat binary columns stored i…
5d436a1
…n Parquet as string columns.

marmbrus reviewed Aug 8, 2014
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala Outdated
marmbrusAug 8, 2014
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: the if here is redundant.

yhuai added 2 commits August 13, 2014 18:46

Merge remote-tracking branch 'upstream/master' into parquetBinaryAsSt…
9d3f199
…ring

Use .toBoolean.
1765ca4

yhuai changed the title ~~[WIP][SPARK-2927][SQL] Add a conf to configure if we always read Binary columns stored in Parquet as String columns~~[SPARK-2927][SQL] Add a conf to configure if we always read Binary columns stored in Parquet as String columnsAug 13, 2014

yhuai added 2 commits August 13, 2014 20:49

Unit test.
80827de

Add missing "=".
689ffa9

asfgit closed this in add75d4Aug 14, 2014

chutium reviewed Aug 15, 2014
View reviewed changes

yhuai deleted the parquetBinaryAsString branch October 6, 2014 16:24

Uh oh!

Conversation

yhuai commented Aug 8, 2014

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

marmbrusAug 8, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 8, 2014

Uh oh!

marmbrus commented Aug 13, 2014

Uh oh!

yhuai commented Aug 13, 2014

Uh oh!

SparkQA commented Aug 13, 2014

Uh oh!

SparkQA commented Aug 14, 2014

Uh oh!

yhuai commented Aug 14, 2014

Uh oh!

marmbrus commented Aug 14, 2014

Uh oh!

marmbrus commented Aug 14, 2014

Uh oh!

SparkQA commented Aug 14, 2014

Uh oh!

SparkQA commented Aug 14, 2014

Uh oh!

SparkQA commented Aug 14, 2014

Uh oh!

SparkQA commented Aug 14, 2014

Uh oh!

marmbrus commented Aug 14, 2014

Uh oh!

chutiumAug 15, 2014

Choose a reason for hiding this comment

Uh oh!

marmbrusAug 16, 2014

Choose a reason for hiding this comment

Uh oh!

chutiumAug 18, 2014

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants