Refactor TableStatsCollectorUtil

[srawat98-dev]

Refactors TableStatsCollectorUtil by extracting reusable helper methods from the populateCommitEventTablePartitions implementation. This improves code organization, testability, and enables future code reuse without changing any functionality.

Summary

This is a pure refactoring PR that extracts well-designed, reusable helper methods from inline code in
populateCommitEventTablePartitions. The goal is to:

• Improve code organization and readability
• Create reusable building blocks for future features
• Reduce code duplication
• No functional changes - behavior remains identical.

Changes

• Client-facing API Changes
• Internal API Changes
• Bug Fixes
• New Features
• Performance Improvements
• Code Style
• Refactoring
• Documentation
• Tests

For all the boxes checked, please include additional details of the changes made in this pull request.

Testing Done

• Manually Tested on local docker setup. Please include commands ran, and their output.
• Added new tests for the changes made.
• Updated existing tests to reflect the changes made.
• No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
• Some other form of testing like staging or soak time in production. Please explain.

For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.

Additional Information

• Breaking Changes
• Deprecations
• Large PR broken into smaller PRs, and PR plan linked in the description.

For all the boxes checked, include additional details of the changes made in this pull request.

[cbb330]

The refactor introduces a subtle issue with the caching logic.

In the original code, partitionsPerCommitDF stays cached until after collectAsList() executes the join. In the new code, buildEnrichedPartitionDataFrame calls unpersist() on partitionsPerCommitDF before returning enrichedDF - but since Spark is lazy, the join hasn't executed yet at that point. When enrichedDF.count() later triggers the join, the cached data may already be gone and Spark has to recompute it.

Also, there's now two count() calls where there used to be one - the helper counts partitionsPerCommitDF for the early return check, then the caller counts enrichedDF again for logging/cache materialization. That's an extra Spark action that wasn't there before, which goes against the "no functional changes" goal.

For a cleaner refactor, I'd suggest having the helper just build and return the DataFrame without managing caching or counting. Let the caller handle the lifecycle:

static Dataset<Row> buildEnrichedPartitionDataFrame(Table table, SparkSession spark) {
    if (table.spec().isUnpartitioned()) {
        return null;
    }
    // just build and return - no caching, no counting
    return partitionsPerCommitDF.join(snapshotsDF, "snapshot_id").select(...);
}

Then the caller can cache, collect, and unpersist as needed. This keeps the helper simple and reusable without baking in behavior that future callers might not want.

[abhisheknath2011]

The refactor introduces a subtle issue with the caching logic.

In the original code, partitionsPerCommitDF stays cached until after collectAsList() executes the join. In the new code, buildEnrichedPartitionDataFrame calls unpersist() on partitionsPerCommitDF before returning enrichedDF - but since Spark is lazy, the join hasn't executed yet at that point. When enrichedDF.count() later triggers the join, the cached data may already be gone and Spark has to recompute it.

Also, there's now two count() calls where there used to be one - the helper counts partitionsPerCommitDF for the early return check, then the caller counts enrichedDF again for logging/cache materialization. That's an extra Spark action that wasn't there before, which goes against the "no functional changes" goal.

For a cleaner refactor, I'd suggest having the helper just build and return the DataFrame without managing caching or counting. Let the caller handle the lifecycle:

static Dataset<Row> buildEnrichedPartitionDataFrame(Table table, SparkSession spark) {
    if (table.spec().isUnpartitioned()) {
        return null;
    }
    // just build and return - no caching, no counting
    return partitionsPerCommitDF.join(snapshotsDF, "snapshot_id").select(...);
}

Then the caller can cache, collect, and unpersist as needed. This keeps the helper simple and reusable without baking in behavior that future callers might not want.

Looks like Christian's comment is already addressed. Thanks.

[abhisheknath2011]

Thanks for extracting the refactored code in this PR.