Skip to content

Deprecate passing constraints to getTableStatistics#11877

Merged
findepi merged 2 commits intotrinodb:masterfrom
nineinchnick:get-table-stats-constraints
Apr 15, 2022
Merged

Deprecate passing constraints to getTableStatistics#11877
findepi merged 2 commits intotrinodb:masterfrom
nineinchnick:get-table-stats-constraints

Conversation

@nineinchnick
Copy link
Member

Description

Deprecate passing constraints to ConnectorMetadata.getTableStatistics(), because it always received Constraints.alwaysTrue(). Connectors should handle constraints in applyFilter(), which prepare the table handle that later will be passed to getTableStatistics().

Is this change a fix, improvement, new feature, refactoring, or other?

other, mostly refactor but it makes SPI changes

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

spi

How would you describe this change to a non-technical end user or system administrator?

n/a

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# SPI
* Deprecate passing constraints to `ConnectorMetadata.getTableStatistics()`. Constraints can be associated with the table handle in `ConnectorMetadata.applyFilter()`. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Apr 8, 2022
@nineinchnick nineinchnick requested a review from findepi April 8, 2022 16:11
Copy link
Member

Choose a reason for hiding this comment

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

My bad, i forgot about this. applyFilter won't make this obsolete, and this is a good idea.

however, the current state is bad: we pass a Constraint and it's useless. So i think it's better to proceed with this PR, and reintroduce Constraint later, if actually going to be used.

@nineinchnick nineinchnick force-pushed the get-table-stats-constraints branch from 1f01381 to bc68076 Compare April 11, 2022 09:57
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

"fixup" lgtm

@findepi findepi force-pushed the get-table-stats-constraints branch from 70ddea7 to 9fcefbf Compare April 13, 2022 08:02
@findepi
Copy link
Member

findepi commented Apr 13, 2022

squashed myself

@findepi
Copy link
Member

findepi commented Apr 15, 2022

Retriggered CI because #11722 has been merged.

@findepi findepi merged commit ee43fc8 into trinodb:master Apr 15, 2022
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Apr 15, 2022
@github-actions github-actions bot added this to the 378 milestone Apr 15, 2022
@nineinchnick nineinchnick deleted the get-table-stats-constraints branch April 18, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

2 participants