Skip to content

SPARK-5049: ParquetTableScan always prepends the values of partition col... #3870

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
wants to merge 2 commits into from
Closed

SPARK-5049: ParquetTableScan always prepends the values of partition col... #3870

wants to merge 2 commits into from

Conversation

@rahulaggarwalguavus
Copy link

@rahulaggarwalguavus rahulaggarwalguavus commented Jan 1, 2015

SPARK-5049: ParquetTableScan always prepends the values of partition columns in output rows irrespective of the order of the partition columns in the original SELECT query

  • now forming a GenericRow by inserting column values at correct indexes

SPARK-5049: ParquetTableScan always prepends the values of partition …
5000110 
…columns in output rows irrespective of the order of the partition columns in the original SELECT query

- forming a Generic row by inserting column values are correct indexes
@AmplabJenkins
Copy link

AmplabJenkins commented Jan 1, 2015

Can one of the admins verify this patch?

@ash211
Copy link
Contributor

ash211 commented Jan 2, 2015

Jenkins this is ok to test

@SparkQA
Copy link

SparkQA commented Jan 2, 2015

Test build #24989 has started for PR 3870 at commit 5000110.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 2, 2015

Test build #24989 has finished for PR 3870 at commit 5000110.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Jan 2, 2015

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

SPARK-5049: ParquetTableScan always prepends the values of partition …
8253a7c 
…columns in output rows irrespective of the order of the partition columns in the original SELECT query

- passing newOutput(correct sequence of attributes) in OutputFaker
@SparkQA
Copy link

SparkQA commented Jan 4, 2015

Test build #25031 has started for PR 3870 at commit 8253a7c.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 4, 2015

Test build #25031 has finished for PR 3870 at commit 8253a7c.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Jan 4, 2015

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

@marmbrus marmbrus mentioned this pull request Jan 11, 2015
Closed
@marmbrus
Copy link
Contributor

marmbrus commented Jan 11, 2015

Thanks for figuring this out and proposing a solution! I guess our test cases missed this since they always perform later column reordering.

I'm a little concerned about the performance impact of this part of the change though:

// Fill outputRow with iter.next()._2 at the correct indexes using normalOutputIndexes
iter.next()._2
  .zipWithIndex
  .foreach(nI => outputRow(normalOutputIndexes(nI._2)) = nI._1)
  new GenericRow(outputRow)

It's both functional programming (which I normally love, but try to avoid in per-tuple codepaths) and allocates an object.

What do you think of the approach I took in #3990?

@rahulaggarwalguavus
Copy link
Author

rahulaggarwalguavus commented Jan 11, 2015

Thanks for reviewing. Yes, the approach you took in #3990 avoids this performance penalty.

asfgit pushed a commit that referenced this pull request Jan 12, 2015
@marmbrus
[SPARK-5049][SQL] Fix ordering of partition columns in ParquetTableScan
6d23af6 
Followup to #3870.  Props to rahulaggarwalguavus for identifying the issue.

Author: Michael Armbrust <[email protected]>

Closes #3990 from marmbrus/SPARK-5049 and squashes the following commits:

dd03e4e [Michael Armbrust] Fill in the partition values of parquet scans instead of using JoinedRow

(cherry picked from commit 5d9fa55)
Signed-off-by: Michael Armbrust <[email protected]>
asfgit pushed a commit that referenced this pull request Jan 12, 2015
@marmbrus
[SPARK-5049][SQL] Fix ordering of partition columns in ParquetTableScan
5d9fa55 
Followup to #3870.  Props to rahulaggarwalguavus for identifying the issue.

Author: Michael Armbrust <[email protected]>

Closes #3990 from marmbrus/SPARK-5049 and squashes the following commits:

dd03e4e [Michael Armbrust] Fill in the partition values of parquet scans instead of using JoinedRow
@yhuai
Copy link
Contributor

yhuai commented Jan 21, 2015

Since this issue has been fixed by #3990, we can close it.

@yhuai
Copy link
Contributor

yhuai commented Jan 21, 2015

close this issue

@asfgit asfgit closed this in 622ff09 Jan 28, 2015
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.

6 participants

@rahulaggarwalguavus @AmplabJenkins @ash211 @SparkQA @marmbrus @yhuai