Short circuit page source creation in Iceberg#11976
Conversation
There was a problem hiding this comment.
Instead of this, can you try taking similar approach as TestIcebergSplitSource to create IcebergTableHandle ?
There was a problem hiding this comment.
Thanks for the suggestion. Indeed the creation of the table schema was much too verbose. I opted for using the iceberg Schema object for this purpose.
Adding the overhead of using a query runner just for retrieving the table schema seems not that good to me because the maintainer of the code might think that this is an integration test when it is a simple unit test.
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNodeLocalDynamicSplitPruning.java
Outdated
Show resolved
Hide resolved
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNodeLocalDynamicSplitPruning.java
Outdated
Show resolved
Hide resolved
findinpath
force-pushed
the
short-circuit-pruned-iceberg-page
branch
from
266592f to
a583b61
Compare
There was a problem hiding this comment.
Instead of assertion on page source class, please assert on the output page which comes from calling ConnectorPageSource#getNextPage.
findinpath
force-pushed
the
short-circuit-pruned-iceberg-page
branch
from
a583b61 to
3ebc62e
Compare
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNodeLocalDynamicSplitPruning.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
There's no need for this to be a partitioned column, we will be able to prune splits even with DF on a data column
There was a problem hiding this comment.
removed all references to "partition" which may be indeed misleading for the functionality tested.
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNodeLocalDynamicSplitPruning.java
Outdated
Show resolved
Hide resolved
findinpath
force-pushed
the
short-circuit-pruned-iceberg-page
branch
from
3ebc62e to
d0bef4a
Compare
In case that the dynamic filter completes after scheduling of split on the worker, the results in the split will be getting pruned.
findinpath
force-pushed
the
short-circuit-pruned-iceberg-page
branch
from
d0bef4a to
a3bb77d
Compare
|
@findinpath @raunaqmorarka please formulate a RN entry for this, i..e. what are the triggering conditions for the bug this is fixing. |
|
RN updated. |
|
Thanks @findinpath @raunaqmorarka |
3 participants
Description
In case that the dynamic filter completes after scheduling of split
on the worker the results in the split will be getting pruned.
Fix
Iceberg connector
See https://trinodb.slack.com/archives/CJ6UC075E/p1649977257483049 Slack discussion
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: