[HUDI-1879] Support Partition Prune For MergeOnRead Snapshot Table #2926
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pengzhiwei2018
force-pushed
the
dev_1879_issue2
branch
from
35852ae
to
31e95ca
Compare
Codecov Report
@@ Coverage Diff @@
## master #2926 +/- ##
============================================
+ Coverage 53.00% 55.11% +2.10%
- Complexity 3743 3845 +102
============================================
Files 488 485 -3
Lines 23435 23512 +77
Branches 2500 2519 +19
============================================
+ Hits 12422 12958 +536
+ Misses 9913 9400 -513
- Partials 1100 1154 +54
Flags with carried forward coverage won't be shown. Click here to find out more.
|
nsivabalan
added
the
priority:major
garyli1019
reviewed
May 15, 2021
| @ParameterizedTest | ||
| @ValueSource(booleans = Array(true, false)) | ||
| def testMORPartitionPrune(partitionEncode: Boolean): Unit = { | ||
| val partitions = Array("2021/03/01", "2021/03/02", "2021/03/03", "2021/03/04", "2021/03/05") |
There was a problem hiding this comment.
does the hive style partitioning year=2021/xxx work as well?
There was a problem hiding this comment.
Yes, the hive style partition is supported.
There was a problem hiding this comment.
should we add a test for hive style as well?
There was a problem hiding this comment.
Nice suggestion! done!
pengzhiwei2018
force-pushed
the
dev_1879_issue2
branch
from
7c720e7
to
5ec8b18
Compare
vinothchandar
approved these changes
May 25, 2021
There was a problem hiding this comment.
Few nits. LGTM overall. We can land once @umehrot2 also take a pass and @garyli1019 is happy with the change
|
|
||
| // Get partition filter and convert to catalyst expression | ||
| val partitionColumns = hoodieFileIndex.partitionSchema.fieldNames.toSet | ||
| val partitionFilters= filters.filter(f => f.references.forall(p => partitionColumns.contains(p))) |
| * Convert Filters to Catalyst Expressions and joined by And. If convert success return an | ||
| * Non-Empty Option[Expression],or else return None. | ||
| */ | ||
| def convertToCatalystExpressions(filters: Array[Filter], |
There was a problem hiding this comment.
can we encapsulate this conversion logic into its own class. I could see general use for this, beyond just partition pruning?
There was a problem hiding this comment.
Make sense to me!
5 tasks
pengzhiwei2018
force-pushed
the
dev_1879_issue2
branch
from
edf6c6b
to
6736f3e
Compare
pengzhiwei2018
force-pushed
the
dev_1879_issue2
branch
from
7765cce
to
c4b07cb
Compare
nsivabalan
approved these changes
May 29, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the pull request
Support partition pruned for merge on read snapshot query. This issue was exposed by #2283 which read hudi as spark DataSource table.
Brief change log
filtersinMergeOnReadSnapshotRelation#buildScanto Catalyst Expression and do the partition prune byHoodieFileIndexVerify this pull request
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.