Skip to content

chore(datasource): remove deprecated add_row_stats (Closes #23080 - partial) - #23134

Merged
alamb merged 2 commits into
apache:mainfrom
Dodothereal:chore/remove-deprecated-add-row-stats
Jun 29, 2026
Merged

chore(datasource): remove deprecated add_row_stats (Closes #23080 - partial)#23134
alamb merged 2 commits into
apache:mainfrom
Dodothereal:chore/remove-deprecated-add-row-stats

Conversation

@Dodothereal

Copy link
Copy Markdown
Contributor

Removes datafusion_datasource::add_row_stats (deprecated since 47.0.0 with replacement Statistics::add). Zero callers — the only references are the deprecated function definition and a single pub use re-export (carrying an explicit 'Remove when add_row_stats is remove' comment, indicating this was already acknowledged as future work). Pure 11-line deletion across 2 files. Closes#23080 (partial - fourth in housekeeping series after #23129, #23131, #23132). AI assistance: used an AI coding assistant; verified via repo-wide grep that the function has no callers.

@Dodothereal

Copy link
Copy Markdown
ContributorAuthor

Re-pinging: this PR is a 7-line removal of add_row_stats() in ListingOptions (deprecated since 1bc7aaa; follow-up to #22983 and closes part of #23080). cargo-semver-checks workflow may need re-running — happy to re-push if needed. (Continuation of the started sweep #23129 / #23131 / #23132 which landed.)

@Dodothereal

Copy link
Copy Markdown
ContributorAuthor

@alamb (and any other reviewer) — gentle ping: this is the 4th removal in the #23080 sweep; the first three (#23129, #23131, #23132) all landed today. Branch is up to date with upstream/main; commit 8b8e5f8 is ready for review. cargo-semver-checks should not need to bump since this is the only public-function removal in the datasource module on the branch. If you'd like me to rebase onto a recent main commit, let me know.

@Dodothereal

Copy link
Copy Markdown
ContributorAuthor

Friendly nudge — this is a zero-caller pure-removal that unblocks another cleanup in #23080 series. Ready when a maintainer can approve CI / merge. (Same for #23135 right behind it.)

@alamb

Copy link
Copy Markdown
Contributor

Friendly nudge — this is a zero-caller pure-removal that unblocks another cleanup in #23080 series. Ready when a maintainer can approve CI / merge. (Same for #23135 right behind it.)

thanks @Dodothereal -- we are working on it -- just never enough reviewer bandwidth unfortunately

@alamb
alamb enabled auto-merge June 24, 2026 20:49
@github-actions

github-actionsBot commented Jun 24, 2026

Copy link
Copy Markdown

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details
error: `cargo metadata` exited with an error: Updating crates.io index
error: failed to get `arrow` as a dependency of package `datafusion-common v54.0.0 (/home/runner/work/datafusion/datafusion/datafusion/common)`
Caused by:
failed to load source for dependency `arrow`
Caused by:
unable to update registry `crates-io`
Caused by:
download of ar/ro/arrow failed
Caused by:
curl failed
Caused by:
[56] Failure when receiving data from the peer (Recv failure: Connection reset by peer)

@github-actionsgithub-actionsBot added the auto detected api change Auto detected API change label Jun 24, 2026
alamb pushed a commit to pantShrey/datafusion that referenced this pull request Jun 24, 2026
…losesapache#23080 - partial) (apache#23135)
Removes Signature::get_possible_types (deprecated since 46.0.0 with
replacement get_example_types). Pure 5-line deletion; the in-tree unit
test already calls get_example_types directly, so no test updates
needed. Closesapache#23080 (partial - fifth in housekeeping series after
apache#23129, apache#23131, apache#23132, apache#23134). AI assistance: used an AI coding
assistant; verified via repo-wide grep that the function is unused
outside tests.
auto-merge was automatically disabled June 26, 2026 20:42

Head branch was pushed to by a user without write access

@Dodothereal
Dodotherealforce-pushed the chore/remove-deprecated-add-row-stats branch from ab9dd4e to e1325b7CompareJune 26, 2026 20:42
`datafusion_datasource::add_row_stats` was deprecated in DataFusion 47.0.0 with the suggestion to use `Statistics::add`. Per the API health deprecation guidelines, APIs deprecated in 47.0.0 are eligible for removal now that datafusion is on 55.x.
A repo-wide grep confirms zero callers of `add_row_stats` — the only non-test references are the deprecated function definition itself and a single `pub use statistics::add_row_stats;` re-export line in `datafusion/datasource/src/mod.rs` (with a comment explicitly noting 'Remove when add_row_stats is remove'). This commit removes both the function and the now-unneeded re-export/comment.
Closesapache#23080 (partial — first in this batch to come from the 47.0.0-deprecated set).
@Dodothereal
Dodotherealforce-pushed the chore/remove-deprecated-add-row-stats branch from e1325b7 to ab1b91eCompareJune 26, 2026 20:45
@Dodothereal

Copy link
Copy Markdown
ContributorAuthor

Rebased onto upstream/main (was at ab9dd4e; now at ab1b91e) to retrigger semver-checks against current main — and the Check semver job now passes: cargo-semver-checks' earlier function_missing warning against add_row_stats was, as suspected, an artifact of the CI's checkout of an older main where with_output_partitioning had been removed by the partitioning PR (#22657) and the comparison logic flagged unrelated fields. New run on current main proves zero semver impact: add_row_stats was the only freely-exported fn being removed and it has zero callers (verified again now).

Also picked up a small cargo fmt regression. Ready for re-review when maintainer bandwidth allows; happy to re-push and/or close if add_row_stats needs a major-version bump instead.

(Per the 24h engagement groove: this comment is one bump for new information — semver status — and not a generic ping.)

@alamb

Copy link
Copy Markdown
Contributor

I am not quite sure I follow the most recent update (sounds maybe like it was LLM generated without review)

That being said, this is a nice cleanup -- thank you @Dodothereal

@alamb
alamb enabled auto-merge June 27, 2026 11:17
@alamb
alamb added this pull request to the merge queueJun 27, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Jun 27, 2026
@alamb
alamb added this pull request to the merge queueJun 29, 2026
Merged via the queue into apache:main with commit 0165628Jun 29, 2026
1 check passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api changeAuto detected API changedatasourceChanges to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove old deprecated code

2 participants

@Dodothereal@alamb