You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
// Removed in #905:fnskip(&self) -> usize{self.limit.skip}fnfetch(&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:
// 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
fromdatafusionimportSessionContextfromdatafusion.exprimportLimitctx=SessionContext()
ctx.sql("CREATE TABLE t (a INT)")
df=ctx.sql("SELECT * FROM t LIMIT 10 OFFSET 5")
plan=df.logical_plan().to_variant()
assertisinstance(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.
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.
Describe the bug
PyLimit, the Python wrapper for aLogicalPlan::Limitnode (crates/core/src/expr/limit.rs), exposes no method to read the actualLIMIT/OFFSETvalue. Onlyinput(),schema(), and__repr__()are defined. As a result, any Python code walking a logical plan (e.g. a custom SQL compiler/backend built ondatafusion-python) cannot determine whatLIMIT/OFFSETa query actually specified — even for a plain integer literal likeLIMIT 10.This is a regression from #905 ("Upgrade to Datafusion 43"), which removed the old
skip()/fetch()methods:because upstream DataFusion changed
Limit.skip/Limit.fetchfromusize/Option<usize>toOption<Box<Expr>>in apache/datafusion#13028 (not #12836 — theTODOcomment currently inlimit.rscites the wrong PR number; #12836 is an unrelatedunnestPR). The old methods no longer compiled against the new field types, so they were deleted outright rather than updated, with aTODOleft in their place:Note the value is still present and correctly parsed internally —
Display for PyLimitprints it fine (Skip: {:?},Fetch: {:?}viaself.limit.skip/self.limit.fetch) — it's only inaccessible as structured data from Python. The only current workaround is regex-parsing therepr()/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
Expected behavior
PyLimitshould exposeskip()/fetch()methods returningOption<PyExpr>(matching the newOption<Box<Expr>>field types), so callers can read the value out — e.g. viaExpr.python_value()for the common literal case, mirroring howPyProjection::projections()andPyTableScan::py_filters()already wrapExpr/Vec<Expr>fields asPyExpr.Additional context
LIMITplan datafusion#13028 (not #12836, which the currentTODOcomment incorrectly cites)LIMIT/OFFSETexpressions (e.g.LIMIT $1, computed expressions) are exposed as-is via the rawPyExpr— this fix doesn't attempt to simplify/fold them, consistent with upstream's own approach of relying onSimplifyExpressionsbefore physical planning and erroring if it can't fold to a constant.