Skip to content

Add DynamicFilter to ConnectorRecordSetProvider SPI#8046

Closed
jerryleooo wants to merge 1 commit intotrinodb:masterfrom
jerryleooo:impl-dynamic-filtering-for-recordsetprovider
Closed

Add DynamicFilter to ConnectorRecordSetProvider SPI#8046
jerryleooo wants to merge 1 commit intotrinodb:masterfrom
jerryleooo:impl-dynamic-filtering-for-recordsetprovider

Conversation

@jerryleooo
Copy link
Copy Markdown
Member

@jerryleooo jerryleooo commented May 23, 2021

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented May 23, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@jerryleooo jerryleooo changed the title implement dynamic filtering for ConnectorRecordSetProvider WIP: implement dynamic filtering for ConnectorRecordSetProvider May 23, 2021
@jerryleooo jerryleooo closed this May 23, 2021
@jerryleooo jerryleooo reopened this May 24, 2021
@cla-bot
Copy link
Copy Markdown

cla-bot bot commented May 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@jerryleooo jerryleooo changed the title WIP: implement dynamic filtering for ConnectorRecordSetProvider Implement dynamic filtering for ConnectorRecordSetProvider May 24, 2021
@jerryleooo
Copy link
Copy Markdown
Member Author

jerryleooo commented May 25, 2021

reopen to refresh the CLA

@jerryleooo jerryleooo closed this May 25, 2021
@jerryleooo jerryleooo reopened this May 25, 2021
@martint
Copy link
Copy Markdown
Member

martint commented May 25, 2021

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label May 25, 2021
@cla-bot
Copy link
Copy Markdown

cla-bot bot commented May 25, 2021

The cla-bot has been summoned, and re-checked this pull request!

electrum
electrum approved these changes May 25, 2021
View reviewed changes
@jerryleooo jerryleooo force-pushed the impl-dynamic-filtering-for-recordsetprovider branch 2 times, most recently from 2669a37 to ebed666 Compare May 26, 2021 14:11
@jerryleooo jerryleooo requested a review from raunaqmorarka May 26, 2021 14:48
raunaqmorarka
raunaqmorarka previously approved these changes May 26, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

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

I would reword the commit message to :
Add DynamicFilter to ConnectorRecordSetProvider SPI

@jerryleooo jerryleooo changed the title Implement dynamic filtering for ConnectorRecordSetProvider Add DynamicFilter to ConnectorRecordSetProvider SPI May 27, 2021
@jerryleooo jerryleooo force-pushed the impl-dynamic-filtering-for-recordsetprovider branch from ebed666 to 62c11ed Compare May 27, 2021 01:38
@jerryleooo
Copy link
Copy Markdown
Member Author

jerryleooo commented May 27, 2021

@raunaqmorarka updated. Then how to ask for a merge? Thanks

@raunaqmorarka
Copy link
Copy Markdown
Member

raunaqmorarka commented May 27, 2021

@raunaqmorarka updated. Then how to ask for a merge? Thanks

@jerryleooo we need to update ClassLoaderSafeConnectorRecordSetProvider as well

@jerryleooo
Copy link
Copy Markdown
Member Author

jerryleooo commented May 27, 2021

@electrum @raunaqmorarka all checks passed, need I squash the commits into one? Thanks

@jerryleooo jerryleooo force-pushed the impl-dynamic-filtering-for-recordsetprovider branch from c3c424f to 7f432bf Compare May 28, 2021 00:08
@jerryleooo
Copy link
Copy Markdown
Member Author

jerryleooo commented May 31, 2021

@electrum can I get this merged?

@jerryleooo jerryleooo mentioned this pull request Jun 3, 2021
Closed
@raunaqmorarka
Copy link
Copy Markdown
Member

raunaqmorarka commented Jun 28, 2022

If this is really needed let's do this as a commit in the PR which adds DF for JDBC sources.
It's probably better to use JdbcSplitManager instead per #8137 (comment)

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

Reviewers

@electrum electrum electrum approved these changes

@raunaqmorarka raunaqmorarka raunaqmorarka left review comments

Assignees

No one assigned

Labels

cla-signed

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jerryleooo @martint @raunaqmorarka @electrum