Spark (4.0, 3.5): Set data file sort_order_id in manifest for writes from Spark

[jbewing]

This PR updates all writes from Spark 4.0 & 3.5 to set the sort_order_id of data files when applicable per the Iceberg Table Spec. I've opened this PR to:

1. Gauge interest in this and validate my approach
2. If the approach is good, make a decision on whether to backport these changes to Spark 3.4. This patch doesn't apply super cleanly to 3.4 as it stands right now, so I don't want to invest effort into it unless others would find value from it.
3. As a follow up to those PRs, implement the ability to then report this ordering information when applicable to Spark to better inform the query planner of potential optimizations. This is rather useful when used in conjunction with SPJ and I have this running on an Iceberg fork with success

This is a successor to my initial PR #13636 which has since been closed for being stale (but I've revived it here and ported the changes forwards to spark 4.0). I've re-opened this PR as of late there has been some increased interest in this:

• A community member has expressed interest in reviving my older PR Spark 3.5: Set data file sort_order_id in manifest when writing/compacting data files #13636 (comment)
• Other community members have reported this same bug on the issue list To persist sort order in new manifest files after it's been updated #14531

So it appears that there is value to these changes being upstreamed instead of confined to a fork.

Testing

I've added tests for newer added utility functions and updated existing tests that write data files and compact data files in a sorted manner to verify that we're setting the sort_order_id entry in the manifests to the correct value. Additionally, I've used this patch on an internal fork and verified that it correctly sets this field during compaction and normal writes.

Issue: #13634

[jbewing]

I'm not in love with this approach, but I went through a couple of iterations of this and plumbing the intended Iceberg SortOrder along through with the Spark SortOrder ended up being the cleanest in my opinion (open to any suggestions of course). I initially had an approach which would attempt to re-guess the Iceberg SortOrder from a Spark SortOrder. It worked pretty well, but was pretty naive in my opinion—the algorithm was to essentially reverse the Iceberg -> Spark SortOrder transformation as best as possible and then see if things matched up. It gets complicated when you start to consider Spark Orderings for stuff like say a positionDeltaUpdateMergeOrdering.

[RussellSpitzer]

I am willing to help as well :) I just introduced a new child to my family so I have been gone for a few of those months but I will do my best to stay on top of any new developments. Feel free to ping me on slack as well

[jbewing]

I can understand the frustration, but I also understand that the OSS community is volunteering time to review the changes (most of the time outside of their work; I'm sure it is the same for you). Also, some of us just came back from the holiday season. I've also found it useful to reach out on the dev list for reviews. We could also join the community sync meetings to seek reviews if you have time.

Yeah I totally get it. I've just had a slew of interesting experiences in this project, so I want to be certain that if I'm putting in the work that someone else is in it with me 😄 .

I'll discuss this PR and my #14948 that reports this ordering to Spark to the Iceberg Spark community sync next week to gather feedback. I'm happy to help with reviews, but I'm not a committer. We will need a committer to take a look as well.

Sweet yeah as I allude to in the PR description, I also have a patch that does this on a private fork. It works quite well in a narrower set of circumstances—you need to ensure that partitions don't get too skewed or large. But in the set of circumstances that things are roughly even, the performance improvement is drastic. I'd be happy to take a look at your implementation and give feedback/lessons learned from ours.

I am willing to help as well :) I just introduced a new child to my family so I have been gone for a few of those months but I will do my best to stay on top of any new developments. Feel free to ping me on slack as well

Sounds good! Totally understand—life happens. I just wanted to make sure before putting a bunch of effort in that it would be matched this time. I look forwards to working together!

[jbewing]

Alright I've rebased to the latest master. I can split out the changes if you'd like @RussellSpitzer. It seems like 4.1 is state of the art now, so would you like a PR that:

• Is opened against 4.1
• Backport all clean branches in a follow up (I plan on taking it back to 4.0 & 3.5 as I recall the 3.4 backport being ugly)

Or would you like it if we kept the current PR and ported forward to 4.1 as it seems like there hasn't been an official iceberg release for Spark 4.1. I'm assuming both are fine as I think the 4.1 setup must be more recent so stuff will probably forward/backport super cleanly.

[anuragmantri]

Hi @jbewing - We discussed this PR in the community, and I would like to help move these forward with the help of other reviewers.

It seems like something went wrong with the rebase. Could you please rebase your change and make a single PR with just 4.1? I will do the same with my PR.

Community prefers that way as it's easier to review. We can have a subsequent PR that back ports to all older branches (in one PR). Usually that will not take much time in reviews.

[RussellSpitzer]

One single PR against 4.1 would be ideal IMHO. Unless you have multiple independent bits you want to get in one at a time.

[jbewing]

Hey @anuragmantri & @RussellSpitzer , will do. I thought I was going to be able to get to it this weekend but wasn't able to find the time, but should have some time in the next couple days. Just wanted to give an update to let you know that I had seen this and am getting to it (just not as quickly as I would've liked to)

[jbewing]

@RussellSpitzer & @anuragmantri, I've opened up #15150. We can move conversation there as I'm closing this PR in favor of that one.