Skip to content

fix(mysql): infer LIMIT placeholders in prepare#8149

Open
discord9 wants to merge 1 commit into
GreptimeTeam:mainfrom
discord9:fix/mysql-limit-placeholder-param-count
Open

fix(mysql): infer LIMIT placeholders in prepare#8149
discord9 wants to merge 1 commit into
GreptimeTeam:mainfrom
discord9:fix/mysql-limit-placeholder-param-count

Conversation

@discord9
Copy link
Copy Markdown
Contributor

@discord9 discord9 commented May 21, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

Closes #8142

What's changed and what's your intention?

This PR fixes MySQL prepared statements with placeholders in LIMIT/OFFSET, e.g.:

SELECT ... FROM t LIMIT ?

Before this change, DataFusion kept the placeholder in the logical plan but could not infer its type. GreptimeDB's existing MySQL prepare path only advertises placeholders with inferred concrete types, so the client saw the prepared statement as requiring 0 parameters and failed before execution with an argument count mismatch.

The changes are:

  • Infer placeholders used by LogicalPlan::Limit (LIMIT/OFFSET) as Int64 in DfLogicalPlanner::get_inferred_parameter_types().
  • This keeps the existing MySQL prepare/execute logic unchanged while making LIMIT/OFFSET placeholders typed.
  • Add regression tests for LIMIT ? and mixed WHERE ? LIMIT ? prepared statements.

This does not change public APIs, schemas, or persisted data.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

Tests:

  • cargo fmt --check
  • cargo test -p query -- test_get_inferred_parameter_types_limit_offset
  • cargo test -p servers --features testing -- test_query_prepared

@discord9 discord9 requested review from a team, evenyag and waynexia as code owners May 21, 2026 09:09
@github-actions github-actions Bot added size/S docs-not-required This change does not impact docs. labels May 21, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements type inference for placeholders in LIMIT and OFFSET clauses, defaulting them to Int64, and ensures the MySQL handler correctly advertises all parameters to clients by using a null data type for unknown parameters. Review feedback identified a critical SQL injection vulnerability in the fallback execution path for unknown parameters, requiring string escaping. Additionally, the reviewer suggested improving error handling in the expression traversal logic to prevent swallowing potential errors.

Comment thread src/servers/src/mysql/handler.rs Outdated
Comment thread src/query/src/planner.rs Outdated
@discord9 discord9 force-pushed the fix/mysql-limit-placeholder-param-count branch 3 times, most recently from 6141346 to 2d8e86d Compare May 21, 2026 09:42
@github-actions github-actions Bot added size/XS and removed size/S labels May 21, 2026
Signed-off-by: discord9 <discord9@163.com>
@discord9 discord9 force-pushed the fix/mysql-limit-placeholder-param-count branch from 2d8e86d to 3e4fff4 Compare May 21, 2026 09:43
@discord9
Copy link
Copy Markdown
Contributor Author

Updated the PR to narrow the fix to the actual #8142 path: LIMIT/OFFSET placeholders are now inferred as Int64, and the MySQL handler/fallback string replacement changes were removed. The previous bot comments about unknown-placeholder fallback string substitution are therefore obsolete for the current diff.

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

Labels

docs-not-required This change does not impact docs. size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mysql interface does not expect arguments for LIMIT

1 participant