-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
HIVE-24350: NullScanOptimizer can lookup emptiness info from stats fo… #1645
Conversation
82ccff0
to
3265bfd
Compare
} | ||
} | ||
|
||
for (Entry<Path, Collection<String>> entry : candidatePathsToAliases.entrySet()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this have the same issue for external tables (e.g text?). For
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to performance issue? Yes this PR does not change anything for external tables. We cannot rely on statistics for external tables. I have another patch ready (need to test first) that deals with external tables. We'll run listStatus calls in parallel for external tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sharing the details. HIVE-24380 is what you are referring to I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
if (pathToPartitionMap == null) { | ||
continue; | ||
} | ||
addCandidatePath(candidatePathsToAliases, path, alias); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be candidatePathsToAliases.computeIfAbsent(path, k -> new ArrayList<>()).add(alias) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
3265bfd
to
87ee0cd
Compare
…r managed tables Change-Id: I7cf7b8f7738c5660ee1c61bcb0456f1c70efe492
87ee0cd
to
ea76f4a
Compare
LGTM pending tests. |
…r managed tables
Change-Id: I7cf7b8f7738c5660ee1c61bcb0456f1c70efe492
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?