Support PathFilter when fetching splits for Hudi tables by bhasudha · Pull Request #13818 · prestodb/presto

bhasudha · 2019-12-07T06:36:27Z

This PR is corresponds to the issue - #13511
Summary:
- Introduce PathFilter support in DirectoryLister interface and HiveFileIterator
- Unit test HiveFileIterator using PathFilter
- Plug specific PathFilter implementation for HoodieParquetInputFormat

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* ...
* ...

Hive Changes
* ...
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

linux-foundation-easycla · 2019-12-07T06:36:30Z

The committers are authorized under a signed CLA.

✅ Bhavani Sudha Saktheeswaran (cb2631d6754f0f8304c1bf607e25cae584c7b7b3)

bhasudha · 2019-12-07T06:37:32Z

@arhimondr please take a look when you can. Thanks much!

shixuan-fan

Quickly glanced through. Flush some comments. Looks good overall.

presto-hive/src/main/java/com/facebook/presto/hive/BackgroundHiveSplitLoader.java

presto-hive/src/main/java/com/facebook/presto/hive/DefaultPathFilterProvider.java

presto-hive/src/test/java/com/facebook/presto/hive/HiveBenchmarkQueryRunner.java

wenleix · 2020-01-08T06:43:36Z

@bhasudha : Assume this is ready for review, I will remove the [WIP] in the title.

bhasudha · 2020-01-08T22:06:41Z

@bhasudha : Assume this is ready for review, I will remove the [WIP] in the title.

@wenleix I kept it as WIP since I am still testing it. I have some related questions. When I bring up local presto server, I believe presto-hudi tries to register two connectors - hudi and hive (since its extending the HiveHadoop2Plugin). I see error saying hive connector is already registered. I am wondering if this is got to do with the way I implemented the presto-hudi connector. Should I change HudiPlugin to extend from HivePlugin like how HiveHadoop2Plugin does instead of extending HiveHadoop2Plugin? Do you have any recommendations around this?

presto-hive/src/main/java/com/facebook/presto/hive/DefaultPathFilterProvider.java

shixuan-fan · 2020-01-08T22:13:52Z

@bhasudha In our internal extension of hive connector, we didn't extend HivePlugin. We just implemented Plugin, and return a HiveConnector in ConnectorFactory#create.

presto-hive-hadoop2/src/main/java/com/facebook/presto/hive/HiveHadoop2Plugin.java

presto-hive/src/main/java/com/facebook/presto/hive/DefaultPathFilterProvider.java

presto-hive/src/main/java/com/facebook/presto/hive/PathFilterProvider.java

presto-hive/src/main/java/com/facebook/presto/hive/BackgroundHiveSplitLoader.java

presto-hudi/pom.xml

presto-hive/src/main/java/com/facebook/presto/hive/BackgroundHiveSplitLoader.java

bhasudha · 2020-01-23T21:42:39Z

[UPDATE] After further discussions, we decided we will move this implementation inside presto-hive instead of a seperate connector for now. Presto-hive can support HoodieParquetInputFormat as a first class citizen eventually. Depending on future requirements, moving hudi to a seperate connector can be considered. I will update this PR and send out the changes soon. @arhimondr @wenleix @highker

bhasudha · 2020-02-07T08:48:37Z

@arhimondr I updated the PR based on our last discussion. Please review when possible. I noticed that the Travis build failed in presto-cassandra module. Looks like it might be a transient connection issue. Would you be able to rekick the build ? I cant see an option for restarting the build in Travis. Thanks much!

arhimondr

LGTM % bunch of nits

presto-hive/src/main/java/com/facebook/presto/hive/BackgroundHiveSplitLoader.java

presto-hive/src/main/java/com/facebook/presto/hive/util/HiveFileIterator.java

presto-hive/src/test/java/com/facebook/presto/hive/util/TestHiveFileIterator.java

- Introduce PathFilter support in DirectoryLister interface and HiveFileIterator - Unit test HiveFileIterator using PathFilter - Plug specific PathFilter implementation for HoodieParquetInputFormat

arhimondr

LGTM

CC: @wenleix @highker

Summary: - Introduce PathFilter support in DirectoryLister interface and HiveFileIterator - Unit test HiveFileIterator using PathFilter - Plug specific PathFilter implementation for HoodieParquetInputFormat Combines following PRs into one: prestodb#13818 prestodb#14085 prestodb#14088 Reviewers: bhasudha Differential Revision: https://code.uberinternal.com/D4168579

rschlussel requested a review from arhimondr December 11, 2019 15:43

shixuan-fan reviewed Dec 16, 2019

View reviewed changes

bhasudha force-pushed the presto-hudi-connector branch from 89d58da to cb2631d Compare January 7, 2020 01:07

wenleix changed the title ~~[WIP] Presto Hudi wrapper connector implementation~~ Presto Hudi wrapper connector implementation Jan 8, 2020

wenleix reviewed Jan 8, 2020

View reviewed changes

presto-hive/src/main/java/com/facebook/presto/hive/DefaultPathFilterProvider.java Outdated Show resolved Hide resolved

arhimondr reviewed Jan 10, 2020

View reviewed changes

bhasudha force-pushed the presto-hudi-connector branch from cb2631d to eded1a3 Compare January 14, 2020 00:56

bhasudha commented Jan 14, 2020

View reviewed changes

presto-hive/src/main/java/com/facebook/presto/hive/BackgroundHiveSplitLoader.java Outdated Show resolved Hide resolved

bhasudha force-pushed the presto-hudi-connector branch from eded1a3 to 6799609 Compare February 7, 2020 00:03

bhasudha changed the title ~~Presto Hudi wrapper connector implementation~~ Support PathFilter when fetching splits for Hudi tables Feb 7, 2020

arhimondr reviewed Feb 10, 2020

View reviewed changes

bhasudha force-pushed the presto-hudi-connector branch from 6799609 to 090b5f4 Compare February 10, 2020 22:50

Support PathFilter when fetching splits for Hudi tables

f2e5fa8

- Introduce PathFilter support in DirectoryLister interface and HiveFileIterator - Unit test HiveFileIterator using PathFilter - Plug specific PathFilter implementation for HoodieParquetInputFormat

bhasudha force-pushed the presto-hudi-connector branch from 090b5f4 to f2e5fa8 Compare February 11, 2020 00:04

arhimondr approved these changes Feb 11, 2020

View reviewed changes

arhimondr merged commit 9fd2459 into prestodb:master Feb 11, 2020

bhasudha mentioned this pull request Feb 11, 2020

Introduce PathFilter support in DirectoryLister #13590

Closed

tooptoop4 mentioned this pull request Feb 21, 2020

Support PathFilter when fetching splits for Hudi tables trinodb/trino#2903

Closed

PennyAndWang mentioned this pull request Jun 9, 2021

Support hudi mor table trinodb/trino#8240

Closed

codope mentioned this pull request Oct 14, 2021

Support Hudi path filter and MoR queries trinodb/trino#9641

Closed

codope mentioned this pull request Dec 17, 2021

Improve listing performance of Hudi tables #17084

Merged

Conversation

bhasudha commented Dec 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Dec 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bhasudha commented Dec 7, 2019

Uh oh!

shixuan-fan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wenleix commented Jan 8, 2020

Uh oh!

bhasudha commented Jan 8, 2020

Uh oh!

Uh oh!

shixuan-fan commented Jan 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bhasudha commented Jan 23, 2020

Uh oh!

bhasudha commented Feb 7, 2020

Uh oh!

arhimondr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arhimondr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bhasudha commented Dec 7, 2019 •

edited

Loading

linux-foundation-easycla bot commented Dec 7, 2019 •

edited

Loading