Fork DataFusion SMJ in Comet repo

[andygrove]

What is the problem the feature request solves?

I would like to suggest that we fork the SMJ implementation from DataFusion and include it in the Comet repo.

I think this would let us customize/optimize for Spark use case and move faster without having to wait for DF release cycles. It also protects us from performance regressions when we don't have the time to test DF major releases until after they are released, which is often the case because we have to wait for iceberg-rust to upgrade first.

Describe the potential solution

No response

Additional context

No response

[andygrove]

@comphead @parthchandra @mbutrovich WDYT?

[mbutrovich]

I'm still okay with the current cadence, even as I sit waiting for reviews on apache/datafusion#20806.

I'm not opposed to start laying the groundwork to accomplish that, though. For example, I think we'd need SpillManager changed in DataFusion to pub. When I tried to bring just apache/datafusion#20806 to Comet to test, I think that was the standout limitation.

That way if we had a strong need to bring the operator here (which I'm not seeing a huge justification for in this issue yet) we could do it temporarily while we wait to land the change in DataFusion (like Spark expressions).

[comphead]

it would be great idea to "vendorize" to Comet some part of DF to Comet for the sake of improving them and not depending on DF release cadence. Having improved then port it back to DataFusion. As an alternative we can also have DataFusion release fork where we can modify DF parts and point Comet to that fork instead of crates.io.

Agree with Matt to port to Comet, we need to modify some DF parts to be accessible from Comet along with we need to plan efforts to backport the changes and also address potential feedback. However such approach was widely used by Synnada

[parthchandra]

I'm neutral on this. I feel that DF has pretty frequent releases. The perf regressions are a bit of a concern but I feel we could address them by contributing some benchmarking tests that could reflect the Comet use cases and be included in DF ci (haven't thought this thru though so it might be tricky to include cases that are inherently distributed).

[rluvaton]

outsider point of view: I think that it is ok to fork that but only as a temporary solution until merged
as there are a lot of people working on data fusion and optimizing it + finding/fixing bugs, that forking that code will make it less stable, and in the long run less efficient.

[2010YOUY01]

Sharing a perspective from a DataFusion contributor: Forking SMJ, iterating on it within Comet for your specific use cases, and periodically syncing the accumulated improvements back upstream could be a good idea.

This can reduce review pressure on DataFusion. Small incremental PRs often lack sufficient context about future directions, and may end up being revised multiple times. In contrast, accumulating changes downstream allows them to be validated in real workloads first, so when proposing them upstream, we could have stronger confidence.

It also makes reviews easier. Instead of sporadic changes that reviewers may lose context on, related improvements can be grouped and reviewed together. Even if the total diff is large, it can still be upstreamed as a series of smaller PRs.