HIVE-24682: Collect dynamic partition info in FileSink for direct insert #1915
Conversation
|
@kuczoram could you please review this? |
kgyrtkirk
added
tests failed
tests pending
tests unstable
and removed
tests pending
tests failed
labels
| POSTHOOK: Lineage: multiinsert_test_mm PARTITION(c=2222).a SIMPLE [(multiinsert_test_text)a.FieldSchema(name:a, type:int, comment:null), ] | ||
| POSTHOOK: Lineage: multiinsert_test_mm PARTITION(c=2222).b SIMPLE [(multiinsert_test_text)a.FieldSchema(name:b, type:int, comment:null), ] | ||
| POSTHOOK: Lineage: multiinsert_test_mm PARTITION(c=__HIVE_DEFAULT_PARTITION__).a SIMPLE [(multiinsert_test_text)a.FieldSchema(name:a, type:int, comment:null), ] | ||
| POSTHOOK: Lineage: multiinsert_test_mm PARTITION(c=__HIVE_DEFAULT_PARTITION__).b SIMPLE [(multiinsert_test_text)a.FieldSchema(name:b, type:int, comment:null), ] | ||
| POSTHOOK: Lineage: multiinsert_test_mm PARTITION(c=__HIVE_DEFAULT_PARTITION__).a SIMPLE [(multiinsert_test_text)a.FieldSchema(name:a, type:int, comment:null), ] |
There was a problem hiding this comment.
are these duplicate rows in the lineage info?
There was a problem hiding this comment.
yes these were duplicates
| @@ -1649,8 +1649,6 @@ POSTHOOK: Lineage: ###Masked### | |||
| POSTHOOK: Lineage: ###Masked### | |||
| POSTHOOK: Lineage: ###Masked### | |||
| POSTHOOK: Lineage: ###Masked### | |||
| POSTHOOK: Lineage: ###Masked### | |||
There was a problem hiding this comment.
Were these also duplicated rows? Could you please check the q.out.orig files before the masking?
| return (fullName.startsWith(BASE_PREFIX) && !dirName.startsWith(BASE_PREFIX)) || | ||
| (fullName.startsWith(DELTA_PREFIX) && !dirName.startsWith(DELTA_PREFIX)) || | ||
| (fullName.startsWith(DELETE_DELTA_PREFIX) && !dirName.startsWith(DELETE_DELTA_PREFIX)); | ||
| return (!dirName.startsWith(BASE_PREFIX)) && !dirName.startsWith(DELTA_PREFIX) && !dirName.startsWith(DELETE_DELTA_PREFIX) |
There was a problem hiding this comment.
Is this an extra () on the first one?
| (fullName.startsWith(DELTA_PREFIX) && !dirName.startsWith(DELTA_PREFIX)) || | ||
| (fullName.startsWith(DELETE_DELTA_PREFIX) && !dirName.startsWith(DELETE_DELTA_PREFIX)); | ||
| return (!dirName.startsWith(BASE_PREFIX)) && !dirName.startsWith(DELTA_PREFIX) && !dirName.startsWith(DELETE_DELTA_PREFIX) | ||
| && (fullName.contains(BASE_PREFIX) || fullName.contains(DELTA_PREFIX) || fullName.contains(DELETE_DELTA_PREFIX)); |
There was a problem hiding this comment.
If I understand correctly this is changing the logic. Is it intentional?
There was a problem hiding this comment.
Yes. Previously it was working only for rootpath like delta_00001/subdir/, now it works for table/partition=1/delta_00001/subdir
The reordering was needed because fullName.contains(delta_prefix) && !dirname.startWith(delta_prefix) is true for delete_delta_000001, and it would yield wrong result
What changes were proposed in this pull request?
The dynamic partition infos can be collected from the manifest files, no need to do a costly file listing later in the MoveTask
Why are the changes needed?
Performance improvement
Does this PR introduce any user-facing change?
No.
How was this patch tested?
All unit tests are working, local performance tests