trino
trino copied to clipboard
Support partitioning on nested ROW fields in Iceberg
Description
Fixes https://github.com/trinodb/trino/issues/15109 Support partitioning on nested ROW fields in Iceberg
Release notes
( ) This is not user-visible or docs only and no release notes are required. ( ) Release notes are required, please propose a release note for me. (X) Release notes are required, with the following suggested text:
# Iceberg
* Add support for partitioning on nested ROW fields. ({issue}`15712`)
@krvikash Could you rebase on upstream to resolve conflicts?
Rebased the PR with latest code.
Hi, @alexjo2144 | @ebyhr | @findepi | @findepi, when you get time could you please review this?
Rebased the PR with latest code.
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
@krvikash @alexjo2144 @findinpath I see no approvals and some yet unresolved conversations. Where are we with the PR?
I believe this PR is waiting for #14837 (https://github.com/trinodb/trino/pull/15712#discussion_r1095417177)
Rebased on top of https://github.com/trinodb/trino/pull/14837 's commit and resolved conflicts. This PR (2nd commit of this PR) is ready for review now.
Fixed CI failure.
Fixed product test (not able to verify on M1, But hopefully it will be fixed).
Rebased and addressed comments.
Thanks, @raunaqmorarka | @findepi for reviewing. Addressed comments and rebased with master.
Rebased with master. CI was failing with https://github.com/trinodb/trino/issues/16406
Rebased with master and resolved conflicts.
Rebased with master.
Is this still being pursued? It would be a great help for Trino's compatibility with Iceberg.
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua
:wave: @krvikash @findepi @findinpath @bitsondatadev - this PR has become inactive. We hope you are still interested in working on it. Please let us know, and we can try to get reviewers to help with that.
We're working on closing out old and inactive PRs, so if you're too busy or this has too many merge conflicts to be worth picking back up, we'll be making another pass to close it out in a few weeks.
@mosabua Please keep this PR opened.
@krvikash Could you rebase on master to resolve conflicts?
@ebyhr (rebased with master)
(fixed CI failure)
Rebased on master to resolve conflicts. Talked with Vikash and I'm going to take over this PR.
that's a good question. I am not sure when it's safe to use block.getChildren() vs ColumnarRow I remember @djsstarburst fixing some correctness(?) issues in MERGE by switching from the former to the latter.
IIRC, the reason to use ColumnarRow is that it correctly accounts for the case of blocks containing blocks, where some of the inner block children that are null. @dain is the expert.
Using ColumnarRow in the bucket function probably requires https://github.com/trinodb/trino/issues/9848 for any reasonable performance.
(some refactoring in TestIcebergV2
)
Rebased on master to resolve conflicts.
Pls rebase the code to resolve conflicts.
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua