Skip Iceberg row filter when not needed#11851
Conversation
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergPageSourceProvider.java
Outdated
Show resolved
Hide resolved
alexjo2144
force-pushed
the
iceberg/row-level-deletes-read
branch
from
60e3363 to
7773c9d
Compare
|
Updated to use Optional.filter. Thanks for reviewing |
|
@electrum can we merge this when you get a chance? |
| dataPageSource.get(), | ||
| projectionsAdapter, | ||
| deleteFilter); | ||
| Optional.of(deleteFilter).filter(filter -> filter.hasPosDeletes() || filter.hasEqDeletes())); |
There was a problem hiding this comment.
The data Page is always rewritten in the IcebergPageSource right now, this makes it conditional on if there are actually any deletes to account for.
There was a problem hiding this comment.
sorry for not being clear. I understand what the change is for.
wonder why Optional#filter is needed here.
if "deleteFilter" is not needed, why was it passed until here? did the split source attach some useless information to the split?
There was a problem hiding this comment.
Oh, I see. We could also make this conditional on split.getDeletes().isEmpty() if that's clearer? They're equivalent
There was a problem hiding this comment.
why was it passed until here?
The DeleteFilter is constructed on the line right above this, the only thing passed around until here are some empty lists. split.getDeletes() should be empty, as should deleteFilterRequiredSchema
findepi
added
the
no-release-notes
4 participants
Description
Avoid recreating Iceberg Pages for the DeleteFilter when not needed.
Improvement/Fix
Iceberg Connector
Small performance improvement
Related issues, pull requests, and links
#11642
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
(x) No release notes entries required.
( ) Release notes entries required with the following suggested text: