laskoviymishka commented on PR #928: URL: https://github.com/apache/iceberg-go/pull/928#issuecomment-4295707181
Agree the table package has grown unwieldy and a split makes sense — but I'd rather do it on transaction boundaries than on concurrency-validation alone, since the concurrency code depends on Metadata / Snapshot / AncestorsBetween and can't cleanly live under table/conflict/ without pulling half of table/ with it. A tx-boundary split would also align us with iceberg-rust's transaction module (Java's package structure is flat, but we don't have to follow that). If we go this route, it's worth a more structured effort — probably extracting more than just transaction (eval, spec, operations), which deserves its own design. One **big open** design question up front: do we ship this as a breaking change, or keep type aliases in `table/*` so the public API surface stays the same while the implementation moves into sub-packages? I'd like to lock that in before we start moving lines. For this PR, I've addressed the non-packaging feedback (dataFiles helper / evaluator duplication notes below) and minimized the public API surface so moving things later is cheap — none of the conflict validation symbols are consumed outside this PR yet (no producer wires them in until the next #830 slice), so we're free to relocate them in a follow-up with zero break. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
