Skip to content

fix: infer LIMIT and OFFSET parameter types - #25364

Open
1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:1fannnw/limit-parameter-types-2e62cbd8
Open

1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:1fannnw/limit-parameter-types-2e62cbd8

Conversation

@1fanwang

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Prepared-statement clients get no type for LIMIT/OFFSET arguments even when the query executes correctly. A later untyped occurrence can also erase an already inferred parameter type.

Prior work: #15980 and #23260.

What changes are included in this PR?

Default bare row-count parameters to Int64 only after collecting other type information. Preserve known fields and keep incomplete PREPARE signatures deferred rather than dropping unknown positions.

What is the testing strategy for this PR?

The tests plan, bind and execute real SQL, including mixed-type PREPARE statements. The same command ran on the base with the regression tests added, then on the fix, using Linux and Rust 1.98.1:

CARGO_BUILD_JOBS=2 cargo test --locked --profile ci -p datafusion \
  --test core_integration test_limit_offset_parameters \
  -- --nocapture --test-threads=1
# Scenario Command Result
1 LIMIT parameter metadata Above None becomes Int64; the query still returns 20

Before:

assertion `left == right` failed: SELECT 20 AS value LIMIT $1
  left: {"$1": None}
 right: {"$1": Some(Int64)}

After:

parameters={"$1": Some(Int64)}
+-------+
| value |
+-------+
| 20    |
+-------+
Additional raw PREPARE output

Before:

test sql::select::test_limit_offset_parameters_prepare_execute ... Prepare: "page" []

After:

test sql::select::test_limit_offset_parameters_prepare_execute ... Prepare: "page" [Int64, Int64]

Are there any user-facing changes?

Yes. Parameter metadata supplies integer types for otherwise-untyped LIMIT/OFFSET arguments while retaining known types and field metadata.

Preserve known parameter fields before applying row-count defaults and keep incomplete PREPARE signatures deferred.

Fixes apache#15978

Signed-off-by: 1fanwang <1fannnw@gmail.com>
…eter-types-2e62cbd8

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@github-actions github-actions Bot added sql SQL Planner logical-expr Logical plan and expressions core Core DataFusion crate labels Sep 16, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.94%. Comparing base (5a44905) to head (7666e87).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25364      +/-   ##
==========================================
- Coverage   81.94%   81.94%   -0.01%     
==========================================
  Files        1135     1135              
  Lines      428278   428292      +14     
  Branches   428278   428292      +14     
==========================================
+ Hits       350945   350951       +6     
- Misses      56378    56384       +6     
- Partials    20955    20957       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Placeholder datatype not inferred after LIMIT clause

2 participants