Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Conversation

@ajacques
Copy link

Improvements as requested by Spark maintainers in apache#21889

@mallman
Copy link

mallman commented Aug 16, 2018

Thanks for your PR, @ajacques. I will review in earnest.

@mallman
Copy link

mallman commented Aug 17, 2018

@ajacques Thanks for addressing the merge conflicts. I've been occupied with some other work-related responsibilities today. I'll try to get to this tomorrow (Friday).

Cheers.

@mallman
Copy link

mallman commented Aug 17, 2018

@ajacques Should I give co-attribution to @HyukjinKwon for this work? As-is it only has your name on it.

@HyukjinKwon
Copy link

I'm okay in any way.

@mallman
Copy link

mallman commented Aug 17, 2018

@ajacques I removed my changes to ParquetTest.scala upstream. Can you do a rebase and verify that your PR includes no changes to ParquetTest.scala? Thanks.

Copy link

Choose a reason for hiding this comment

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

I don't see field1 in structOfArray. Please remove it here or add it there.

Copy link
Author

Choose a reason for hiding this comment

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

You're correct, good catch. I've removed this one. field1 is belongs in the previous schema.

@ajacques
Copy link
Author

@ajacques Should I give co-attribution to @HyukjinKwon for this work? As-is it only has your name on it.

Sure, let's do that. HyukjinKwon's comments were a valuable resource in this pull request. Is there anything I need to do on my end for this?

@ajacques I removed my changes to ParquetTest.scala upstream. Can you do a rebase and verify that your PR includes no changes to ParquetTest.scala? Thanks.

Rebased, and checked that there's no changes to that file now.

@mallman mallman merged commit 3056896 into VideoAmp:spark-4502-parquet_column_pruning-foundation Aug 20, 2018
@mallman
Copy link

mallman commented Aug 20, 2018

Merged. Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants