findepi commented on code in PR #13028:
URL: https://github.com/apache/datafusion/pull/13028#discussion_r1812061636


##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -2799,14 +2817,71 @@ impl PartialOrd for Extension {
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
 pub struct Limit {
     /// Number of rows to skip before fetch
-    pub skip: usize,
+    pub skip: Option<Box<Expr>>,
     /// Maximum number of rows to fetch,
     /// None means fetching all rows
-    pub fetch: Option<usize>,
+    pub fetch: Option<Box<Expr>>,
     /// The logical plan
     pub input: Arc<LogicalPlan>,
 }
 
+/// Different types of skip expression in Limit plan.
+pub enum SkipType {
+    /// The skip expression is a literal value.
+    Literal(usize),
+    /// Currently only supports expressions that can be folded into constants.
+    UnsupportedExpr,

Review Comment:
   @jonahgao good point. I also thought about this, but ignored, assuming we 
don't plan to support this. 
   
   We could have `Limit` node as-is _for now_ (and do trivial constant folding 
when building the plan), and introduce expressions in the plan when we add 
support for `limit (<uncorrelated subquery>)`. WDYT?



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

Reply via email to