Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove redundant TupleDomain conversions #11077

Closed
wants to merge 1 commit into from
Closed

Remove redundant TupleDomain conversions #11077

wants to merge 1 commit into from

Conversation

findepi
Copy link
Member

@findepi findepi commented Feb 17, 2022

Remove redundant TupleDomain.transformKeys invocations used to change
TupleDomain type only.

@findepi
Remove redundant TupleDomain conversions
251154d 
Remove redundant `TupleDomain.transformKeys` invocations used to change
`TupleDomain` type only.
@cla-bot cla-bot bot added the cla-signed label Feb 17, 2022
@losipiuk
Copy link
Member

CI

@findepi
Copy link
Member Author

findepi commented Feb 17, 2022

CI failed because i missed an update in new ConnectorTableProperties.
this can be changed by relaxing TupleDomain<ColumnHandle> to TupleDomain<? extends ColumnHandle> in the constructor.

  • we then either agree to use such less pretty type in other places in the code
  • or we "upcast" it to TupleDomain<ColumnHandle>, which is a safe operations, but java type system doesn't know that (see private static <U, T extends U> TupleDomain<U> upcast(TupleDomain<T> domain) in Flexible TupleDomain.intersect type #11078)

idk whether it matters enough to have this discussion, so let me park this for now

@findepi findepi closed this Feb 17, 2022
@findepi findepi deleted the findepi/remove-redundant-tupledomain-conversions-65e7db branch February 17, 2022 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@losipiuk losipiuk losipiuk approved these changes

@raunaqmorarka raunaqmorarka raunaqmorarka approved these changes

Assignees
No one assigned
Labels
cla-signed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@findepi @losipiuk @raunaqmorarka