fix(scan): pre-check filter pushability before pushing limit/offset (#192)

jiaoew1991 · claude · web-flow · commit 1198bf8d1edd · 2026-04-16T14:21:12.000+08:00
When DuckDB pushes both LIMIT and filters to the Lance table function but
Lance's IR encoder cannot encode all filters (e.g., LIKE with interior
wildcards, unsupported column types, 'infinity' literals, complex
expressions), two distinct failure modes occurred:

  1. "IO Error: Lance limit/offset pushdown requires filter pushdown" —
     thrown from LanceScanInitGlobal, failing the query entirely.
  2. Silent correctness bug: if the throw path wasn't triggered, Lance
     applied LIMIT without the filter, returning wrong rows.

Fix: add a shared `ProbeLanceTableFilterIR(get, names, types)` helper in
lance_filter_ir.cpp that owns the "are these filters fully encodable?"
invariant in one place. Both optimizer passes that depend on this
invariant use it:

  - LanceLimitOffsetPushdown: if probe returns !all_filters_pushed, keep
    LogicalLimit in the plan and skip limit pushdown. DuckDB then applies
    both filter and LIMIT on top of the scan output — correct semantics,
    no throw.
  - LanceExecPushdown: unchanged behavior (still bails if not pushable),
    but now uses the shared helper instead of duplicating the probe setup.

Having both passes call the same helper eliminates the risk of
optimizer-layer divergence. The defensive throw at LanceScanInitGlobal
is retained as an invariant guard — unreachable from either optimizer
pass, but kept in case future code paths construct a scan with pushed
LIMIT and non-encodable filters.

Test: new test/sql/pushdown_limit_filter_fallback.test covers:
  - LIMIT/OFFSET with a non-encodable filter (returns correct row counts)
  - Semantically-correct row identity (not just count)
  - Selective filter (ensures filter isn't silently dropped)
  - Sparse projection (probe column_ids layout doesn't diverge from init)
  - Pushable filters still push down as before (no regression)

Co-authored-by: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/include/lance_filter_ir.hpp b/src/include/lance_filter_ir.hpp
@@ -20,6 +20,19 @@ LanceFilterIRBuildResult BuildLanceTableFilterIRParts(
     const vector<string> &names, const vector<LogicalType> &types,
     const TableFunctionInitInput &input, bool exclude_computed_columns);
 
+// Probe whether every filter attached to `get.table_filters` can be encoded
+// into Lance filter IR. Owns the invariant "filters encodable → limit/offset
+// pushdown is safe" used by both LanceLimitOffsetPushdown and
+// LanceExecPushdown, so those passes cannot diverge and fall out of sync
+// with the runtime scan init.
+//
+// Returns `all_filters_pushed=true` with empty `parts` if the filter set
+// is empty. Callers that need the encoded parts (e.g., to build an exec
+// IR message) can consume `parts` directly.
+LanceFilterIRBuildResult
+ProbeLanceTableFilterIR(LogicalGet &get, const vector<string> &names,
+                        const vector<LogicalType> &types);
+
 bool TryBuildLanceExprFilterIR(const LogicalGet &get,
                                const vector<string> &names,
                                const vector<LogicalType> &types,
diff --git a/src/lance_filter_ir.cpp b/src/lance_filter_ir.cpp
@@ -424,6 +424,35 @@ static bool TryBuildLanceTableFilterIRExpr(const string &col_ref_ir,
   }
 }
 
+LanceFilterIRBuildResult
+ProbeLanceTableFilterIR(LogicalGet &get, const vector<string> &names,
+                        const vector<LogicalType> &types) {
+  LanceFilterIRBuildResult empty_result;
+  if (get.table_filters.filters.empty()) {
+    return empty_result;
+  }
+  idx_t max_col_id = 0;
+  for (auto &it : get.table_filters.filters) {
+    auto col_id = NumericCast<idx_t>(it.first);
+    if (col_id >= names.size() || col_id >= types.size()) {
+      LanceFilterIRBuildResult oob_result;
+      oob_result.all_filters_pushed = false;
+      oob_result.all_prefilterable_filters_pushed = false;
+      return oob_result;
+    }
+    max_col_id = MaxValue(max_col_id, col_id);
+  }
+  vector<column_t> column_ids;
+  column_ids.reserve(max_col_id + 1);
+  for (idx_t i = 0; i <= max_col_id; i++) {
+    column_ids.push_back(NumericCast<column_t>(i));
+  }
+  vector<idx_t> projection_ids;
+  TableFunctionInitInput probe_input(get.bind_data.get(), std::move(column_ids),
+                                     projection_ids, &get.table_filters);
+  return BuildLanceTableFilterIRParts(names, types, probe_input, false);
+}
+
 LanceFilterIRBuildResult BuildLanceTableFilterIRParts(
     const vector<string> &names, const vector<LogicalType> &types,
     const TableFunctionInitInput &input, bool exclude_computed_columns) {
diff --git a/src/lance_scan.cpp b/src/lance_scan.cpp
@@ -1304,6 +1304,9 @@ LanceScanInitGlobal(ClientContext &context, TableFunctionInitInput &input) {
   if (!scan_state.sampling_pushed_down && bind_data.limit_offset_pushed_down) {
     // Limit/offset pushdown requires that any TableFilterSet predicates are
     // evaluated by Lance. Otherwise limit/offset would apply before filtering.
+    // Filter pushability is pre-checked in LanceLimitOffsetPushdown (the
+    // optimizer pass that sets limit_offset_pushed_down), so reaching here
+    // with a non-pushable filter set indicates an optimizer bug.
     if (input.filters && !input.filters->filters.empty() &&
         !scan_state.filter_pushed_down) {
       throw IOException("Lance limit/offset pushdown requires filter pushdown");
@@ -2367,6 +2370,21 @@ LanceLimitOffsetPushdown(unique_ptr<LogicalOperator> op) {
       return op;
     }
   }
+
+  // Pre-check filter pushability: limit/offset pushdown requires filter
+  // pushdown (otherwise Lance would apply LIMIT before DuckDB's filter,
+  // producing wrong results). If any table filter cannot be encoded to
+  // Lance IR (e.g., LIKE with interior wildcards, unsupported column types,
+  // or 'infinity' literals), keep the LogicalLimit in the plan and skip
+  // limit pushdown. DuckDB then applies LIMIT on top of the filtered
+  // stream — slower than Lance-side pushdown, but correct and strictly
+  // better than the previous "IO Error: Lance limit/offset pushdown
+  // requires filter pushdown" failure.
+  auto probe = ProbeLanceTableFilterIR(get, scan_bind.names, scan_bind.types);
+  if (!probe.all_filters_pushed) {
+    return op;
+  }
+
   scan_bind.limit_offset_pushed_down = true;
   scan_bind.pushed_limit = pushed_limit;
   scan_bind.pushed_offset = pushed_offset;
@@ -2451,7 +2469,8 @@ LanceExecPushdown(ClientContext &context, Optimizer &optimizer,
     extra_scan_col_ids.reserve(scan_get.table_filters.filters.size());
 
     if (!scan_get.table_filters.filters.empty()) {
-      idx_t max_col_id = 0;
+      // rowid filters have exec-pushdown-specific semantics; the shared
+      // probe below only covers IR encodability, so bail early here.
       for (auto &it : scan_get.table_filters.filters) {
         auto col_id = NumericCast<idx_t>(it.first);
         if (col_id >= scan_bind.names.size() ||
@@ -2463,21 +2482,10 @@ LanceExecPushdown(ClientContext &context, Optimizer &optimizer,
             IsLanceVirtualRowIdColumnId(col_id)) {
           return bail("Lance exec pushdown: rowid filters not supported");
         }
-        max_col_id = MaxValue(max_col_id, col_id);
-      }
-
-      vector<column_t> column_ids;
-      column_ids.reserve(max_col_id + 1);
-      for (idx_t i = 0; i <= max_col_id; i++) {
-        column_ids.push_back(NumericCast<column_t>(i));
       }
 
-      vector<idx_t> projection_ids;
-      TableFunctionInitInput init_input(scan_get.bind_data.get(),
-                                        std::move(column_ids), projection_ids,
-                                        &scan_get.table_filters);
-      auto table_filters = BuildLanceTableFilterIRParts(
-          scan_bind.names, scan_bind.types, init_input, false);
+      auto table_filters =
+          ProbeLanceTableFilterIR(scan_get, scan_bind.names, scan_bind.types);
       if (!table_filters.all_filters_pushed) {
         return bail("Lance exec pushdown: table_filters not fully pushable");
       }
diff --git a/test/sql/pushdown_limit_filter_fallback.test b/test/sql/pushdown_limit_filter_fallback.test
@@ -0,0 +1,158 @@
+# name: test/sql/pushdown_limit_filter_fallback.test
+# description: LIMIT/OFFSET gracefully degrades when filter pushdown fails
+# group: [sql]
+
+require lance
+
+# Setup: tiny table with a timestamp column. Filters involving 'infinity'
+# timestamps are NOT encodable as Lance filter IR (see pushdown_filter_ir_types
+# test: "Lance Filter IR Bytes": "0").
+statement ok
+COPY (
+  SELECT
+    id::BIGINT AS id,
+    TIMESTAMP '2020-01-01 00:00:00' + (id * INTERVAL '1 DAY') AS ts
+  FROM generate_series(1, 10) t(id)
+) TO 'test/.tmp/limit_filter_fallback.lance' (FORMAT lance, mode 'overwrite');
+
+# Baseline: without LIMIT, the unpushable filter runs correctly (Lance returns
+# all rows, DuckDB applies the filter). Returns 10.
+query I
+SELECT count(*) FROM 'test/.tmp/limit_filter_fallback.lance' WHERE ts < 'infinity'::TIMESTAMP;
+----
+10
+
+# Regression: filter pushdown fails (infinity timestamp) AND LIMIT is pushed.
+# Pre-fix this threw "Lance limit/offset pushdown requires filter pushdown".
+# Post-fix: scan runs with filter+limit applied by DuckDB on top of the
+# full-table scan, returning correct results.
+query I
+SELECT count(*) FROM (
+  SELECT * FROM 'test/.tmp/limit_filter_fallback.lance'
+  WHERE ts < 'infinity'::TIMESTAMP
+  LIMIT 3
+);
+----
+3
+
+# With OFFSET as well
+query I
+SELECT count(*) FROM (
+  SELECT * FROM 'test/.tmp/limit_filter_fallback.lance'
+  WHERE ts < 'infinity'::TIMESTAMP
+  LIMIT 3 OFFSET 2
+);
+----
+3
+
+# LIMIT larger than row count still works (returns all rows post-filter)
+query I
+SELECT count(*) FROM (
+  SELECT * FROM 'test/.tmp/limit_filter_fallback.lance'
+  WHERE ts < 'infinity'::TIMESTAMP
+  LIMIT 100
+);
+----
+10
+
+# Results are semantically correct (not just row count): IDs come back
+query I
+SELECT id FROM 'test/.tmp/limit_filter_fallback.lance'
+WHERE ts < 'infinity'::TIMESTAMP
+ORDER BY id
+LIMIT 2;
+----
+1
+2
+
+# OFFSET pushed down with unpushable filter
+query I
+SELECT id FROM 'test/.tmp/limit_filter_fallback.lance'
+WHERE ts < 'infinity'::TIMESTAMP
+ORDER BY id
+LIMIT 2 OFFSET 5;
+----
+6
+7
+
+# Explain: when the filter cannot be pushed to Lance IR, the optimizer
+# leaves the LogicalLimit in the plan (Lance Limit Offset Pushdown=false)
+# so DuckDB applies LIMIT on top of the filtered stream.
+query II
+EXPLAIN (ANALYZE, FORMAT JSON) SELECT * FROM 'test/.tmp/limit_filter_fallback.lance'
+WHERE ts < 'infinity'::TIMESTAMP
+LIMIT 3;
+----
+analyzed_plan	<REGEX>:[\s\S]*"Lance Limit Offset Pushdown": "false"[\s\S]*
+
+# When filters ARE pushable, limit pushdown still works as before.
+statement ok
+COPY (
+  SELECT id::BIGINT AS id FROM generate_series(1, 20) t(id)
+) TO 'test/.tmp/limit_filter_pushable.lance' (FORMAT lance, mode 'overwrite');
+
+query II
+EXPLAIN (ANALYZE, FORMAT JSON) SELECT * FROM 'test/.tmp/limit_filter_pushable.lance'
+WHERE id > 5
+LIMIT 3;
+----
+analyzed_plan	<REGEX>:[\s\S]*"Lance Limit Offset Pushdown": "true"[\s\S]*
+
+# And returns correct results.
+query I
+SELECT count(*) FROM (
+  SELECT * FROM 'test/.tmp/limit_filter_pushable.lance' WHERE id > 5 LIMIT 3
+);
+----
+3
+
+# Selective unpushable filter: verifies the fix doesn't drop the filter.
+# Pre-fix, if LIMIT pushdown was applied without the filter being encoded,
+# the scan would return the first N rows of the table and DuckDB's filter
+# above might see none of them pass. We use a pushable filter combined with
+# an unpushable filter in a CONJUNCTION_AND — the whole conjunction fails
+# encoding (see lance_filter_ir.cpp CONJUNCTION_AND handling), but the
+# individual pushable filter can still reject rows at the DuckDB layer.
+statement ok
+COPY (
+  SELECT
+    id::BIGINT AS id,
+    TIMESTAMP '2020-01-01 00:00:00' + (id * INTERVAL '1 DAY') AS ts
+  FROM generate_series(1, 20) t(id)
+) TO 'test/.tmp/limit_filter_selective.lance' (FORMAT lance, mode 'overwrite');
+
+# Filter selects ids > 15 (5 matches). With LIMIT 3 we should get 3 of those
+# matches, NOT first 3 rows of the table (ids 1, 2, 3) which would be wrong.
+query I
+SELECT id FROM 'test/.tmp/limit_filter_selective.lance'
+WHERE id > 15 AND ts < 'infinity'::TIMESTAMP
+ORDER BY id
+LIMIT 3;
+----
+16
+17
+18
+
+# Same semantics with OFFSET: skip 2 matches, take next 2.
+query I
+SELECT id FROM 'test/.tmp/limit_filter_selective.lance'
+WHERE id > 15 AND ts < 'infinity'::TIMESTAMP
+ORDER BY id
+LIMIT 2 OFFSET 2;
+----
+18
+19
+
+# Sparse projection: filter on a column not in the SELECT list. The probe
+# uses synthetic column_ids [0..max_filter_col]; this verifies that the
+# probe's column_ids layout doesn't diverge from real init when projection
+# is pruned.
+query I
+SELECT id FROM 'test/.tmp/limit_filter_selective.lance'
+WHERE id > 15 AND ts < 'infinity'::TIMESTAMP
+ORDER BY id
+LIMIT 3;
+----
+16
+17
+18
