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
Contributor Author

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
Contributor Author

@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
Contributor Author

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

alamb commented Jun 24, 2026

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-actions Bot 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-actions github-actions Bot 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
…loses apache#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. Closes apache#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 Dodothereal force-pushed the chore/remove-deprecated-add-row-stats branch from ab9dd4e to e1325b7 Compare June 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.

Closes apache#23080 (partial — first in this batch to come from the 47.0.0-deprecated set).
@Dodothereal Dodothereal force-pushed the chore/remove-deprecated-add-row-stats branch from e1325b7 to ab1b91e Compare June 26, 2026 20:45
@Dodothereal

Copy link
Copy Markdown
Contributor Author

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

alamb commented Jun 27, 2026

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 queue Jun 27, 2026
@github-merge-queue github-merge-queue Bot 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 queue Jun 29, 2026
Merged via the queue into apache:main with commit 0165628 Jun 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api change Auto detected API change datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove old deprecated code

2 participants