jayzhan211 commented on issue #25124:
URL: https://github.com/apache/datafusion/issues/25124#issuecomment-5618663508
Thanks @thinkharderdev, the pain is real, and it's the same one described in
#16622. I support the goal but have concerns about the mechanism. Here's where
I land.
### 1. Why `-api` crates alone won't solve this
A trait is only as stable as the types in its signatures. Following the
closure of `TableProvider::scan`:
- `&dyn Session` → `ConfigOptions`, `RuntimeEnv`, function registry
- `&[Expr]` → UDF types, `Subquery` → `LogicalPlan`
- `Arc<dyn ExecutionPlan>` → `PlanProperties`, `EquivalenceProperties`,
`PhysicalExpr`, `TaskContext`
- plus `Statistics`, `Constraints`, `DataFusionError`, and arrow's
`SchemaRef`
So `datafusion-catalog-api` either depends on `datafusion-expr` /
`datafusion-physical-plan` and breaks whenever they do, or absorbs those types
and changes as often as core does. These are some of our most actively changing
APIs.
Two more limits:
- **Arrow sets a floor.** Arrow types appear in every signature, and
arrow-rs releases a new major version roughly quarterly. Downstream crates like
delta-rs and iceberg-rust also expose arrow types directly, so apps must match
arrow versions with them regardless of what we do.
- **Providers use more than traits.** Many build on our implementation
crates (parquet scan, `FileScanConfig`, pruning), so they'd still depend on
those crates.
A crate split would add crates and publishing overhead without making the
API more stable.
### 2. Proposal: longer breaking-change period (1 month → 2 months, extend
if needed)
- Keep releasing monthly, but only every other release is a major (breaking)
release. The release in between is semver-compatible (`N.0 → N.1 → (N+1).0`),
so downstream crates get a two-month upgrade window instead of a monthly race.
- Enforce it with `cargo-semver-checks` in CI (#16078). Breaking PRs get an
`api-change` label and wait for the next major.
- Prefer "add new + deprecate old" so most changes can still land in minor
releases. That also keeps the queue of held PRs small.
- Re-evaluate after a few cycles. If downstream still can't keep up, extend
to 3 months, which would also line up with arrow's major cadence.
This needs no refactoring, only a release policy plus CI.
### 3. Teach contributors (human and AI agents) how to handle breaking
changes
A lot of PRs are now written with coding agents. Longer breaking periods
only work if both humans and agents default to non-breaking changes. Two
lightweight pieces:
**a) A repo-level `AGENTS.md` section on API changes**, which most agents
read automatically, e.g.:
```markdown
## Public API changes
- Treat every `pub` item in published crates as public API.
- Prefer additive changes: new trait methods with default impls, new
constructors/builders, `#[non_exhaustive]` on new enums/structs.
- Never change an existing signature in place. Add the new API and mark the
old one `#[deprecated(since = "X.Y.0", note = "use `new_fn` instead")]`,
following the API health policy.
- Run `cargo semver-checks` before opening a PR. If a break is unavoidable,
say so in the PR description, add the `api-change` label, and add an
entry to the upgrade guide.
```
**b) Inline markers on key extension points** (`TableProvider`,
`ExecutionPlan`, `Expr`, `Session`, ...), so an agent sees the rule right where
it edits:
```rust
/// ...
///
/// # API stability
/// This trait is a public extension point implemented by downstream crates.
/// Do not change existing method signatures; add new methods with default
/// implementations instead. See AGENTS.md#public-api-changes.
pub trait TableProvider: ...
```
The doc section also helps human contributors and shows up on docs.rs.
`cargo-semver-checks` in CI remains the backstop, since neither humans nor
agents will get it right every time.
--
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]
