limit_push_down is not working properly with OFFSET

[ming535]

Describe the bug
limit_push_down tries to push a limit to TableScan when OFFSET is without LIMIT, which makes all the result disappeared.
OFFSET without LIMIT is allowed in Postgres.

To Reproduce
in limit_push_down.rs, add this test code:
`#[test]
fn limit_pushdown_should_not_pushdown_limit_with_offset_only() -> Result<()> {

    let table_scan = test_table_scan()?;
    let plan = LogicalPlanBuilder::from(table_scan)
        .offset(10)?
        .build()?;

    // Should not push any limit down to table provider
    // When it has a select
    let expected = "Offset: 10\
    \n  TableScan: test projection=None";

    assert_optimized_plan_eq(&plan, expected);

    Ok(())
}

`

The problem is here in the code, which adds a limit when OFFSET doesnt have one:

Expected behavior
The test case should pass. When OFFSET is without LIMIT, then no limit should pushed down to TableScan.

Additional context
I was trying to implement the OFFSET physical plan.

[ming535]

Any idea on how to handle this? I was thinking adding another boolean attribute has_limit inside LogicalPlan::OffsetPlan, and handle the case when there is no limit in limit_push_down.

[ming535]

How about add another LogicalPlan LimitWithOffset. So that the three LogicalPlan used in different cases:

1. Limit: without offset
2. Offset: without limit
3. LimitWithOffset: offset + limit

Then in the limit_push_down, we need to handle two cases: Limit and LimitWithOffset.
For Limit, the push down logic is roughly the same as before; for LimitWithOffset, we adjust the limit pushed down.

[alamb]

What about consolidating Limit and Offset into LimitWithOffset (both logical and physical plans), with optional limit or offset:

  limit: Option<usize>,
  offset: Option<usize>

Then when a limit / offset is pushed down, it can be transformed from Limit(limit, offset) to Limit(limit+offset, 0) ?