Skip to content

PyLimit (LogicalPlan Limit node) has no way to read skip/fetch — regressed in #905 #1673

Description

@hakunamatata-sb

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:
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:

// 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

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.

Additional context

  • Regression introduced in: Upgrade to Datafusion 43 #905
  • Actual upstream root cause: feat: support arbitrary expressions in LIMIT plan 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions