terrymanu commented on issue #39120: URL: https://github.com/apache/shardingsphere/issues/39120#issuecomment-5003746917
Hi @YM-Sky, thanks for the detailed report. This is a bug in SQL Federation’s pagination parameter binding, not invalid JDBC or MyBatis-Plus usage. SQL Federation explicitly supports pagination, and ShardingSphere-JDBC accepts `PreparedStatement#setLong`. However, the SQL Federation compiler represents `LIMIT ?` as a dynamic parameter while the execution layer passes the original `Long` object unchanged to Calcite. Calcite’s generated `EnumerableLimit` path expects an `Integer`, producing the reported `Long`-to-`Integer` cast failure. Although SQL Federation is documented as experimental, that status does not make this valid numeric JDBC parameter unsupported. The fix should adapt only dynamic `LIMIT`/`OFFSET` parameters at the SQL Federation binding boundary, preserve unrelated `Long` parameters, and define range behavior without silent truncation. It should include focused unit coverage and a MySQL SQL Federation E2E case using a `long` pagination parameter. I suggest labeling this `type: bug`, `feature: SQL federation`, `in: kernel`, `in: JDBC`, and `db: MySQL`. Contributors are welcome to submit a PR with the fix and regression tests. The reply above is based on the analysis below; the detailed reasoning is kept here for reference and follow-up contributors. ### Problem Understanding - **Issue:** ShardingSphere-JDBC 5.5.3 fails during SQL Federation execution when MySQL `LIMIT ?` is bound through `PreparedStatement#setLong`. - **Topology:** JDBC + Standalone; MySQL; SQL Federation enabled; registry/config center N/A. - **Fast Triage:** The issue provides the version, topology, SQL, YAML, parameter type, execution plan, exception, and a plain JDBC reproducer. The evidence is sufficient to classify the issue. - **OBS-1:** The reproduction reaches `EnumerableLimit(fetch=[?0])` with parameter `20(Long)` and fails with `Long cannot be cast to Integer`. Source: [issue #39120](https://github.com/apache/shardingsphere/issues/39120). - **OBS-2:** The official SQL Federation documentation lists pagination among the supported distributed-query scenarios. Its limitations page only describes SQL Federation as experimental; it does not exclude parameterized pagination or `Long` numeric values. Sources: [SQL Federation](https://shardingsphere.apache.org/document/current/en/features/sql-federation/) and [SQL Federation limitations](https://shardingsphere.apache.org/document/current/en/features/sql-federation/limitation/). - **OBS-3:** `AbstractPreparedStatementAdapter#setLong` forwards the boxed `Long` to the parameter list without conversion: `jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/adapter/AbstractPreparedStatementAdapter.java:82` and `:263`. - **OBS-4:** `PaginationValueSQLConverter` converts a pagination parameter marker to `SqlDynamicParam`, and `SelectStatementConverter` installs it as the `SqlOrderBy` offset/fetch node: `kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/limit/PaginationValueSQLConverter.java:46` and `kernel/sql-federation/compiler/src/main/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/statement/type/SelectStatementConverter.java:56`. - **OBS-5:** `StandardSQLFederationProcessor#createParameters` stores every JDBC value unchanged under `?0`, `?1`, etc.; `ExecutorBindContext#get` returns that same object: `kernel/sql-federation/core/src/main/java/org/apache/shardingsphere/sqlfederation/engine/processor/impl/StandardSQLFederationProcessor.java:128` and `kernel/sql-federation/executor/src/main/java/org/apache/shardingsphere/sqlfederation/executor/context/ExecutorBindContext.java:57`. - **OBS-6:** Existing tests verify the pagination dynamic-parameter index and empty-parameter execution, but not `Long` pagination binding. The SQL Federation pagination E2E cases are currently commented out: `kernel/sql-federation/compiler/src/test/java/org/apache/shardingsphere/sqlfederation/compiler/sql/ast/converter/segment/limit/PaginationValueSQLConverterTest.java:44`, `kernel/sql-federation/core/src/test/java/org/apache/shardingsphere/sqlfederation/engine/processor/impl/StandardSQLFederationProcessorTest.java:193`, and `test/e2e/sql/src/test/resources/cases/dql/cases/db_tbl_sql_federation/e2e-dql-select-pagination-for-db-tbl-sql-federation.xml:20`. - **OBS-7:** Same-repository searches found no merged fix for this type-binding failure. [PR #21229](https://github.com/apache/shardingsphere/pull/21229) added SQL Federation `LIMIT` plan conversion, while [PR #21386](https://github.com/apache/shardingsphere/pull/21386) changed federation decider logic. Neither adapts runtime pagination parameter types. The current upstream path in `StandardSQLFederationProcessor` remains unchanged in this respect. ### Root Cause - **Observation:** The runtime chain is `setLong` → boxed `Long` → pagination `SqlDynamicParam` → unchanged SQL Federation `DataContext` value → `EnumerableLimit` expecting `Integer` (`OBS-1`, `OBS-3`, `OBS-4`, `OBS-5`). - **Inference (`INF-1`):** SQL Federation lacks context-aware numeric adaptation for dynamic pagination parameters at its Calcite execution boundary (`OBS-1`, `OBS-4`, `OBS-5`). - **Inference (`INF-2`):** This is not caused by MyBatis-Plus because the same failure is reproducible with plain JDBC `setLong` (`OBS-1`, `OBS-3`). - **Inference (`INF-3`):** This is not a duplicate or already-fixed issue; the related historical PRs address different stages of SQL Federation processing, and current upstream retains the failing pass-through behavior (`OBS-5`, `OBS-7`). - **Confidence:** High. ### Problem Analysis - **Issue Type:** Bug. - **Evidence:** Pagination is an intended SQL Federation capability, the JDBC layer accepts `setLong`, and the failure results from an internal type mismatch rather than unsupported SQL or configuration (`OBS-2`, `OBS-3`, `INF-1`). - **Duplicate Check:** No same-root-cause issue or merged fixing PR was found (`OBS-7`, `INF-3`). - **Compatibility Checklist:** Behavior change required; no configuration, API/SPI, or SQL syntax change should be necessary. ### Code-Level Design Suggestions - **Affected Modules:** `kernel/sql-federation/core`, potentially `kernel/sql-federation/compiler` for pagination-index metadata, and `test/e2e/sql`. - **Key Classes:** `StandardSQLFederationProcessor`, `PaginationValueSQLConverter`, `SelectStatementConverter`, and `ExecutorBindContext`. - **Minimum Fix Scope:** Identify parameter indexes owned by `LIMIT`/`OFFSET` and adapt compatible numeric values only at the SQL Federation binding boundary. Do not globally convert `Long` values in the JDBC adapter or all `DataContext` parameters. Define and test out-of-range behavior; do not silently truncate values. - **Required Test Scope:** Cover MySQL `LIMIT ?` with an in-range `Long`, MySQL offset/row-count pagination with `Long` values, an `Integer` control case, preservation of non-pagination `Long` parameters, and deterministic out-of-range behavior. Add an active cross-database SQL Federation E2E sentinel using `parameters="20:long"`. - **Rollback Hint:** Keep the adaptation isolated to pagination binding so it can be reverted without configuration, metadata, or public API changes. ### Problem Conclusion - **Evidence Confidence:** High. - **Severity:** S2. - **Impact Scope:** Confirmed for MySQL SQL Federation pagination through ShardingSphere-JDBC; broader database or Proxy impact should be checked because the federation execution boundary is shared. - **Topology:** JDBC + Standalone. - **Issue Type:** Bug. - **Recommended Labels:** `type: bug`, `feature: SQL federation`, `in: kernel`, `in: JDBC`, `db: MySQL`. - **Next Action:** Keep the issue open and accept a scoped PR that fixes pagination parameter binding with focused unit and E2E regression tests. - **Compatibility:** Behavior only; no intended Config/API-SPI/SQL syntax changes. - **Regression Scope:** Dynamic SQL Federation `LIMIT`/`OFFSET`, existing `Integer` pagination, non-pagination `Long` parameters, and numeric range handling. -- 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]
