hakunamatata-sb opened a new issue, #1673:
URL: https://github.com/apache/datafusion-python/issues/1673
**Describe the bug**
`PyLimit`, the Python wrapper for a `LogicalPlan::Limit` node
(`crates/core/src/expr/limit.rs`), exposes no method to read the actual
`LIMIT`/`OFFSET` value. Only `input()`, `schema()`, and `__repr__()` are
defined. As a result, any Python code walking a logical plan (e.g. a custom SQL
compiler/backend built on `datafusion-python`) cannot determine what
`LIMIT`/`OFFSET` a query actually specified — even for a plain integer literal
like `LIMIT 10`.
This is a regression from #905 ("Upgrade to Datafusion 43"), which removed
the old `skip()`/`fetch()` methods:
```rust
// Removed in #905:
fn skip(&self) -> usize { self.limit.skip }
fn fetch(&self) -> Option<usize> { self.limit.fetch }
```
because upstream DataFusion changed `Limit.skip`/`Limit.fetch` from
`usize`/`Option<usize>` to `Option<Box<Expr>>` in apache/datafusion#13028
(**not** #12836 — the `TODO` comment currently in `limit.rs` cites the wrong PR
number; #12836 is an unrelated `unnest` PR). The old methods no longer compiled
against the new field types, so they were deleted outright rather than updated,
with a `TODO` left in their place:
```rust
// NOTE: Upstream now has expressions for skip and fetch
// TODO: Do we still want to expose these?
// REF: https://github.com/apache/datafusion/pull/12836
```
Note the value **is** still present and correctly parsed internally —
`Display for PyLimit` prints it fine (`Skip: {:?}`, `Fetch: {:?}` via
`self.limit.skip`/`self.limit.fetch`) — it's only inaccessible as structured
data from Python. The only current workaround is regex-parsing the
`repr()`/`str()` output, which is fragile.
I couldn't find any existing open issue or PR tracking this gap; searching
for "PyLimit", "Limit skip fetch", etc. in this repo only surfaces #905 itself.
**To Reproduce**
```python
from datafusion import SessionContext
from datafusion.expr import Limit
ctx = SessionContext()
ctx.sql("CREATE TABLE t (a INT)")
df = ctx.sql("SELECT * FROM t LIMIT 10 OFFSET 5")
plan = df.logical_plan().to_variant()
assert isinstance(plan, Limit)
print(plan.fetch()) # AttributeError: 'datafusion.expr.Limit' object has no
attribute 'fetch'
print(plan.skip()) # AttributeError: 'datafusion.expr.Limit' object has no
attribute 'skip'
```
**Expected behavior**
`PyLimit` should expose `skip()`/`fetch()` methods returning
`Option<PyExpr>` (matching the new `Option<Box<Expr>>` field types), so callers
can read the value out — e.g. via `Expr.python_value()` for the common literal
case, mirroring how `PyProjection::projections()` and
`PyTableScan::py_filters()` already wrap `Expr`/`Vec<Expr>` fields as `PyExpr`.
Confirmed working end-to-end (built via `maturin develop`, ran against
`python/tests/test_expr.py::test_limit` updated with real assertions instead of
`repr()` string-matching):
```python
df = ctx.sql("select c1 from test LIMIT 10")
plan = df.logical_plan().to_variant()
assert plan.skip() is None
assert plan.fetch().python_value().as_py() == 10
df = ctx.sql("select c1 from test LIMIT 10 OFFSET 5")
plan = df.logical_plan().to_variant()
assert plan.skip().python_value().as_py() == 5
assert plan.fetch().python_value().as_py() == 10
```
Full `test_expr.py` suite (176 tests) passes with this change, no
regressions.
**Additional context**
- Regression introduced in: #905
- Actual upstream root cause: apache/datafusion#13028 (not #12836, which the
current `TODO` comment incorrectly cites)
- Non-literal `LIMIT`/`OFFSET` expressions (e.g. `LIMIT $1`, computed
expressions) are exposed as-is via the raw `PyExpr` — this fix doesn't attempt
to simplify/fold them, consistent with upstream's own approach of relying on
`SimplifyExpressions` before physical planning and erroring if it can't fold to
a constant.
--
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]
