Repository navigation
Conversation
AI PR Review — ❌ 不可通行
问题清单高
中
低
最终总结[2m ▎ thinking�[0m PR #954 最终裁定报告（head
|
| 级别 | 位置 | 问题 |
|---|---|---|
| 高 | CommonDbConnector.java:1131 |
executeCommand 现在调用 4 参 execute。MysqlConnector 只覆写了 3 参 execute（MysqlConnector.java:1220，其中 setFetchSize(Integer.MIN_VALUE) 实现 MySQL 流式读取，并用 filterTimeForMysql 处理时间列）。4 参方法 MysqlConnector 未覆写，因而落到基类实现（CommonDbConnector.java:1151），导致：(1) setFetchSize(Integer.MIN_VALUE) 丢失，改为 setFetchSize(batchSize>0?batchSize:1000)（:1157）——MySQL Connector/J 下大结果集将整体在客户端缓冲，大结果 OOM（原注释即 //protected from OM）；(2) MySQL 专用的 filterTimeForMysql 被基类 filterData（:1168，对基类为 no-op）替代，TIME/DATE/DATETIME/TIMESTAMP 列的取值格式/时区行为改变。这是本 PR 引入的回归，且走 supportExecuteCommandFunction(this::executeCommand)（MysqlConnector.java:322）完全可达。 |
| 中 | SqlExecutor.java:16 |
新增的 default execute(..., queryTimeoutSeconds) 直接委托回旧签名，静默丢弃 queryTimeoutSeconds。任何未覆写新方法的 SqlExecutor 实现，其超时设置会被无声忽略（对共享库的外部/企业端实现者是行为陷阱）。注：本仓库内唯一实现 DefaultSqlExecutor 已正确覆写 6 参方法，故仓库内实际影响为 0；风险落在仓外实现者。 |
| 低 | CommonDbConnector.java:1154 |
新增的超时能力无任何测试：全仓无 queryTimeoutSeconds/setQueryTimeout 相关测试，变更文件也不含测试。executeCommand 传参→setQueryTimeout 应用→超时抛 SQLTimeoutException 被包成 error 结果这条链路无回归保护。 |
| 低 | CommonDbConnector.java:1128 |
(int) params.get("batchSize") 的裸强转：若 batchSize 从 JSON 反序列化为 Long 会抛 ClassCastException。（既有代码，本次未改动，紧邻新增的 instanceof Number 更稳健写法，建议一并加固：params.get("batchSize") instanceof Number ? ((Number)...).intValue() : 1000。） |
被驳回 / 澄清
- 无被驳回的实质发现：codexcli 的两条我均已在 head 源码核实为真，未发现可豁免项，也无可用于降级的维护者风险接受评论。
- claudecli 本轮未产出 finding（服务端 503），不构成第二个独立确认，但 codexcli 的 P1 已由我独立核实。
- 未发现另外的连接器也覆写了 3 参
execute：全仓仅MysqlConnector一处覆写（CommonDbConnector自身除外），故 P1 的破坏面精确落在 MySQL（及其子类语义）上。 - 单元层面：
setQueryTimeout单位（秒）与queryTimeoutSeconds一致、only if > 0语义正确（0/负数视为无超时）、instanceof Number解析稳健——这些非缺陷。
需要做什么
- （高，阻塞） 让超时分派不破坏子类覆写：在
MysqlConnector中覆写新的 4 参execute(sql, consumer, batchSize, queryTimeoutSeconds)（把 MySQL 的Integer.MIN_VALUE流式 +filterTimeForMysql逻辑迁到 4 参方法，3 参委托到 4 参），或让executeCommand仍走 3 参路径由子类自行委托。必须保留 MySQL 的流式读取与时间列处理。 - （中） 消除 default 方法的静默丢弃：要么让 default 方法在
queryTimeoutSeconds > 0且实现未覆写时显式抛出不支持异常/日志告警，要么让SqlExecutor的 3 参也带超时语义，避免共享库外部实现者无声失效。 - （低） 补充超时链路的测试：
executeCommand传入queryTimeoutSeconds后setQueryTimeout被调用、超时时返回 error 结果；并覆盖 MySQL 路径仍保持流式与时间列处理。 - （低） 加固
batchSize的解析（instanceof Number），与新增的queryTimeoutSeconds解析保持一致。
VERDICT: BLOCK —— 第 1 条是具体的正确性/数据风险回归（MySQL 命令查询在本次改动后走基类实现），未修复前不应合并。
以下是可机读的发现块（供逐条贴到对应改动行）：
FINAL_VERDICT: BLOCK
· 71340 tok · in 66313 (65792 cached / 521 new) · out 5027 (2485 reasoning) · ¥0.0119
codexcli review
-
P1 — MySQL command queries lose streaming and can exhaust memory. CommonDbConnector.java:1131 now dispatches to the new four-argument overload.
MysqlConnectoroverrides only the old three-argument method, which setsfetchSize(Integer.MIN_VALUE)to stream MySQL results; the new overload bypasses it and sets a normal fetch size instead. Largeexecute/executeQueryresults may therefore be buffered in memory, including when no timeout is supplied. -
P2 — Existing custom
SqlExecutorimplementations silently ignore requested timeouts. SqlExecuteCommandFunction.java:77 passes the timeout, but the new default method in SqlExecutor.java:16 discards it by delegating to the old signature. This preserves source compatibility while making a positive timeout ineffective for implementations that have not overridden the new method.
VERDICT: BLOCK
reasonix review
[2m ▎ thinking�[0m
I'll investigate the actual PR checkout to verify the reviewers' findings before arbitrating. Let me start by understanding the change scope.
· 26448 tok · in 25905 (256 cached / 25649 new) · out 543 (323 reasoning) · ¥0.0267
-> bash {"command": "cd /tmp/agentic-ui-pr-954-13706276211275287832/checkout && git log --oneline -5 2>/dev/null; echo "---HEAD...
-> glob {"pattern": "/sql-core/src/main/java/io/tapdata/common/{CommonDbConnector,SqlExecutor,SqlExecuteCommandFunction}.java"...
-> glob {"pattern": "/MysqlConnector.java"}
⊘ bash bash sandbox requested but unavailable on this host; refusing to run unconfined. Install bubblewrap (bwrap) or set [sandbox] bash = "off" in config.toml / Settings -> Sandbox to restore pre-1.16 unconfined shell execution.
�[2m ▎ thinking�[0m
Bash sandbox is unavailable on this host; I'll verify using read-only tools. Let me read the changed files.
· 26901 tok · in 26709 (26368 cached / 341 new) · out 192 (20 reasoning) · ¥0.0013
-> read_file {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-common/sql-core/src/main/java/io/tapdata/commo...
-> read_file {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-common/sql-core/src/main/java/io/tapdata/commo...
�[2m ▎ thinking�[0m
Now let me read CommonDbConnector.java around the changed method and MysqlConnector.
· 28656 tok · in 28403 (26880 cached / 1523 new) · out 253 (23 reasoning) · ¥0.0026
-> grep {"pattern": "execute|queryTimeout|fetchSize|SqlExecutor", "path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/...
-> grep {"pattern": "execute|queryTimeout|fetchSize|SqlExecutor|class MysqlConnector", "path": "/tmp/agentic-ui-pr-954-137062762...
�[2m ▎ thinking�[0m
Let me read the full implementations of both execute methods.
· 32202 tok · in 31939 (28544 cached / 3395 new) · out 263 (32 reasoning) · ¥0.0045
-> read_file {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-common/sql-core/src/main/java/io/tapdata/commo...
-> read_file {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors/mysql-connector/src/main/java/io/tapdata/conne...
�[2m ▎ thinking�[0m
Both P1 and P2 appear real so far. Let me dig into the implications and check for additional issues — implementers of SqlExecutor, the MySQL result-filter difference, and any added tests.
· 36903 tok · in 34742 (32128 cached / 2614 new) · out 2161 (1822 reasoning) · ¥0.0076
-> grep {"pattern": "implements SqlExecutor|extends DefaultSqlExecutor|new DefaultSqlExecutor|SqlExecuteCommandFunction", "path"...
-> grep {"pattern": "filterData|filterTimeForMysql", "path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-co...
-> grep {"pattern": "filterTimeForMysql|filterData", "path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors/my...
�[2m ▎ thinking�[0m
Confirmed — MysqlConnector.execute uses filterTimeForMysql while the base uses filterData. Now let me read DefaultSqlExecutor and the base filterData to assess the full impact.
· 39150 tok · in 38741 (36864 cached / 1877 new) · out 409 (181 reasoning) · ¥0.0034
-> read_file {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-common/sql-core/src/main/java/io/tapdata/commo...
-> read_file {"limit": 30, "offset": 1420, "path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors-common/sql-core/s...
�[2m ▎ thinking�[0m
DefaultSqlExecutor does override the 6-arg method and applies the timeout. Let me find all SqlExecutor implementers and locate any added tests and the diff.
· 43742 tok · in 43189 (39040 cached / 4149 new) · out 553 (396 reasoning) · ¥0.0060
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "SqlExecutor", "timeout_seconds": 120}
-> ls {"path": "/tmp"}
�[2m ▎ thinking�[0m
There's a patch file. Let me read the PR diff.
· 51197 tok · in 50818 (43648 cached / 7170 new) · out 379 (318 reasoning) · ¥0.0088
-> read_file {"path": "/tmp/tap12846-query-timeout.patch"}
�[2m ▎ thinking�[0m
Now let me find all connectors that override the 3-arg execute (which the new 4-arg call would bypass) and confirm the full changed-file set.
· 53327 tok · in 52766 (51072 cached / 1694 new) · out 561 (341 reasoning) · ¥0.0038
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "void execute\(String sql, Consumer", "time...
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "queryTimeoutSeconds|queryTimeout", "timeout...
�[2m ▎ thinking�[0m
Let me examine filterTimeForMysql to characterize the second-order impact, and confirm no tests were added.
· 57365 tok · in 54574 (53248 cached / 1326 new) · out 2791 (2562 reasoning) · ¥0.0080
-> read_file {"limit": 70, "offset": 638, "path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout/connectors/mysql-connector/sr...
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "queryTimeoutSeconds|setQueryTimeout|execute...
�[2m ▎ thinking�[0m
Let me confirm the full set of SqlExecutor-shaped implementers (including anonymous/other-interface ones) to scope P2 accurately.
· 65837 tok · in 62502 (57344 cached / 5158 new) · out 3335 (3102 reasoning) · ¥0.0130
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "TapSupplier connectionSupplier,...
-> grep {"path": "/tmp/agentic-ui-pr-954-13706276211275287832/checkout", "pattern": "new SqlExecutor|SqlExecutor\(\)|implement...
�[2m ▎ thinking�[0m
I have verified both reviewer findings against the head source (read-only; bash sandbox is unavailable on this host, so I could not execute tests). Here is the reconciliation.
PR #954 最终裁定报告（head a3b7fea87e939879688586e8e08103a1cab90e7a，TAP-12846: support SQL query timeout）
本轮只有 codexcli 产出发现（2 条，VERDICT: BLOCK）；claudecli 因推理网关 503（no available accounts）未产出任何发现。我逐行对照 head 源码核实了 codexcli 的两条发现——均属实且可达，另外补充 2 条低优问题。核心结论：新的超时能力在 CommonDbConnector.executeCommand 路径上回归破坏了 MySQL 的命令查询执行，故维持 BLOCK。
我已核实的变更范围
变更仅 4 个文件，全部在 connectors-common/sql-core：
CommonDbConnector.java：executeCommand改为分派 4 参execute；新增 4 参execute(sql, consumer, batchSize, queryTimeoutSeconds)并在其中setQueryTimeout。DefaultSqlExecutor.java：3 参execute委托到新的 6 参execute，后者setQueryTimeout。SqlExecuteCommandFunction.java：解析queryTimeoutSeconds并传入新的 6 参execute。SqlExecutor.java：新增带queryTimeoutSeconds的 default 方法。
已接受的发现
| 级别 | 位置 | 问题 |
|---|---|---|
| 高 | CommonDbConnector.java:1131 |
executeCommand 现在调用 4 参 execute。MysqlConnector 只覆写了 3 参 execute（MysqlConnector.java:1220，其中 setFetchSize(Integer.MIN_VALUE) 实现 MySQL 流式读取，并用 filterTimeForMysql 处理时间列）。4 参方法 MysqlConnector 未覆写，因而落到基类实现（CommonDbConnector.java:1151），导致：(1) setFetchSize(Integer.MIN_VALUE) 丢失，改为 setFetchSize(batchSize>0?batchSize:1000)（:1157）——MySQL Connector/J 下大结果集将整体在客户端缓冲，大结果 OOM（原注释即 //protected from OM）；(2) MySQL 专用的 filterTimeForMysql 被基类 filterData（:1168，对基类为 no-op）替代，TIME/DATE/DATETIME/TIMESTAMP 列的取值格式/时区行为改变。这是本 PR 引入的回归，且走 supportExecuteCommandFunction(this::executeCommand)（MysqlConnector.java:322）完全可达。 |
| 中 | SqlExecutor.java:16 |
新增的 default execute(..., queryTimeoutSeconds) 直接委托回旧签名，静默丢弃 queryTimeoutSeconds。任何未覆写新方法的 SqlExecutor 实现，其超时设置会被无声忽略（对共享库的外部/企业端实现者是行为陷阱）。注：本仓库内唯一实现 DefaultSqlExecutor 已正确覆写 6 参方法，故仓库内实际影响为 0；风险落在仓外实现者。 |
| 低 | CommonDbConnector.java:1154 |
新增的超时能力无任何测试：全仓无 queryTimeoutSeconds/setQueryTimeout 相关测试，变更文件也不含测试。executeCommand 传参→setQueryTimeout 应用→超时抛 SQLTimeoutException 被包成 error 结果这条链路无回归保护。 |
| 低 | CommonDbConnector.java:1128 |
(int) params.get("batchSize") 的裸强转：若 batchSize 从 JSON 反序列化为 Long 会抛 ClassCastException。（既有代码，本次未改动，紧邻新增的 instanceof Number 更稳健写法，建议一并加固：params.get("batchSize") instanceof Number ? ((Number)...).intValue() : 1000。） |
被驳回 / 澄清
- 无被驳回的实质发现：codexcli 的两条我均已在 head 源码核实为真，未发现可豁免项，也无可用于降级的维护者风险接受评论。
- claudecli 本轮未产出 finding（服务端 503），不构成第二个独立确认，但 codexcli 的 P1 已由我独立核实。
- 未发现另外的连接器也覆写了 3 参
execute：全仓仅MysqlConnector一处覆写（CommonDbConnector自身除外），故 P1 的破坏面精确落在 MySQL（及其子类语义）上。 - 单元层面：
setQueryTimeout单位（秒）与queryTimeoutSeconds一致、only if > 0语义正确（0/负数视为无超时）、instanceof Number解析稳健——这些非缺陷。
需要做什么
- （高，阻塞） 让超时分派不破坏子类覆写：在
MysqlConnector中覆写新的 4 参execute(sql, consumer, batchSize, queryTimeoutSeconds)（把 MySQL 的Integer.MIN_VALUE流式 +filterTimeForMysql逻辑迁到 4 参方法，3 参委托到 4 参），或让executeCommand仍走 3 参路径由子类自行委托。必须保留 MySQL 的流式读取与时间列处理。 - （中） 消除 default 方法的静默丢弃：要么让 default 方法在
queryTimeoutSeconds > 0且实现未覆写时显式抛出不支持异常/日志告警，要么让SqlExecutor的 3 参也带超时语义，避免共享库外部实现者无声失效。 - （低） 补充超时链路的测试：
executeCommand传入queryTimeoutSeconds后setQueryTimeout被调用、超时时返回 error 结果；并覆盖 MySQL 路径仍保持流式与时间列处理。 - （低） 加固
batchSize的解析（instanceof Number），与新增的queryTimeoutSeconds解析保持一致。
VERDICT: BLOCK —— 第 1 条是具体的正确性/数据风险回归（MySQL 命令查询在本次改动后走基类实现），未修复前不应合并。
以下是可机读的发现块（供逐条贴到对应改动行）：
===FINDINGS===
[高] connectors-common/sql-core/src/main/java/io/tapdata/common/CommonDbConnector.java:1131
新的 4 参 execute 分派绕过了 MysqlConnector 的 3 参覆写，MySQL 命令查询丢失流式读取与时间列处理
executeCommand（:1131）现在调用 4 参 execute(sql, …, batchSize, queryTimeoutSeconds)（新方法见 :1151）。但 MysqlConnector 只覆写了 3 参 execute（MysqlConnector.java:1220），其中 setFetchSize(Integer.MIN_VALUE) 是 MySQL Connector/J 的流式读取开关，并用 filterTimeForMysql 处理 TIME/DATE/DATETIME/TIMESTAMP 列。4 参方法未被覆写，动态分派落到基类实现（:1157 用 setFetchSize(batchSize>0?batchSize:1000)；:1168 用 no-op 的 filterData）。后果：(1) 大结果集在客户端整体缓冲，存在 OOM 风险（此行原本的注释即 //protected from OM）；(2) MySQL 时间列取值格式/时区行为改变。该路径经 supportExecuteCommandFunction(this::executeCommand)（MysqlConnector.java:322）完全可达，属本次改动引入的回归。
建议：在 MysqlConnector 中覆写新的 4 参 execute（保留 Integer.MIN_VALUE 流式与 filterTimeForMysql，让 3 参委托到 4 参），或让 executeCommand 继续走 3 参路径由子类委托；并补「MySQL 命令查询仍流式 + 时间列处理不变」的测试。
[中] connectors-common/sql-core/src/main/java/io/tapdata/common/SqlExecutor.java:16
新增 default 方法静默丢弃 queryTimeoutSeconds
default execute(..., queryTimeoutSeconds)（:16）直接委托回旧签名 execute(..., batchSize)（:18），把 queryTimeoutSeconds 静默丢弃。任何未覆写新方法的 SqlExecutor 实现，其请求的超时会被无声忽略（SqlExecuteCommandFunction.java:81 明确传入了该参数）。本仓库内唯一实现 DefaultSqlExecutor（DefaultSqlExecutor.java:50）已覆写 6 参方法并生效，故仓库内无实际破坏；风险位于共享库的外部/企业端实现者。
建议：default 方法在 queryTimeoutSeconds > 0 且实现未覆写时抛出不支持异常或记录告警，避免静默失效；或在接口层即为 3 参方法补超时语义。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/CommonDbConnector.java:1154
新增的 SQL 超时能力无任何测试覆盖
变更仅落在 4 个 main 文件，仓库内无任何针对 queryTimeoutSeconds/setQueryTimeout 的测试。新增的解析（:1129-1130）、传参（:1131-1132）与应用（:1154-1156）这条链路无回归保护，正是发现 1 未被测试暴露的原因。
建议：新增测试覆盖「executeCommand 传入正超时 → setQueryTimeout 被调用」与「超时时返回 error 结果」，并覆盖 MySQL 子类的流式与时间列行为保持。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/CommonDbConnector.java:1128
batchSize 的 (int) 裸强转在 Long 下会抛 ClassCastException（既有代码，紧邻本次改动）
int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000;（:1128）。若 params 来自 JSON 反序列化且 batchSize 被解析为 Long，该强制转换会抛 ClassCastException。此行本次未改动，但紧邻新增的 queryTimeoutSeconds 已改用稳健的 instanceof Number 写法，二者不一致。
建议：与 queryTimeoutSeconds 解析保持一致，改为 params.get("batchSize") instanceof Number ? ((Number) params.get("batchSize")).intValue() : 1000。
===END===
FINAL_VERDICT: BLOCK
· 71340 tok · in 66313 (65792 cached / 521 new) · out 5027 (2485 reasoning) · ¥0.0119
标记：❌ 不可通行
| execute(sql, list -> consumer.accept(new ExecuteResult().result(list)), batchSize); | ||
| int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number | ||
| ? ((Number) params.get("queryTimeoutSeconds")).intValue() : 0; | ||
| execute(sql, list -> consumer.accept(new ExecuteResult().result(list)), batchSize, |
There was a problem hiding this comment.
[高] 新的 4 参 execute 分派绕过了 MysqlConnector 的 3 参覆写，MySQL 命令查询丢失流式读取与时间列处理
executeCommand（:1131）现在调用 4 参 execute(sql, …, batchSize, queryTimeoutSeconds)（新方法见 :1151）。但 MysqlConnector 只覆写了 3 参 execute（MysqlConnector.java:1220），其中 setFetchSize(Integer.MIN_VALUE) 是 MySQL Connector/J 的流式读取开关，并用 filterTimeForMysql 处理 TIME/DATE/DATETIME/TIMESTAMP 列。4 参方法未被覆写，动态分派落到基类实现（:1157 用 setFetchSize(batchSize>0?batchSize:1000)；:1168 用 no-op 的 filterData）。后果：(1) 大结果集在客户端整体缓冲，存在 OOM 风险（此行原本的注释即 //protected from OM）；(2) MySQL 时间列取值格式/时区行为改变。该路径经 supportExecuteCommandFunction(this::executeCommand)（MysqlConnector.java:322）完全可达，属本次改动引入的回归。
建议：在 MysqlConnector 中覆写新的 4 参 execute（保留 Integer.MIN_VALUE 流式与 filterTimeForMysql，让 3 参委托到 4 参），或让 executeCommand 继续走 3 参路径由子类委托；并补「MySQL 命令查询仍流式 + 时间列处理不变」的测试。
|
|
||
| void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, Supplier<Boolean> aliveSupplier, int batchSize) throws Throwable; | ||
|
|
||
| default void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, |
There was a problem hiding this comment.
[中] 新增 default 方法静默丢弃 queryTimeoutSeconds
default execute(..., queryTimeoutSeconds)（:16）直接委托回旧签名 execute(..., batchSize)（:18），把 queryTimeoutSeconds 静默丢弃。任何未覆写新方法的 SqlExecutor 实现，其请求的超时会被无声忽略（SqlExecuteCommandFunction.java:81 明确传入了该参数）。本仓库内唯一实现 DefaultSqlExecutor（DefaultSqlExecutor.java:50）已覆写 6 参方法并生效，故仓库内无实际破坏；风险位于共享库的外部/企业端实现者。
建议：default 方法在 queryTimeoutSeconds > 0 且实现未覆写时抛出不支持异常或记录告警，避免静默失效；或在接口层即为 3 参方法补超时语义。
| protected void execute(String sql, Consumer<Object> consumer, int batchSize, int queryTimeoutSeconds) throws Throwable { | ||
| try (Connection connection = jdbcContext.getConnection(); | ||
| Statement sqlStatement = connection.createStatement()) { | ||
| if (queryTimeoutSeconds > 0) { |
There was a problem hiding this comment.
[低] 新增的 SQL 超时能力无任何测试覆盖
变更仅落在 4 个 main 文件，仓库内无任何针对 queryTimeoutSeconds/setQueryTimeout 的测试。新增的解析（:1129-1130）、传参（:1131-1132）与应用（:1154-1156）这条链路无回归保护，正是发现 1 未被测试暴露的原因。
建议：新增测试覆盖「executeCommand 传入正超时 → setQueryTimeout 被调用」与「超时时返回 error 结果」，并覆盖 MySQL 子类的流式与时间列行为保持。
| @@ -1126,7 +1126,10 @@ | |||
| case "executeQuery": | |||
| String sql = (String) params.get("sql"); | |||
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | |||
There was a problem hiding this comment.
[低] batchSize 的 (int) 裸强转在 Long 下会抛 ClassCastException（既有代码，紧邻本次改动）
int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000;（:1128）。若 params 来自 JSON 反序列化且 batchSize 被解析为 Long，该强制转换会抛 ClassCastException。此行本次未改动，但紧邻新增的 queryTimeoutSeconds 已改用稳健的 instanceof Number 写法，二者不一致。
建议：与 queryTimeoutSeconds 解析保持一致，改为 params.get("batchSize") instanceof Number ? ((Number) params.get("batchSize")).intValue() : 1000。
AI PR Review — ✅ 可通行
问题清单中
低
最终总结[2m ▎ thinking�[0m 核对方式本机 仲裁结论两位 reviewer 的 PASS 结论我接受：改动方向正确（超时只在该语句新建的 同时确认两位都只给了 Low，但代码里确实存在若干"静默失效"路径，属于必须写清楚但不必阻塞的问题。 已接受 findings（1 中 + 6 低）
未采纳 / 已澄清项
下一步① 把两处 params 解析抽成一个 helper（兼容 String、clamp 到 FINAL_VERDICT: PASS · 76251 tok · in 69600 (59648 cached / 9952 new) · out 6651 (2736 reasoning) · ¥0.0244 codexcli reviewNo concrete issues found in the five reviewable implementation files. The timeout reaches the default SQL executor and both common and MySQL connector execution paths; absent or non-positive values preserve the existing behavior. Legacy VERDICT: PASS claudecli reviewLooking at PR #954 which adds optional SQL query timeout support across the connector framework. Code AnalysisArchitecture & Design✅ Good backward compatibility: Uses method overloading and interface default methods to avoid breaking existing implementations Findings[Low] CommonDbConnector.java:1129 & SqlExecuteCommandFunction.java:77 — Silent type handling inconsistency int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000;
int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number
? ((Number) params.get("queryTimeoutSeconds")).intValue() : 0;
Impact: String timeout values from external configs fail silently. Given that query timeout is defensive (preventing runaway queries), silent failure means expected protection doesn't activate. Recommendation: Either make both lenient ( [Low] CommonDbConnector.java:1130 — Integer overflow wraps to negative, bypassing timeout int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number
? ((Number) params.get("queryTimeoutSeconds")).intValue() : 0;
if (queryTimeoutSeconds > 0) {If params contains a Long like Recommendation: Either clamp to Integer.MAX_VALUE or document that values must fit in a 32-bit signed int. [Low] Missing inline documentation — Timeout behavior not commented The parameter name
Recommendation: Add javadoc or inline comment explaining JDBC timeout semantics, especially since this is a public API parameter. [Low] MysqlConnectorTest.java:68 — Test doesn't verify timeout exception behavior Test verifies
Recommendation: Add a test with a mock that simulates timeout exception to ensure exception handling path is correct. Positive Observations✅ Thread-safe: Statement is method-local and try-with-resources scoped Correctness Check
The implementation is functionally correct and safe. The findings are polish issues around input validation consistency and documentation. VERDICT: PASS reasonix review[2m ▎ thinking�[0m 核对方式本机 仲裁结论两位 reviewer 的 PASS 结论我接受：改动方向正确（超时只在该语句新建的 同时确认两位都只给了 Low，但代码里确实存在若干"静默失效"路径，属于必须写清楚但不必阻塞的问题。 已接受 findings（1 中 + 6 低）
未采纳 / 已澄清项
下一步① 把两处 params 解析抽成一个 helper（兼容 String、clamp 到 FINAL_VERDICT: PASS ===FINDINGS===
|
|
|
||
| connector.execute("select created_at from records", batches::add, 10, 8); | ||
|
|
||
| verify(statement).setQueryTimeout(8); |
There was a problem hiding this comment.
[中] 新增测试只覆盖 MySQL 一条路径，共享实现与异常路径零测试
本 PR 改动的核心在两个被几十个连接器共享的类：CommonDbConnector.execute（经 this::executeCommand 被 clickhouse/doris/dws/greenplum/highgo/opengauss/postgres/starrocks/tidb/vastbase/oceanbase/mysql 等约 20 个连接器注册）和 DefaultSqlExecutor.execute（被 hive/hive1/hudi/snowflake/databend 经 SqlExecuteCommandFunction 使用）。但仓库内 connectors-common/sql-core/src/test/java/io/tapdata/common/ 下没有任何 DefaultSqlExecutorTest/SqlExecuteCommandFunctionTest，CommonDbConnectorTest 也不含 execute/executeCommand 的用例（全仓 grep queryTimeout 只命中实现代码与本条 MysqlConnector 测试）。唯一的新用例只断言 verify(statement).setQueryTimeout(8)，未覆盖：①queryTimeoutSeconds <= 0 时不得调用 setQueryTimeout（防回归到"总是设置"）；②超时抛 SQLException 后 Statement/Connection 是否被 try-with-resources 正确关闭、connection.commit() 是否按预期未执行；③CommonDbConnector 的 instanceof Number 解析分支（字符串/超 int 值）；④SqlExecuteCommandFunction 从 params 取值的端到端路径。
建议：为 CommonDbConnector.execute 与 DefaultSqlExecutor.execute 各补一组 Mockito 用例（timeout>0 设置、timeout=0/负数不设置、execute 抛 SQLException 时资源被 close），并加一条覆盖 SqlExecuteCommandFunction params 解析（含字符串值与超 int 值）的用例。
| String sql = (String) params.get("sql"); | ||
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | ||
| execute(sql, list -> consumer.accept(new ExecuteResult().result(list)), batchSize); | ||
| int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number |
There was a problem hiding this comment.
[低] queryTimeoutSeconds 与 batchSize 的入参校验策略相反，字符串型超时会被静默忽略
同一段代码里两个参数用了互相矛盾的解析方式：batchSize 是 params.get("batchSize") != null ? (int) params.get("batchSize") : 1000（类型不对直接 ClassCastException），而 queryTimeoutSeconds 是 instanceof Number ? ((Number) ...).intValue() : 0（类型不对静默变 0）。后果是：调用方传入字符串形式的超时（如 {"queryTimeoutSeconds": "30"}，外部配置/前端传参很常见）时不会报错，也不会设置超时，> 0 判断永远不成立，使用者以为超时保护已生效而实际没有——对一个"防止失控 SQL"的防护特性来说，静默失效比快速失败更危险。
建议：抽一个公共 helper 统一解析（优先 Number，其次 String 走 Integer.parseInt 并捕获异常），两处（CommonDbConnector:1129、SqlExecuteCommandFunction:77）共用；非法值至少打一条 warn 日志，或按仓库既有风格 fail-fast，别让两个相邻参数走两套语义。
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | ||
| execute(sql, list -> consumer.accept(new ExecuteResult().result(list)), batchSize); | ||
| int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number | ||
| ? ((Number) params.get("queryTimeoutSeconds")).intValue() : 0; |
There was a problem hiding this comment.
[低] intValue() 对超 int 的数值回绕成负数，超时被静默关闭
((Number) params.get("queryTimeoutSeconds")).intValue() 在值为 Long 2147483648L、9999999999L 或大 BigDecimal 时按 32 位截断成负数（如 -2147483648），随后 if (queryTimeoutSeconds > 0) 判定失败，setQueryTimeout 根本不会被调用。params 一般经 JSON 反序列化，超过 Integer.MAX_VALUE 的整数会自然变成 Long，因此这不是纯理论输入。结果是用户以为设了一个"很大但仍有限"的超时，实际得到的是"无限"。
建议：解析后 Math.min(value, Integer.MAX_VALUE)（对 long 先比较再收窄），或在 helper 里显式拒绝超范围值并回报错误；并在 javadoc 里写明取值范围。
| String sql = (String) params.get("sql"); | ||
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | ||
| sqlExecutor.execute(sql, connectionSupplier, list -> executeResultConsumer.accept(new ExecuteResult().result(list)), aliveSupplier, batchSize); | ||
| int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number |
There was a problem hiding this comment.
[低] 同一段参数解析逻辑被复制到第二处，修正时容易漏改
SqlExecuteCommandFunction 里又写了一遍与 CommonDbConnector:1129-1130 完全相同的两行（instanceof Number + intValue()），同样的类型静默回落与溢出问题在这里也成立。两份拷贝意味着后续修一处漏一处，而这个类正是 hive/hive1/hudi/snowflake/databend 五个连接器执行 SQL 的唯一入口，行为必须与 SQL 系连接器保持一致。
建议：把解析逻辑抽成 SqlParamsUtils.getQueryTimeoutSeconds(Map)（或放进 CommonDbConnector 的静态工具方法）两处共用；同时把紧邻的 batchSize（:76，(int) 强转，Long 输入会 ClassCastException）一并收敛到同一 helper。
| sqlExecutor.execute(sql, connectionSupplier, list -> executeResultConsumer.accept(new ExecuteResult().result(list)), aliveSupplier, batchSize); | ||
| int queryTimeoutSeconds = params.get("queryTimeoutSeconds") instanceof Number | ||
| ? ((Number) params.get("queryTimeoutSeconds")).intValue() : 0; | ||
| sqlExecutor.execute(sql, connectionSupplier, |
There was a problem hiding this comment.
[低] call（存储过程/函数）路径完全没有超时，PR 目标在这条执行路径上不完整
同一个 executeCommand 的 switch 里，execute/executeQuery 分支现在会透传 queryTimeoutSeconds，但 case "call"（:83）依旧只走 sqlExecutor.call(funcName, callParams, connectionSupplier)；对应实现 DefaultSqlExecutor.call:133 与 MysqlConnector.call:1275/1305 的 CallableStatement 都没有 setQueryTimeout。也就是说一条卡死的存储过程仍然可以无限期挂住任务——而这正是"支持 SQL 查询超时"最需要防护的场景之一。同时 SqlExecutor 的 call 方法签名也没有超时参数，能力缺口在接口层就已存在。
建议：至少在本 PR 说明里明确"call 路径暂不支持超时"；若要做，给 SqlExecutor.call 增加带 queryTimeoutSeconds 的重载（或新增 default 方法保持兼容），在 CallableStatement 创建后 setQueryTimeout，并同步 MysqlConnector.call 的两个 CallableStatement 分支。
| @@ -1142,8 +1145,15 @@ protected void executeCommand(TapConnectorContext connectorContext, TapExecuteCo | |||
| } | |||
|
|
|||
| protected void execute(String sql, Consumer<Object> consumer, int batchSize) throws Throwable { | |||
There was a problem hiding this comment.
[低] 三参 execute 退化为纯委派，只覆盖三参版本的外部子类会被静默旁路
改造后 executeCommand（:1131）调用四参 execute，而三参版本（:1147）只是 execute(sql, consumer, batchSize, 0) 的转发。在本 PR 之前，三参 execute 是唯一的扩展点：子类覆盖它即可替换整段执行逻辑。现在任何仍只覆盖三参版本的子类，executeCommand 的虚分派会直接落到 CommonDbConnector 的四参实现上，子类的自定义执行逻辑（自定义 fetchSize、自定义行转换等）被整体跳过，且没有任何编译期或运行期提示。仓库内只有 MysqlConnector:1221/1226 覆盖此方法且已同步改造成对，所以当前无实际故障，但这是个隐蔽的破坏性扩展点变更。
建议：把三参 execute 标 @Deprecated 并在 javadoc 写明"子类必须覆盖四参版本，三参版本仅为兼容保留"；或直接删除三参重载（仓库内已无独立调用者），避免后来者继续按旧方式扩展。
| void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, Supplier<Boolean> aliveSupplier, int batchSize) throws Throwable; | ||
|
|
||
| default void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, | ||
| Supplier<Boolean> aliveSupplier, int batchSize, int queryTimeoutSeconds) throws Throwable { |
There was a problem hiding this comment.
[低] 新增的 default 重载会静默丢弃超时，且接口层没有任何契约说明
SqlExecutor 新增的六参 execute 以 default 方法实现为"直接转调五参版本"，因此任何未覆盖六参版本的实现都会静默忽略 queryTimeoutSeconds——没有异常、没有日志。当前仓库内只有 DefaultSqlExecutor 实现该接口且已覆盖六参，所以现在安全，但这是一个面向 connector 模块的公开接口，未来新增实现（或外部 PDK 连接器）极易踩中"设了超时却不生效"。此外 queryTimeoutSeconds 的语义（单位秒、0 与负数表示不限、超时以 SQLException 抛出）在任何注释里都没有出现，只有实现里的 > 0 暗示。
建议：在 SqlExecutor 的六参方法上补 javadoc（单位、<=0 语义、超时异常类型、default 实现忽略该参数的事实）；DefaultSqlExecutor:45 的五参重载也加一行说明"转发到六参并传 0"。
| try (Connection connection = jdbcContext.getConnection(); | ||
| Statement sqlStatement = connection.createStatement()) { | ||
| if (queryTimeoutSeconds > 0) { | ||
| sqlStatement.setQueryTimeout(queryTimeoutSeconds); |
There was a problem hiding this comment.
[低] MySQL 流式读取下的超时语义未经验证，且超时同样会作用在逐行 fetch 阶段
execute 保留了 setFetchSize(Integer.MIN_VALUE) 的流式语义（:1232），这很好；但 setQueryTimeout 在 Connector/J 中的实现是借独立连接下发 KILL QUERY，而流式结果集的"语句执行期"包含逐行 fetch，因此一张大表的长时间流式扫描也可能在 queryTimeoutSeconds 到点后被中断——这对"防止慢 SQL"是期望行为，对"大表导出"则是行为变更，需要明确语义而不是靠用户踩坑。现有证据只有 Mockito 桩（verify(statement).setQueryTimeout(8)），既证明不了真实 MySQL 上超时确实生效，也证明不了流式 fetch 的实际表现；MysqlConnectorTest 也没有覆盖"超时异常后 connection.commit() 未执行"的路径。
建议：补一条基于真实 MySQL 的 IT（SELECT SLEEP(10) + queryTimeoutSeconds=1 应抛 SQLException），并在一条大表流式读取的 IT 上确认超时被触发时的连接清理；同时在 execute 上注释"超时同时覆盖 fetch 阶段，大表全扫请设为 0 或足够大"。
AI PR Review — ✅ 可通行
问题清单中
低
最终总结[2m ▎ thinking�[0m 核对方式本机 仲裁结论这个 head 相比上一轮实质改进了：上一轮我提的三条"静默失效"路径已被新增的 codexcli 的 BLOCK 我降级为「中」，理由是硬证据：全仓 claudecli 的 PASS 我接受，其对三条执行路径（ 接受的 findings（2 中 + 5 低）
驳回/澄清
下一步① 给 FINAL_VERDICT: PASS · 83485 tok · in 78033 (77696 cached / 337 new) · out 5452 (1321 reasoning) · ¥0.0128 codexcli review
VERDICT: BLOCK claudecli reviewLooking at PR #954, I'll provide a thorough review of the SQL query timeout implementation. Review of PR #954: SQL Query Timeout SupportCritical Analysis of Codexcli's Blocking ConcernThe Default Method Silent Failure Pattern Codexcli identified that the default method in default void execute(..., int queryTimeoutSeconds) throws Throwable {
execute(sql, connectionSupplier, consumer, aliveSupplier, batchSize);
}This means custom Assessment: This is a legitimate concern but not blocking because:
However: This should be documented with Code Quality Review✅ SqlCommandParams.java (New utility class)This addresses previous reviewers' concerns about type safety: new BigDecimal(value.toString()).intValueExact()
✅ Timeout ApplicationAll three execution paths correctly apply timeout:
Only sets timeout when ✅ MySQL Streaming Preserved
Findings[Medium] SqlExecutor.java:18 — Default method should document silent timeout degradation default void execute(..., int queryTimeoutSeconds) throws Throwable {
execute(sql, connectionSupplier, consumer, aliveSupplier, batchSize);
}The default implementation discards Recommendation: Add javadoc: /**
* Execute SQL with optional query timeout.
* @param queryTimeoutSeconds maximum execution time in seconds; 0 or negative means no limit
* @implNote The default implementation ignores the timeout parameter for backward compatibility.
* Implementations should override this method to support timeouts via Statement.setQueryTimeout().
*/[Low] SqlExecutor.java:18 — No javadoc for timeout parameter semantics
Recommendation: Add comprehensive javadoc as shown above. [Low] SqlCommandParams.java:6 — Package-private utility with no class-level documentation Recommendation: Add class javadoc: /**
* Utility for parsing parameters from SQL executeCommand requests with type safety.
*/[Low] Test coverage gaps — MysqlConnectorTest.java:46-74
Not covered:
Recommendation: Add tests for:
[Low] Stored procedure path unsupported — SqlExecuteCommandFunction.java:79-83 This is a known limitation based on the PR scope, but should be documented in the PR description or as a TODO comment. Positive Observations✅ Security & Correctness
VerdictThe implementation is functionally correct and safe. The default method pattern is a standard and acceptable approach for interface evolution. The VERDICT: PASS reasonix review[2m ▎ thinking�[0m 核对方式本机 仲裁结论这个 head 相比上一轮实质改进了：上一轮我提的三条"静默失效"路径已被新增的 codexcli 的 BLOCK 我降级为「中」，理由是硬证据：全仓 claudecli 的 PASS 我接受，其对三条执行路径（ 接受的 findings（2 中 + 5 低）
驳回/澄清
下一步① 给 FINAL_VERDICT: PASS ===FINDINGS===
|
|
|
||
| default void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, | ||
| Supplier<Boolean> aliveSupplier, int batchSize, int queryTimeoutSeconds) throws Throwable { | ||
| execute(sql, connectionSupplier, consumer, aliveSupplier, batchSize); |
There was a problem hiding this comment.
[中] 默认 6 参 execute 静默丢弃超时，且接口层没有任何契约说明
default void execute(..., int queryTimeoutSeconds) 直接转调 5 参版本，任何未覆盖该重载的实现都会无异常、无日志地丢掉超时（codexcli 因此判 BLOCK）。我核实后确认当前仓库内没有可达的失效路径：全仓 implements SqlExecutor 只有 DefaultSqlExecutor，它已正确覆盖 6 参并在 :54 设置超时；SqlExecuteCommandFunction(SqlExecutor) 这个可注入构造器在仓内无任何调用方（hive/hive1/hudi/snowflake/databend 全走静态入口 + 内部 new DefaultSqlExecutor()），所以这不是现网缺陷，也不该改成抽象方法（那会破坏外部实现，比静默降级更糟）。真正的缺口是契约缺失：queryTimeoutSeconds 的单位（秒）、0/负数=不限、超时以 SQLException 抛出、以及"default 实现会忽略该参数、实现方必须覆盖"这一事实，在任何注释里都没有出现，而 SqlExecutor 是 sql-core 对外暴露的接口，tapdata-connectors-enterprise / 外部 PDK 连接器最容易踩中"设了超时却不生效"。
建议：在该重载上补 javadoc 与 @implNote（"The default implementation ignores queryTimeoutSeconds for backward compatibility; implementations must override and call Statement.setQueryTimeout()"），DefaultSqlExecutor:45 的 5 参重载也加一行"转发到 6 参并传 0"；同时在 PR 描述/release note 记录该兼容性约定。
|
|
||
| connector.execute("select created_at from records", batches::add, 10, 8); | ||
|
|
||
| verify(statement).setQueryTimeout(8); |
There was a problem hiding this comment.
[中] 新增测试只覆盖 MySQL 一条 happy path，新解析类与两条共享执行路径零测试
唯一的新用例只断言 verify(statement).setQueryTimeout(8) 与 setFetchSize(Integer.MIN_VALUE)。而本 PR 的核心在共享代码：CommonDbConnector.executeCommand/execute（14 个连接器经 supportExecuteCommandFunction(this::executeCommand) 注册：clickhouse/doris/dws/greenplum/highgo/opengauss/vastbase/oceanbase-mysql/mysql/postgres/starrocks/tidb/paimon-plus 等）与 DefaultSqlExecutor.execute（hive/hive1/hudi/snowflake/databend 经 SqlExecuteCommandFunction）。我核实 connectors-common/sql-core/src/test/java/io/tapdata/common/ 下没有 SqlCommandParamsTest、没有 DefaultSqlExecutorTest，CommonDbConnectorTest 只测索引 SQL。也就是说全新的 SqlCommandParams（两条 fail-fast 分支 + Integer.MAX_VALUE 上界分支）、<=0 不调用 setQueryTimeout 的防回归、以及"超时抛 SQLException 后 Statement/Connection 被关闭、connection.commit() 未执行"全都没有任何断言。
建议：新增 SqlCommandParamsTest（null→默认、"30"、"abc"→IAE、2147483648→IAE、-1→0、1000.5），给 DefaultSqlExecutor.execute 与 CommonDbConnector.execute 各加一组 Mockito 用例（>0 设置、=0/负数不设置 setQueryTimeout、执行抛异常时资源被 close 且未 commit），并补 MysqlConnector.execute 收到 0/负数的用例。
| @@ -1142,8 +1144,15 @@ protected void executeCommand(TapConnectorContext connectorContext, TapExecuteCo | |||
| } | |||
|
|
|||
| protected void execute(String sql, Consumer<Object> consumer, int batchSize) throws Throwable { | |||
There was a problem hiding this comment.
[低] 三参 execute 退化为纯委派，只覆盖三参版本的外部子类会被整体旁路
改造后 executeCommand（:1130）虚分派到四参 execute，而三参版本（:1146）只剩 execute(sql, consumer, batchSize, 0) 的转发。在本 PR 之前，三参 execute 是唯一的扩展点：子类覆盖它即可替换整段执行逻辑（自定义 fetchSize、自定义行转换、自定义提交策略）。现在任何仍只覆盖三参版本的子类，其自定义逻辑会被完全跳过（不只是丢超时），且没有编译期或运行期提示。仓库内唯一覆盖者 MysqlConnector:1221/1226 已同步改造，所以当前无故障；但 CommonDbConnector 是 enterprise/外部连接器的基类，本工作区无法核对那些子类——这一条两位 reviewer 都没有提到。
建议：给三参 execute 标 @Deprecated 并在 javadoc 写明"子类必须覆盖四参版本，三参版本仅为兼容保留"，在 release note 中提示扩展点迁移；或为三参版本保留一个可见的 hook 让旧覆盖仍然生效。
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | ||
| sqlExecutor.execute(sql, connectionSupplier, list -> executeResultConsumer.accept(new ExecuteResult().result(list)), aliveSupplier, batchSize); | ||
| int batchSize = SqlCommandParams.getInteger(params, "batchSize", 1000); | ||
| int queryTimeoutSeconds = SqlCommandParams.getQueryTimeoutSeconds(params); |
There was a problem hiding this comment.
[低] call（存储过程/函数）路径完全没有超时，PR 目标在这条执行路径上不完整
同一个 executeCommand 的 switch 里，execute/executeQuery 分支已透传 queryTimeoutSeconds，但 case "call"（:82）依旧只走 sqlExecutor.call(funcName, callParams, connectionSupplier)；对应实现 DefaultSqlExecutor.call:122/133 与 MysqlConnector.call:1260/1270、CommonDbConnector.call:1185/1196 的 CallableStatement 都没有 setQueryTimeout，SqlExecutor.call 的方法签名也没有超时入参——能力缺口在接口层就已存在。结果是一条卡死的存储过程仍可无限期挂住任务，而这正是"防止失控 SQL"最需要覆盖的场景之一。
建议：至少在 PR 描述里明确"call 路径暂不支持超时"；若要做，给 SqlExecutor.call 增加带 queryTimeoutSeconds 的 default 重载（保持兼容），在 CallableStatement 创建后 setQueryTimeout，并同步 MysqlConnector.call 与 CommonDbConnector.call。
| } catch (NumberFormatException | ArithmeticException e) { | ||
| throw new IllegalArgumentException("Parameter 'queryTimeoutSeconds' must be an integer", e); | ||
| } | ||
| if (seconds <= 0) { |
There was a problem hiding this comment.
[低] 非正数超时被静默归一为"无限制"，与相邻参数的 fail-fast 策略相反，且类无 javadoc
getQueryTimeoutSeconds 对 <= 0 直接返回 0（= 不限），而同一个类里的 getInteger（:11）对非法值一律抛 IllegalArgumentException。后果是客户端把 queryTimeoutSeconds 误填 -1 时既无异常也无日志，得到的是"无限等待"——一个防护特性静默失效；对 "abc" 这类真非法值反而是 fail-fast，两种坏输入两种语义。此外新增的 final class SqlCommandParams 完全没有类级注释说明它存在的目的（统一/安全地解析 execute command params），两处调用方（CommonDbConnector:1128、SqlExecuteCommandFunction:76）的语义也无处可查；getInteger 用 intValueExact 而 getQueryTimeoutSeconds 用 longValueExact+上界比较，风格不统一（都正确）。
建议：<= 0 时按仓库日志风格打一条 debug/warn，或在 javadoc 中明确"<=0 表示不限制、负值按 0 处理"；补类级 javadoc 与方法级 javadoc（单位、取值范围、抛 IllegalArgumentException 的两种情形、异常最终会以 ExecuteResult.error 返回给调用方）。
| String sql = (String) params.get("sql"); | ||
| int batchSize = params.get("batchSize") != null ? (int) params.get("batchSize") : 1000; | ||
| execute(sql, list -> consumer.accept(new ExecuteResult().result(list)), batchSize); | ||
| int batchSize = SqlCommandParams.getInteger(params, "batchSize", 1000); |
There was a problem hiding this comment.
[低] batchSize 的接受域与异常类型随 helper 悄然改变，且新 helper 未校验 batchSize > 0
(int) params.get("batchSize") 换成 SqlCommandParams.getInteger(params, "batchSize", 1000) 是改进（字符串 "1000"、JSON 里变成 Long 的整数现在都能用，之前直接 ClassCastException），但它同时改变了既有参数的契约：小数（如 1000.5）从 ClassCastException 变成 IllegalArgumentException，最终以 ExecuteResult.error 呈现给调用方，属于未在 PR 描述中记录的对外行为变化。更值得顺手补的是范围校验：getInteger 不限制 batchSize > 0，而 execute 用 if (list.size() == batchSize)（:1169）触发 flush、用 batchSize > 0 ? batchSize : 1000（:1156）设 fetchSize——传入 batchSize = 0 时 fetchSize 变成 1000 但永远不会 flush，整个结果集堆积在内存里（此问题既有，但新 helper 正是加校验的最自然位置）。
建议：在 SqlCommandParams 里对 batchSize 增加 > 0 校验（或显式回落默认值并 warn），并在 PR 描述中记录"数字型字符串/Long 现在被接受、小数报错"这一行为变化。
| try (Connection connection = jdbcContext.getConnection(); | ||
| Statement sqlStatement = connection.createStatement()) { | ||
| if (queryTimeoutSeconds > 0) { | ||
| sqlStatement.setQueryTimeout(queryTimeoutSeconds); |
There was a problem hiding this comment.
[低] 流式读取下的超时语义未经验证，且超时同样作用于逐行 fetch 阶段
execute 完整保留了 setFetchSize(Integer.MIN_VALUE) 的流式语义（:1232），这点是对的；但 Connector/J 的 setQueryTimeout 是靠独立连接下发 KILL QUERY 实现的，而流式结果集的"语句执行期"包含逐行 fetch，因此一张大表的长时间流式扫描也会在超时到点后被中断——对"防止慢 SQL"是期望行为，对"大表导出"则是行为变更，需要明确语义而不是让用户踩坑。现有证据只有 Mockito 桩（verify(statement).setQueryTimeout(8)），既证明不了真实 MySQL 上超时确实生效，也证明不了流式 fetch 时的实际表现，更没有覆盖"超时后 connection.commit() 未执行、连接被正确清理"的路径。
建议：补一条真实 MySQL IT（SELECT SLEEP(10) + queryTimeoutSeconds=1 应抛 SQLException，并断言连接被关闭），并加一条大表流式读取触发超时的用例；在 execute 上注释"超时同时覆盖 fetch 阶段，大表全扫请设为 0 或足够大"。
AI PR Review — ❌ 不可通行
问题清单高
中
低
最终总结我先核对仓库实际代码，再逐条裁定两位 reviewer 的发现。 仲裁摘要两位 reviewer 合计 23 条（codex 3 条 + claude 20 条），去重后 12 条采纳、11 条驳回。 唯一确定阻塞项： 其余采纳项： 驳回的 11 条均有代码证据： 下一步必须做的事
已采纳（12 条）高（1）
中（4） 低（7） 已驳回（11 条，含证据）
FINAL_VERDICT: BLOCK codexcli reviewFindings
VERDICT: BLOCK claudecli reviewI'll conduct a comprehensive review of PR #954, examining all changed files for correctness, compatibility, security, and edge cases. Critical Issues1. [P1] SqlExecutor default method silently ignores timeout parameterFile: The new 6-parameter method has a default implementation that delegates to the old method without applying the timeout: default void execute(..., int queryTimeoutSeconds) throws Throwable {
execute(sql, connectionSupplier, consumer, aliveSupplier, batchSize);
}Impact: Any existing custom Fix: The interface should either be abstract (forcing implementations to handle it), or the default should throw 2. [P1] Paimon applies 300-second timeout by default, breaking existing queriesFile: long seconds = positive(values, "queryTimeoutSeconds", 300, Integer.MAX_VALUE);When Impact: Existing queries that legitimately take longer than 5 minutes will start failing. Fix: For backward compatibility, the default should match SQL standard behavior where absent/0 means unlimited. Consider defaulting to 3. [P1] Paimon maxResultRows default of 1,000 breaks existing queriesFile: maxResultRows = (int) positive(values, "maxResultRows", 1000, Integer.MAX_VALUE);Queries returning more than 1,000 rows will now fail with Impact: Fix: Either default to a much higher value (e.g., 1,000,000) or detect legacy queries ( 4. [P1] No way to specify "unlimited" timeout or results in PaimonFile: if (value <= 0 || value > max) { throw new ArithmeticException("out of range"); }The validation rejects zero and negative values, so there's no way to specify "no timeout" or "unlimited results." This violates SQL conventions where Fix: Treat High Severity5. [P2] Missing test coverage for zero/negative/absent timeout valuesFile: The test only verifies
This is the critical contract for backward compatibility but has no test coverage. 6. [P2] Aggregate COUNT overflow throws unhelpful ArithmeticExceptionFile: count = Math.addExact(count, 1);
Fix: Catch and wrap with context: 7. [P2] Paimon timeout doesn't interrupt blocked I/OFile: Documentation states: "超时不能强制中断已阻塞的 Paimon/文件系统 I/O" (timeout cannot interrupt blocked Paimon/filesystem I/O). This means Impact: Users may expect a 300s timeout to guarantee termination, but a hung S3 read could block indefinitely. Fix: Document this prominently in English docs and consider adding Medium Severity8. [P3] SqlCommandParams.getInteger allows fractional inputs to roundFile: return new BigDecimal(value.toString()).intValueExact();
Fix: Catch and provide clearer message: "Parameter 'batchSize' must be a whole number, got: 1.5" 9. [P3] CommonDbConnector.execute overload missing @OverRide annotationFile: protected void execute(String sql, Consumer<Object> consumer, int batchSize, int queryTimeoutSeconds) throws Throwable {Missing 10. [P3] PaimonQueryPlan allows duplicate result column labels in edge caseFile: String label = expressionItem.getAlias() == null ? expression.toString()
: PaimonSqlQueryParser.unquote(expressionItem.getAlias().getName());
if (!labels.add(label)) {
throw unsupported("duplicate result label; assign unique aliases: " + label);
}For Fix: Add field names from 11. [P3] Timeout nanosecond calculation could overflow for edge caseFile: long seconds = positive(values, "queryTimeoutSeconds", 300, Integer.MAX_VALUE);
timeoutNanos = seconds * 1_000_000_000L;If 12. [P3] PaimonQueryLimits.check() doesn't handle clock going backwardsFile: void check() throws SQLTimeoutException, InterruptedException {
if (Thread.currentThread().isInterrupted()) { throw new InterruptedException(...); }
if (clock.getAsLong() - started >= timeoutNanos) {
throw new SQLTimeoutException(...);
}
}If the system clock is adjusted backwards (NTP correction, manual change), Fix: Use 13. [P3] Missing validation that Paimon query limits fit in memoryFile: maxGroups = (int) positive(values, "maxGroups", 100_000, Integer.MAX_VALUE);
maxResultRows = (int) positive(values, "maxResultRows", 1000, Integer.MAX_VALUE);User can set Recommendation: Document max practical limits or add a sanity check (e.g., Low Severity / Polish14. [P4] Inconsistent null handling in PaimonQueryPlan.orderIndexFile: for (int i = 0; i < items.size(); i++) {
if (items.get(i).expressionSql.equalsIgnoreCase(expression.toString()) || items.get(i).label.equals(expression.toString())
|| expression instanceof Column && items.get(i).label.equals(
PaimonSqlQueryParser.unquote(((Column) expression).getColumnName()))) { return i; }
}If 15. [P4] PaimonService.executeQuery swallows allowFullScan from connection config when params is nullFile: public Object executeQuery(String sql, Map<String, Object> params, Log log) throws Exception {
Map<String, Object> options = new java.util.HashMap<>();
if (params != null) { options.putAll(params); }
options.putIfAbsent("allowFullScan", config.getQueryAllowFullScan());If 16. [P4] Misleading variable name
|
| 原发现 | 驳回理由 |
|---|---|
| claudecli#4 / codex"无法指定 unlimited" | positive() 接受至 Integer.MAX_VALUE，可显式关闭限制；真正问题是默认值（已并入 [高]1、[中]2） |
| claudecli#6 COUNT 溢出报错无上下文 | Math.addExact 需 9.2e18 次自增才溢出；maxScanRows 默认 10M 先拦下，实际不可达 |
claudecli#9 execute 重载缺 @Override |
MysqlConnector.java:1225 明确有 @Override |
claudecli#10 SELECT * 后跟表达式漏检重复标签 |
star 分支前置条件为 getSelectItems().size() == 1；SELECT *, id 走 else 分支直接抛 "only column expressions and aggregates are supported" |
| claudecli#11 nanoTime 计算溢出 | 2,147,483,647 × 1e9 = 2.1e18 < 9.2e18；reviewer 自己也标注"Not an issue" |
| claudecli#12 时钟回退导致超时不触发 | 生产路径 clock 默认 System::nanoTime（单调），可注入 clock 仅用于包内测试构造器 |
claudecli#13 maxGroups 无内存上限校验 |
文档已明示"行数/分组数限制不等于精确内存字节上限"，默认值有界（100k/1000），调大是运维显式动作 |
claudecli#14 orderIndex 潜在 NPE |
jsqlparser Expression.toString() 非空，Item.label/expressionSql 构造时均非 null |
claudecli#15 params=null 与 allowFullScan 合并语义微妙 |
无缺陷：HashMap.putIfAbsent 在该场景行为确定，params 优先级正确且已有测试覆盖 |
| claudecli#16 测试未用别名 | 该用例刻意使用客户原始无别名 SQL 并断言默认标签，与文档"建议为所有表达式指定唯一别名"一致 |
claudecli#17 spec.json 新增字段默认 false |
该字段只影响新式查询；若新式查询本身即本 PR 引入则非回归，相关风险已并入 [中]4 |
claudecli#20 finally 中 elapsedMillis() 可能抛错 |
方法体只有减法与整除，生产时钟 System::nanoTime，不会抛出 |
FINAL_VERDICT: BLOCK
===FINDINGS===
[高] connectors/paimon-plus-connector/src/main/java/io/tapdata/connector/paimon/service/PaimonQueryLimits.java:26
queryTimeoutSeconds 默认 300 秒，经既有 2 参重载静默作用于所有 Paimon executeQuery 调用
本行把新增超时参数的默认值定为 300 秒，而 PaimonService.java:3135 的 executeQuery(String sql, Log log) 传入 Collections.emptyMap()，因此所有既有调用（包括历史 SELECT * 与 COUNT 包装）都会拿到 5 分钟硬截止。check() 命中后抛 SQLTimeoutException("...no partial result returned")，原本能跑完的 5 分钟以上查询从此稳定失败，且调用方无从得知默认值存在；异常信息也没有提示可用 params.queryTimeoutSeconds 调大。两位 reviewer 独立命中同一行同一行为。
建议：默认改为"不设超时"（absent 或 0 视为无限制，与 SqlCommandParams.getQueryTimeoutSeconds 一致），仅在调用方显式传正值时启用；如需保留默认值，至少在 2 参重载中保持旧行为，并在 SQLTimeoutException 中给出当前值与调大方式。
[中] connectors/paimon-plus-connector/src/main/java/io/tapdata/connector/paimon/service/PaimonQueryLimits.java:25
maxResultRows 默认 1000（配合 maxScanRows 默认 1000 万）使既有结果集直接失败
maxResultRows 默认 1000，PaimonQueryPlan.java:256 在下一行即 throw limits.exceeded("maxResultRows", ...)；PaimonService.java:3194 同理会因 maxScanRows 默认 10,000,000 抛错。这些上限由 executeQuery(sql, log) 无条件下发，docs/paimon_zh_CN.md:178 也承认"历史 SELECT *…仍应用下面的资源限制"。因此超过 1000 行的既有 SELECT */投影查询、超过 1000 组的既有聚合、以及超过 1000 万行的计数查询都会从"成功返回"变成"整次命令失败、不返回部分结果"。请确认这些上限是否由本 PR 引入：若非本 PR 引入，则本条为确认项；若是，则属未在兼容性说明中体现的破坏性变更。
建议：默认值改为不限制（或显著提高并在 changelog 与英文文档中写明破坏性变更），或仅在调用方显式传参时启用上限；文档与错误信息需说明既有调用路径同样受限。
[中] connectors-common/sql-core/src/main/java/io/tapdata/common/SqlExecutor.java:16
新增的 6 参默认方法静默丢弃 queryTimeoutSeconds
默认实现只调用 5 参旧方法，queryTimeoutSeconds 被完全忽略且不报错。本仓库内唯一实现 DefaultSqlExecutor 已正确覆写，所以运行时暂时无感；但 SqlExecutor 是 sql-core 对外公开的接口，任何未覆写（或今后新写的）实现都会在主流程上"接受超时参数却不生效"，调用方以为限时已生效，属于典型的静默失效契约。这与本 PR "支持 SQL 查询超时"的目标直接冲突。
建议：默认实现改为抛 UnsupportedOperationException（附明确说明），或直接把 6 参方法设为抽象方法强制实现；至少在 Javadoc 中标注"默认实现忽略超时"。
[中] connectors/paimon-plus-connector/src/main/java/io/tapdata/connector/paimon/service/PaimonService.java:3159
新式查询默认要求键过滤，queryAllowFullScan 对旧连接取 false，可能拒绝既有查询
!queryPlan.legacyRows() && !limits.allowFullScan && !hasPruningFilter(...) 才放行。legacyRows() 仅覆盖 star && sorts.isEmpty() && !aggregate，因此不带键过滤的普通投影（SELECT a, b FROM t）、聚合、排序查询一律抛 IllegalArgumentException。opt-in 来源是新增连接配置 queryAllowFullScan，PaimonConfig.java:108 与 spec.json:32 的默认值都是 false，旧连接（配置里没有该字段）加载后即为 false，原先可执行的这类查询会开始报错。相关说明只写在 paimon_zh_CN.md:178。若该门禁本身即本 PR 引入，则需在兼容性说明中明确。
建议：确认门禁是否本 PR 引入；若是，请给出旧连接的迁移路径（如首次命中时提示开关位置、或在 changelog 中列为破坏性变更），并补齐 en_US / zh_TW 文档。
[中] connectors/mysql-connector/src/test/java/io/tapdata/connector/mysql/MysqlConnectorTest.java:68
新特性在 sql-core 侧没有任何单测，只验证了"传 8 秒会调用 setQueryTimeout"
本例是唯一覆盖超时链路的测试，且只覆盖 queryTimeoutSeconds = 8 一个点。以下关键契约全部无覆盖：SqlCommandParams.getQueryTimeoutSeconds 对 absent/0/负数/> Integer.MAX_VALUE/小数/非数字的解析与报错（该包内类没有任何测试文件，connectors-common/sql-core/src/test 下 grep setQueryTimeout|execute\( 零命中）；execute(..., 0) 与省略参数时不得调用 setQueryTimeout；SqlExecuteCommandFunction:77-80 把参数透传到 6 参方法；CommonDbConnector:1129-1131 同样路径。这正是向后兼容的边界，却没有锁定。
建议：为 SqlCommandParams 增加参数化单测，为 MysqlConnector.execute(sql, consumer, batchSize) 与 (..., 0) 增加"不调用 setQueryTimeout"断言，并为 SqlExecuteCommandFunction 增加透传测试；同时增加 Paimon executeQuery(sql, log) 2 参重载在默认配置下的回归用例。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/SqlCommandParams.java:34
负数/0 被静默当作"无限制"，与 Paimon 对同一参数的语义冲突
此处 seconds <= 0 直接返回 0（无限制），即传入 -1 这类明显笔误也不会报错；而 Paimon 的 PaimonQueryLimits.positive() 对 0 和负数抛 IllegalArgumentException("Parameter 'queryTimeoutSeconds' must be an integer between 1 and ...")。同一个平台命令名 executeQuery、同一个参数名，在 SQL 连接器上"忽略"，在 Paimon 上"报错"，运维与上层调度很难写出通用参数。
建议：统一语义。推荐两边都接受 0 表示无限制、负数报参数错误；或至少在 Paimon 的中文/英文文档中并列说明这一差异。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/SqlCommandParams.java:17
intValueExact 的报错信息无法说明"小数非法"
new BigDecimal("1.5").intValueExact() 抛出的 cause 是 ArithmeticException: Rounding necessary，而对外消息只有 "Parameter 'batchSize' must be a 32-bit integer"，用户看到 batchSize=1.5 时无法判断是被截断还是被拒绝。
建议：先判断 stripTrailingZeros().scale() > 0 或捕获后区分消息，例如 "Parameter 'batchSize' must be a whole number, got: 1.5"。
[低] connectors/paimon-plus-connector/src/main/java/io/tapdata/connector/paimon/service/PaimonQueryLimits.java:37
协作式超时无法中断已阻塞的 I/O，用户容易误以为超时等于硬性终止
check() 只在 catalog/规划/批量/行/结果等边界轮询，标题注释也写明 "cooperative at ... boundaries"。一旦某次 S3/HDFS 读或 catalog.getTable 卡住，超时不会触发，queryTimeoutSeconds 无法给出时间上界。该限制目前只写在 paimon_zh_CN.md:192 的一句中文里。
建议：在英文与繁体文档同步该限制，并在 SQLTimeoutException 的消息或参数说明中明确"协作式截止时间，不中断阻塞 I/O"；如需真正取消，考虑向支持中断的 I/O 层传播 Thread.interrupt()。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/CommonDbConnector.java:1129
新增 queryTimeoutSeconds 参数在 SQL 连接器侧没有任何文档或 spec 描述
executeCommand 现在读取 params.queryTimeoutSeconds（SqlExecuteCommandFunction:77 同），但 mysql 等连接器的 docs/mysql_en_US.md、mysql_zh_CN.md、mysql_zh_TW.md 与 spec.json 中都没有该参数的说明（全仓库 grep queryTimeoutSeconds 只在 Paimon 中文文档出现），而 batchSize 同样是命令参数，用户无从发现新能力。
建议：在各 SQL 连接器文档中补充 executeQuery 命令参数表（sql/batchSize/queryTimeoutSeconds，含 0 或省略表示无限制、单位秒、驱动侧 Statement.setQueryTimeout 生效）。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/DefaultSqlExecutor.java:54
未处理驱动不支持 setQueryTimeout 的情况，可能让整条命令失败
Statement.setQueryTimeout 在 JDBC 规范中是可选的，部分国产库/代理驱动会抛 SQLFeatureNotSupportedException。当前实现未捕获，一旦调用方传了正值，整个 execute 直接失败，退化为"不能查询"而不是"无法限时"。
建议：捕获 SQLFeatureNotSupportedException 并降级为记录 warn 后继续执行（明确告知限时未生效），或在连接初始化时探测一次并缓存能力标记。
[低] connectors/mysql-connector/src/test/java/io/tapdata/connector/mysql/MysqlConnectorTest.java:48
测试名承诺"KeepsValueConversion"，实际只断言转换方法被调用
用例只 verify(connector).filterTimeForMysql(resultSet, metaData) 并把 convertedRow 固定为 mock 返回值，并未验证超时路径下 MySQL 的时间类型转换仍然正确，名称与断言强度不匹配，后续重构容易误以为已有保护。
建议：要么改名（如 executeWithTimeoutUsesMysqlStreamingPath），要么补一条真实 MetaData 驱动的转换断言（例如 TIME/TIMESTAMP 列）。
[低] connectors/paimon-plus-connector/src/main/resources/docs/paimon_zh_CN.md:176
新增的执行限制/超时章节只有中文，en_US 与 zh_TW 文档完全缺失
paimon_zh_CN.md 已扩到 10.7KB 并包含"执行限制"整节（含 queryTimeoutSeconds/maxScanRows/maxGroups/maxResultRows/allowFullScan 表格），而 paimon_en_US.md（4.1KB）与 paimon_zh_TW.md（3.8KB）grep executeQuery|queryTimeout|maxResultRows 零命中，非中文用户没有任何可用说明，而这一节恰恰描述了会让查询失败的新默认值。
建议：把该章节同步到 en_US 与 zh_TW 文档，至少补齐参数表与"限制触发即整次失败、不返回部分结果"这两点。
===END===
标记：❌ 不可通行
- GROUP BY keys normalize -0.0 to 0.0 so both zeros share one group - ORDER BY comparison collapses signed zeros, matching the documented total order - add maxBufferedRows (default 100000) so ordered queries fail instead of buffering unlimited rows - update en/zh_CN/zh_TW docs and add regression tests
376baee to
1db6675
Compare
AI PR Review — ❌ 不可通行
问题清单高
中
低
最终总结我逐条核对了两位 reviewer 的发现，并做了实证验证（ 核对结论
必须做的事（合并前）
已接受的全部发现（17 项）高（阻断）
中 低 （第 18 项计入"低"，合计 18 条，前文"17 项"表述以清单为准。） 被驳回的发现无。两位 reviewer 的每一条具体意见都在 head（ FINAL_VERDICT: BLOCK codexcli (part 1/2) review
VERDICT: BLOCK claudecli (part 1/2) reviewPR #954 review, part 1 of 2 (head
|
| limits.check(); | ||
| Object[] result = new Object[items.size()]; | ||
| for (int i = 0; i < result.length; i++) { result[i] = state[i].result(); } | ||
| rows.add(result); |
There was a problem hiding this comment.
[高] 聚合 + ORDER BY 绕过 maxBufferedRows
finish() 的聚合分支只校验 states.size() > maxResultRows（默认 Integer.MAX_VALUE），随后把每组结果全部塞进 rows 再排序；只有非聚合的 add()（:275 附近）检查 maxBufferedRows。
实测（maxGroups=1000000, maxBufferedRows=2，SELECT region, COUNT(*) AS n FROM orders GROUP BY region ORDER BY region，3 组）：无异常返回 3 行。
默认值（各 100000）掩盖了问题，但文档明确承诺"带 ORDER BY 的查询缓存上限为 maxBufferedRows"，调大 maxGroups 即可突破该内存上限。
建议：在聚合物化前加 if (!sorts.isEmpty() && states.size() > limits.maxBufferedRows) throw limits.exceeded("maxBufferedRows", limits.maxBufferedRows);，并补 maxBufferedRows=2 + 3 组的回归测试。
| switch (root) { | ||
| case "TINYINT": value = number.byteValueExact(); break; | ||
| case "SMALLINT": value = number.shortValueExact(); break; | ||
| case "INTEGER": value = number.intValueExact(); break; |
There was a problem hiding this comment.
[高] 合法数值谓词被拒，并报成"invalid SELECT syntax"
DoubleValue 现在变成 BigDecimal，整数列走 byteValueExact/shortValueExact/intValueExact/longValueExact。WHERE id > 1.5、WHERE id < 3000000000（INT 列）是合法 SQL，却抛 ArithmeticException，被 parse() 的 catch (Exception) 包装为 Unsupported Paimon SQL: invalid SELECT syntax（实测确认）。
旧实现走 PredicateBuilder.convertJavaObject，实测 1.5→1、3000000000→-1294967296，即静默截断。拒绝比错谓语安全，但①这是行为回归，②用户看到的是"语法错误"，误导排障，③新测试把该行为固化为期望。
建议：优先精确改写比较（id > 1.5 → id >= 2；越界字面量 → 恒真/恒假）；至少把 ArithmeticException 单独捕获，抛出 literal not representable as INT 类明确错误，并在文档与测试中断言该消息。
| @Override | ||
| protected void execute(String sql, Consumer<Object> consumer, int batchSize, int queryTimeoutSeconds) throws Throwable { | ||
| JdbcQueryExecutor.execute(sql, jdbcContext::getConnection, consumer, this::isAlive, | ||
| batchSize, queryTimeoutSeconds, Integer.MIN_VALUE, resultSet -> { |
There was a problem hiding this comment.
[中] MySQL 流式读的超时覆盖范围需在文档说明
该路径 setFetchSize(Integer.MIN_VALUE)（Connector/J 流式结果集），而 Statement.setQueryTimeout 的计时绑定在语句执行上；execute() 返回后的逐行迭代时间可能不受该超时约束。mysql_* 文档却写成"正值通过 JDBC Statement.setQueryTimeout 生效"，读起来像整条命令有界。
建议：在 mysql 三语文档注明该限制（超时仅约束语句执行阶段，流式行读取可能超出），或对真实 MySQL 验证后写明实测结论。
|
|
||
| Projection, aggregation and sorted queries allow non-key filters and full scans by default. Set connection `queryAllowFullScan=false` (or command boolean `allowFullScan=false`) to opt into the partition/leading-primary-key filter requirement. Command options take precedence; direct/wrapped `COUNT(1)` remains exempt, while the gate also applies to historical unsorted `SELECT *`. For `AND`, any qualifying condition is enough; for `OR`, every branch must constrain an eligible key. Same-key equality alternatives can be rewritten as `IN`. Positive `IS NULL` on a key qualifies; `NOT` and negated forms do not. Use `allowFullScan=true` only when a full scan is intended. A supported filter is pushed down, but SQL syntax alone does not guarantee file pruning. | ||
|
|
||
| The 1000-row inspection limit belongs to the engine, not the connector. It counts result rows across callbacks and fails on row 1001, discarding all partial results. `executeCommand` uses its existing `batchSize` and result callback to stream unordered projections; a consumer exception stops reading and closes resources. Aggregation and sorting must finish scanning before final rows are delivered; input rows are not capped at 1000. Ordered queries buffer result rows in memory up to `maxBufferedRows`; group-by aggregation keeps one state per group up to `maxGroups`. Direct command callers can receive more than 1000 result rows. Streaming callers must discard already received batches if a later callback reports an error. |
There was a problem hiding this comment.
[中] 文档对 ORDER BY 缓冲上限的描述对聚合查询不成立
三语文档（en_US:129、zh_CN:183、zh_TW:128）均称"带 ORDER BY 的查询会把结果行缓存到内存，上限为 maxBufferedRows"，但 GROUP BY … ORDER BY 实际只受 maxGroups 约束（见 PaimonQueryPlan.java:344 的检查缺口）。修复 M1 后文档才成立；否则应改写为"聚合 + 排序受 maxGroups 约束"。
建议：与 M1 的代码修复一起落地，并在参数表 maxBufferedRows 行补一句"聚合查询按组缓存，受 maxGroups 约束"。
| assertEquals(3, streamed.drain().size()); | ||
| assertEquals(0, streamed.bufferedRows()); | ||
| // Grouped results are capped by maxGroups and keep one state per group, not one row per input. | ||
| PaimonQueryPlan.Accumulator grouped = plan("SELECT region, COUNT(*) AS n FROM orders GROUP BY region") |
There was a problem hiding this comment.
[中] 聚合缓冲上限的测试缺口
orderedQueriesBoundBufferedRowsWhileStreamingAndAggregatesDoNot 的聚合分支（:197-200）用的是无 ORDER BY 的 GROUP BY，因此永远不会走到 finish() 中缺失的检查；用例名还把"聚合不受 maxBufferedRows 限制"描述成预期行为，等于把 bug 写进了约束。
建议：修复 M1 时新增用例：maxBufferedRows=2，GROUP BY region ORDER BY region 覆盖 3 个分组，断言错误消息包含 maxBufferedRows。
| "SELECT id, SUM(id) FROM orders", | ||
| "SELECT SUM(id) OVER () FROM orders", | ||
| "SELECT id + 1 FROM orders", | ||
| "SELECT * FROM orders WHERE id = 1.5", |
There was a problem hiding this comment.
[中] 解析器测试把误导性报错固化为期望
新增的拒绝列表包含 WHERE id = 1.5、WHERE id = 2147483648，只断言 IllegalArgumentException 类型，因此"合法 SQL 被报成 invalid SELECT syntax"这个缺陷不会被测试发现；文档也只说支持"列与常量比较"，从未说明超范围字面量会被拒绝。
建议：随 M2 一起调整——若选择精确改写则把这些用例移出拒绝列表并断言改写后的语义；若保留拒绝，则断言消息（如 "not representable as INT"）并在三语文档写明规则。
|
|
||
| protected void execute(String sql, Consumer<Object> consumer, int batchSize, int queryTimeoutSeconds) throws Throwable { | ||
| if (queryTimeoutSeconds == 0) { | ||
| execute(sql, consumer, batchSize); |
There was a problem hiding this comment.
[低] 3 参/4 参 execute 重载可能互相递归直至栈溢出
基类 4 参在 timeout==0 时回调 execute(sql, consumer, batchSize)（虚拟分派）。若子类按"只重写 3 参、内部委托 4 参"的写法（execute(sql,c,b){ execute(sql,c,b,0); }），而自己没有重写 4 参，就会 3 参↔基类 4 参无限递归 → StackOverflowError。当前仓库内只有 MysqlConnector 重写且两个都重写了，所以尚未触发。
建议：在 4 参的 timeout==0 分支直接用 executeJdbcQuery(...)（仅在检测到 3 参被重写时才委派），或在方法注释/README 明确"3 参重写不得回调 4 参"的契约。
| // A subclass that implements the timeout overload can deliberately delegate to super. | ||
| boolean timeoutOverridden = false; | ||
| boolean legacyOverridden = false; | ||
| for (Class<?> type = getClass(); type != CommonDbConnector.class; type = type.getSuperclass()) { |
There was a problem hiding this comment.
[低] 每次正超时调用都用反射遍历类层级
getDeclaredMethod 逐层扫描的结果对同一个类永远不变，却在每次调用时重算；并且 private 方法或 bridge 方法同名也会被记为"已重写"，导致本应报 UnsupportedOperationException 的连接器静默走通用 JDBC 路径。
建议：用 ClassValue 缓存判定结果，并要求方法 modifier 非 private、非 bridge/synthetic。
| private Object value; | ||
| private State(Item item) { this.item = item; } | ||
| void add(Object next) { | ||
| if (item.function == null) { value = next; return; } |
There was a problem hiding this comment.
[低] 分组浮点列的展示值依赖扫描顺序
groupKey 已把 -0.0/+0.0 归一化到同一分组，但普通分组列的 State.add 直接 value = next（最后到达者胜出）。同一分组内同时出现 -0.0 与 0.0 时，展示哪个符号取决于扫描顺序，不影响相等性，但会造成跨端 diff 噪声。
建议：对浮点列在存储时同样归一化（如 value = groupKey(next) 或统一存 +0.0），并补一条断言。
| throw unsupported("ORDER BY ordinal is out of range"); | ||
| } | ||
| for (int i = 0; i < items.size(); i++) { | ||
| if (expression instanceof Column ? aliasMatches(items.get(i).label, ((Column) expression).getColumnName()) |
There was a problem hiding this comment.
[低] ORDER BY 匹配忽略列表限定符
orderIndex 只用 getColumnName() 做别名匹配，因此 SELECT col FROM t ORDER BY other_table.col 会静默匹配已选 col；而 fieldIndex 对其它表限定符是显式拒绝的，行为不一致。
建议：当 Column 带 table qualifier 时，先校验其等于当前表名，否则抛 unsupported("ORDER BY references another table: ...")，与 fieldIndex 保持一致。
| if (!sorts.isEmpty()) { | ||
| limits.check(); | ||
| Comparator<Object[]> comparator = (a, b) -> { | ||
| try { limits.check(); } |
There was a problem hiding this comment.
[低] 排序比较器每次比较都读时钟并检查中断
limits.check() 里含 clock.getAsLong()（超时开启时）与 Thread.interrupted 检查，被放在比较器内部，最多约 100k 行的排序会产生 O(n log n) 次系统调用级读取。
建议：按比较次数节流（例如每 1024 次比较检查一次），既保持取消响应性又显著降低开销。
| <source>${java.version}</source> | ||
| <target>${java.version}</target> | ||
| <!-- Existing integration tests use Java 11 APIs; production stays Java 8. --> | ||
| <testSource>11</testSource> |
There was a problem hiding this comment.
[低] 编译目标回退到 Java 8，但缺少 校验
删除重复的 compiler 插件后，生产代码从 Java 11 字节码回到 8；新增 testSource/testTarget=11 意味着构建需要 JDK 11+。当前主源码没有 Java 9+ API（已 grep List.of/isBlank/var/Stream.toList() 等），但 javac 未对 JDK 8 API 做校验，未来回归不会在编译期被发现。
建议：把 <source>/<target> 换成 <release>8</release>（测试用 testRelease 11），让编译期真正按 JDK 8 API 校验。
| - [Apache Paimon 文檔](https://paimon.apache.org/) | ||
| - [Paimon GitHub 倉庫](https://github.com/apache/paimon) | ||
|
|
||
| ## 自訂唯讀 SQL（`executeQuery`） |
There was a problem hiding this comment.
[低] "限制"章节仍称只支持写入，与新增 executeQuery 章节矛盾
第 89 行（en_US:90、zh_CN:110 同）写"此連接器目前僅支持寫入操作。不支持讀取和 CDC 操作"，而紧随其后的 executeQuery 章节（:122 起）详细描述了只读 SQL 查询。
建议：把该条收窄为"不支持 CDC/流式读取（仅支持只读 ad-hoc 查询）"，三语同步。
|
|
||
| | 参数 | 默认值 | 含义 | | ||
| | --- | --- | --- | | ||
| | `queryTimeoutSeconds` | 0 | 非负整数秒；省略或 0 不设超时，正值在 catalog/规划/批次/行/排序/结果阶段检查协作式截止时间 | |
There was a problem hiding this comment.
[低] 三语参数表在 queryTimeoutSeconds 行不一致
en_US（:135）与 zh_TW（:134）写明范围 0..2147483647，zh_CN 只描述协作式检查点而缺范围。
建议：zh_CN 补齐范围与"JDBC 上界一致"的说明，使三语参数表一致。
| @ValueSource(strings = {"UTC", "Asia/Shanghai", "America/Los_Angeles"}) | ||
| void realPaimonSelectStarPreservesTemporalJavaTypes(String zone) throws Exception { | ||
| java.util.TimeZone previous = java.util.TimeZone.getDefault(); | ||
| java.util.TimeZone.setDefault(java.util.TimeZone.getTimeZone(zone)); |
There was a problem hiding this comment.
[低] 测试可在失败时污染 JVM 时区
realPaimonSelectStarPreservesTemporalJavaTypes 在 try 之前调用 TimeZone.setDefault(...) 与 new PaimonService(...)；若构造或 config() 抛异常，finally 不执行，时区会残留给同 JVM 的后续测试。兄弟用例 executesCustomerDateAggregation...（:164）已把这两步放进 try。
建议：把 setDefault 之后的 new PaimonService(...) 移入 try，或用 try/finally 包住整个方法体。
| SQLTimeoutException failure = assertThrows(SQLTimeoutException.class, accumulator::finish); | ||
| assertTrue(failure.getMessage().contains("queryTimeoutSeconds=1")); | ||
| // Constructor, finish, pre-sort and first comparator: expiry occurs during comparison. | ||
| assertEquals(1_600_000_000L, ticks.get()); |
There was a problem hiding this comment.
[低] 超时测试依赖精确的时钟读取次数
assertEquals(1_600_000_000L, ticks.get()) 只有在恰好 4 次 check() 时才成立；一旦按建议给比较器节流（或新增检查点），行为仍正确但测试会误报。
建议：只断言抛出 SQLTimeoutException 且消息含 queryTimeoutSeconds=1，并断言比较器确实执行过（例如计数器 > 0），不绑定 tick 具体数值。
- Bound materialized groups for GROUP BY ... ORDER BY by maxBufferedRows, and cover the gap with a regression test that previously returned rows silently. - Report fractional or out-of-range integer literals as "not representable as <type>" instead of wrapping them in "invalid SELECT syntax"; drop the now unreachable Double/DECIMAL literal branches and assert the message in tests. - Normalize signed zeros for grouped float columns so the displayed sign does not depend on scan order; reject ORDER BY references to another table. - Sample the sort deadline (first comparison, then every 1024) instead of reading the clock on every comparison, and stop pinning the tick count. - Cache execute-override detection per class, ignore private/bridge methods, and fail with a clear error when the legacy overload delegates back instead of overflowing the stack. - Keep the deprecated SqlExecuteCommandFunction path on the streaming path so queryTimeoutSeconds is no longer silently dropped. - Compile production code with <release>8</release> (testRelease 11) so Java 9+ APIs fail at build time. - Document the MySQL streaming timeout scope, ordered aggregate buffering and integer literal rules in all three languages; fix the stale "write only" limitation and the incomplete zh_CN timeout range; move PaimonService construction inside the integration test's try block.
|
| 根源 | 典型 finding（轮次） | 本质 |
|---|---|---|
① 公共 SPI 形状变更：SqlExecutor 新增 6 参 default 方法、CommonDbConnector 3/4 参互调、DefaultSqlExecutor 5/6 参、废弃 executeCommand 改道 |
R1/R2/R3/R5/R6 的"丢弃超时"、R3/R8 的"3 参递归"、R7/R9 的"只覆写 5 参被子类被绕过"、R9 的"空结果集不再回调" | 想在不破坏子类覆写的前提下加超时，最后演化成 ClassValue 反射探测 + ThreadLocal 递归守卫 + UnsupportedOperationException 契约（CommonDbConnector.java:1152-1204）—— 这套机制本身就是"接口形状不对"的症状 |
② 兼容性用 SQL 形状启发式表达：PaimonQueryPlan.legacyRows() = star && sorts.isEmpty() && !aggregate（:250） |
R4 高"SELECT * 返回 Java 类型变了"、R7 高"±0.0"、R7 中"COUNT(*) vs COUNT(1) 语义分裂"、R9 中"SELECT * 含复杂列从可用变失败" |
一个布尔同时决定扫描限额（PaimonService.java:3175 的 forLegacyQuery()）、全表门禁豁免、返回形状与 Java 类型三件正交的事。任何一件要改，就会从另外两件的表象上冒出来 |
③ 限额/超时语义没有单一权威：默认值、0 的含义、上限优先级 |
R1/R2/R3 高"300s 默认作用于所有既有查询"、"1000 行默认让既有结果直接失败"、R8 高"聚合+ORDER BY 绕过 maxBufferedRows"、R3 低"batchSize 接受 0/负数" |
同一参数在 sql-core 与 Paimon 两侧语义相反或缺失；SqlCommandParams.getInteger 对 <=0 静默回落，getQueryTimeoutSeconds 却显式报错（SqlCommandParams.java:17 vs :28）—— 同一个类里两套口径 |
| ④ 一个 PR 装了两个产品级变更 | 全部 | sql-core 超时管道（影响所有 SQL 连接器）+ paimon 自建 SQL 引擎（560 行 plan + 624 行 parser + 102 行 limits + 三语文档 ≈ 3.6k 行） |
核心两难
executeQuery对外承诺的是哪一种东西？
- 若是薄透传（"你把 SQL 给我，我按基线那样把行给你"），那它的契约必须逐位不变：
SELECT *的 Java 类型、空结果集回调次数、子类覆写点、未加限制的扫描——一条都不能动。- 若是有界 SQL 引擎（"我只支持这个子集，有上限、有超时、类型有映射表"），那它必须有自己的显式契约和退出通道，旧的透传语义要能被关掉。
当前代码选了第三条路：不声明，用 legacyRows() 这个形状谓词在运行时猜。这解释了为什么 R4 已经因为 SELECT * 类型变化被阻断，R9 又因为 SELECT * 复杂列再被阻断一次——同一处模糊地带，两次不同的侧面。
三个可选方向
方案 A：拆 PR + 显式开关（params.queryEngine=legacy|semantic，默认 legacy）
做什么
- PR-1（sql-core，约 150 行）：不做 SPI 形状变更。
JdbcQueryExecutor保留为新工具类，但只被新增的 opt-in 方法使用；execute(sql, consumer, batchSize)与 4 参重载的现有分派语义原样不动，删掉ClassValue探测与ThreadLocal守卫；废弃executeCommand完全不动（保持"恰好一次回调、空结果为[]"）。 - PR-2（paimon）：
executeQueryAdmitted入口按queryEngine分支。legacy= 基线convertInternalRowToMap整行路径 + 仅加maxScanRows/超时/流式；semantic才走PaimonQueryPlan。legacyRows()启发式删除。
优点：既有一切行为 byte-identical（R4/R7/R9 三类兼容性 finding 直接消失）；反射+ThreadLocal 那套机制不需要存在；两个 PR 各自可独立评审合入。
代价：两条路径要同时维护（行转换双实现）；新能力默认关闭，需要 spec 增加一个字段并由使用方打开。
代码影响：PaimonService.executeQueryAdmitted 增加一个分支；CommonDbConnector 减少约 50 行（删探测机制）；spec.json 三语各加一个枚举字段。
方案 B：单个 PR，但把契约写成单点分类（QueryKind 枚举）
做什么
PaimonSqlQueryParser.parse结束时产出显式QueryKind（LEGACY_ROW/BOUNDED_ENGINE/COUNT/STAR_LEGACY）并写进ParsedQuery；限额、门禁豁免、返回形状、返回 Java 类型只读这一个枚举，不再散落。SELECT *含复杂列 → 回退convertInternalRowToMap，不再报错（消除唯一的硬兼容破坏）。- sql-core 只保留 5 参作为唯一扩展点，删掉 6 参 default 方法与探测机制；超时走
SqlCommandParams已解析好的值，在构造期决定。 - 新增一节契约文档：支持语法、列类型→Java 类型映射表、回调次数、多结果集、限额优先级、超时语义（含"不可中断 I/O"）。
- 统一
SqlCommandParams对0/负数的口径（batchSize/maxResultRows/queryTimeoutSeconds一致）。
优点：无新开关，用户可见行为一次说清；反射机制消失；三语文档有唯一权威来源。
代价：仍是 ~3.6k 行单一 diff，评审疲劳不会消失；必须一次性接受"新语义替代旧语义"（COUNT(1) 的返回类型、SELECT * 新引擎路径的类型映射）；没有回退通道，线上遇到回归只能改代码。
方案 C：本 PR 只交超时+限额，聚合能力单独走后续
做什么：executeQuery 保持 convertInternalRowToMap；只加 maxScanRows / queryTimeoutSeconds / batch 流式 / allowFullScan 门禁；GROUP BY/ORDER BY/聚合表达式一律"明确不支持"；聚合下推（Paimon compute 或客户端聚合）另开设计与 PR。
优点：9 轮 finding 里约 80% 直接消失；PR 的原始目的（JDBC 超时）立刻可合；契约面最小。
代价：需求方要的"聚合自定义 SQL"落不了地；已写的 1400 行 plan/parser 变 dead code 或另起 PR；需要在需求侧确认时间线。
我的建议
A 为主，B 作为 PR-2 的目标形态。 理由：本 PR 的 9 轮里，真正卡住合入的从来不是算法正确性，而是"改动了未声明的既有行为"。方案 A 把这类风险一次性归零（默认路径不变），同时让 PR-1 在 1 轮内可结束；PR-2 再用 B 的单点分类把契约固化，避免第五类根源在下一轮重现。
C 是风险最低但可能不满足需求的兜底；若需求方接受"聚合先不做"，它的投入产出比其实最高。
需要作者/需求方确认的 3 个事实
- 聚合自定义 SQL 有真实用户在等吗？ 若时间线可接受"opt-in 且默认关闭"，选 A；若能延后，选 C。
- 现有
SELECT *用户中，含 ARRAY/MAP/ROW 列的表在用吗？ 这决定"从返回整行变为报错"能否作为可接受的破坏性变更（否则必须修成回退）。 queryTimeoutSeconds是产品级契约还是 Paimon 内部参数？ 若影响所有 SQL 连接器，它应单独走一次 spec 评审，不应和 Paimon 引擎绑在同一个 PR 里。
在方向确定前，我不建议再做增量修复 —— 上一轮 18 项全修完，本轮仍新增 7 项，其中 3 项是中级的兼容性回归，趋势没有收敛。
Next Steps: Please review the options above and clarify your design intent. Once the approach is agreed upon, the implementation can proceed more smoothly.
This comment was automatically generated after detecting a back-and-forth pattern.
AI PR Review — ❌ 不可通行
问题清单中
低
最终总结我会先核对仓库实际代码，再逐条裁定。 验证方式
上轮 18 项复核结论全部 18 项已修复或脆弱性已消除，无一仍存在或部分残留：
本轮新发现（7 项）
被驳回的发现无。两位 reviewer 的具体意见均复核成立（仅对"基线能正确处理复杂列"这一理由做了修正，结论不变）。GitHub 上未见晚于基线 head 的新人工评论；此前评论提到的复杂列兼容性问题由上面第 1 条覆盖。 下一步
FINAL_VERDICT: BLOCK Design decision required: Review history shows an A→B→A finding cycle. Clarify the design choice before continuing. codexcli reviewFindings
上轮 18 项复核以下均为已修复；括号内为当前代码证据。
GitHub 上未发现晚于基线 head VERDICT: BLOCK claudecli reviewPR #954 增量审查（
|
| # | 问题 | 状态 | 代码证据 |
|---|---|---|---|
| 1 | 聚合 + ORDER BY 绕过 maxBufferedRows |
已修复 | PaimonQueryPlan.finish() 在物化前检查分组数（:348-352），并有 orderedAggregatesAlsoBoundMaterializedGroups 测试 |
| 2 | 合法整数谓词被报成语法错误 | 已修复 | 作者没有改写比较，而是给出明确报错，即上轮建议中的保底方案。narrow() 抛出 "not representable as …"（:526-538）。这个异常是 IllegalArgumentException，parse() 的 catch (IllegalArgumentException) 会原样抛出，不会再包装成语法错误 |
| 3 | 废弃的 executeCommand 丢弃超时 |
已修复，但引入了新问题 | 见新问题 N1 |
| 4 | MySQL 流式读超时的覆盖范围 | 已修复 | 三语文档已补充说明，但给出的配置建议有误，见 L2 |
| 5 | 文档的缓冲上限描述 | 已修复 | 三语文档都已说明聚合排序同时受两个上限约束 |
| 6 | 聚合缓冲上限缺测试 | 已修复 | 新用例为 maxBufferedRows=2、3 组，并断言错误消息 |
| 7 | 解析器测试固化了误导性报错 | 已修复 | 测试改为断言消息内容，并排除 "invalid SELECT syntax" |
| 8 | 3 参 / 4 参 execute 互相递归 |
已修复 | 用 ThreadLocal 守卫并明确报错（CommonDbConnector.java:1163-1173），有回归测试 |
| 9 | 每次调用都用反射扫描，且误判 private/bridge 方法 | 已修复 | 改用 ClassValue 缓存，并排除 private、bridge、synthetic 方法 |
| 10 | 字面量改动留下死分支 | 已修复 | 两处死分支都已删除 |
| 11 | 分组浮点列展示值依赖扫描顺序 | 已修复 | 新增 normalizeZero，分组键和 State.add 都经过它，测试断言 +0.0 |
| 12 | ORDER BY 忽略表限定符 |
已修复 | 限定符校验与 fieldIndex 一致（:145-151） |
| 13 | 排序比较器每次比较都读时钟 | 已修复 | 改为每 1024 次比较检查一次；计数器 int 溢出后 & 1023 仍然正确 |
| 14 | 缺少 <release> 校验 |
已修复 | 用 <release>${java.version}</release>（值为 8）加 <testRelease>11</testRelease>；主源码没有 sun.* 或 jdk.internal 依赖，--release 8 不会误报 |
| 15 | "仅支持写入"的说法自相矛盾 | 已修复 | 三语文档都已改为"不支持 CDC 和流式读取" |
| 16 | zh_CN 文档缺超时范围 | 已修复 | 已补上 0..2147483647 |
| 17 | 集成测试可能污染 JVM 时区 | 已修复 | 构造移入 try，finally 对 null 做了判断 |
| 18 | 测试绑定时钟读取次数 | 已修复，但新断言测不出问题 | 见 L1 |
新问题
[中] N1：废弃的 executeCommand 改走流式路径，返回结果的约定变了（SqlExecuteCommandFunction.java:39-43，JdbcQueryExecutor.java:51-60）
- 原来的行为： 调用
sqlExecutor.execute(sql, supplier)。每条命令正好回调一次：SELECT 回调一次完整的List<DataMap>，结果为空时是空列表；出错时回调一个 error 结果。 - 现在的行为： 走 6 参流式路径，有三处变化：
- 结果按
batchSize（默认 1000）拆成多次回调。 - SELECT 结果为空时一次都不回调（
if (!batch.isEmpty())）。旧版调用方如果等"恰好一个结果"，会挂起，或者当成没有返回。 - 多结果集会全部推出来；而自定义
SqlExecutor里改写过的废弃 2 参execute会被绕过。
- 结果按
- 影响范围： 仓库内的调用方（Hive、Hive1、Hudi、Databend、Snowflake）都用带
aliveSupplier的重载，不受影响；受影响的只有外部或遗留调用方。新测试deprecatedCommandAlsoPassesTimeoutToJdbc只覆盖了更新计数的路径，空 SELECT 和多批次都没测到。 - 建议： 废弃入口保留原来的单结果路径。超时为 0 时行为完全不变；为正值时，要么在旧路径里调用
setQueryTimeout，要么明确报错。
[中]【上轮漏报】SELECT * 遇到复杂类型列，从返回整行变成报错（PaimonQueryPlan.java:70-73、:177-178）
- 星号投影会对每一列调用
column()，而column()拒绝 ARRAY、MAP、MULTISET、ROW。 - 基线版本的
PaimonService.executeQuery（base 第 3172 行）用convertInternalRowToMap转换整行，可以处理这些列。 - 所以对含复杂列的表，原本能用的
SELECT *现在直接失败。文档写了这个限制，但这仍是兼容性回归。我同意 Codex 的结论。
[中]【上轮漏报】只改写 5 参 execute 的 DefaultSqlExecutor 子类会被绕过（DefaultSqlExecutor.java:45-60）
- 5 参方法委派给 6 参，而 6 参直接走 JDBC，不会回调 5 参。命令路径调用的是 6 参。
- 结果是：子类在 5 参里写的自定义转换不再生效。仓库内没有这样的子类，但现在废弃入口也走这条路径（N1），影响面变大了。
- 建议： 用与
CommonDbConnector相同的覆盖检测：超时为 0 时回调 5 参，为正值时明确报错。
[低] L1：超时测试的新断言测不出问题（PaimonQueryPlanTest.java:251-263）
- 构造函数读一次时钟，所以
ticks.get() > 0永远成立。 - 按每次 +400ms 推算：如果删掉比较器里的检查，排序结束后的
limits.check()恰好在第 4 次读时钟时（1.2s）到期，同样抛出SQLTimeoutException。因此测试名说的"排序中检查截止时间"其实没被验证。 - 建议： 在比较器触发检查的那次让时钟到期，并断言超时发生在排序之前（比如断言排序后的检查没有执行到），或者断言
SortCancelled这条路径确实走过。
[低] L2：MySQL 文档建议的超时参数不对（mysql_*.md:87）
- 它说可以配合"服务端
socketTimeout/net_read_timeout"来限制逐行读取。但socketTimeout是 Connector/J 客户端属性，不是服务端设置。 net_read_timeout控制服务端从客户端读数据，与流式结果的推送无关。真正相关的是net_write_timeout；Connector/J 的netTimeoutForStreamingResults调的就是它。batchSize也不限制时间。三语都需要更正。
[低] L3：zh_TW 文档的整型字面量说明放错了段落（paimon_zh_TW.md）
- 这条规则被写进了 ORDER BY 那段（"排序需將結果行快取…；整型欄位只接受…"）。en_US 和 zh_CN 都放在 WHERE 那段。
- 应移到下面"WHERE 支援…"那段。
[低] L4：递归守卫会误伤合法的嵌套查询（CommonDbConnector.java:1163-1166）
- 守卫以线程为单位。如果某个只改写 3 参的子类，在自己的方法里用 4 参（超时 0）再执行一个不同的查询，比如预查询，也会被当成递归报
IllegalStateException。 - 这种情况很少见，但报错信息会误导排查。建议在注释里写明这个契约。
已检查、没有发现问题
ClassValue的计算会沿类层级逐层往上，MySQL 的子类能继承到"已覆盖"的标记。finally里的remove()不会把外层调用设的标记残留下来。narrow()对1.0这类没有小数部分的值能正常转换；BETWEEN也走同一路径，测试覆盖了。normalizeZero现在把 Float 保留为 Float 键，不再转成 Double；同一列类型内相等性不受影响，NaN 仍然归为一组。- 父 POM 的
source/target会合并进来，但设置了release时 3.7.0 优先使用--release。
VERDICT: BLOCK
reasonix review
我会先核对仓库实际代码，再逐条裁定。
核查完毕，我逐条用仓库实际代码（head efcc6a52）验证了两位 reviewer 的意见，并运行了两组测试。
验证方式
mvn -o test -Dtest=PaimonQueryPlanTest,PaimonSqlQueryParserTest→ 25 用例全部通过（18 + 7）。mvn -o test -Dtest=SqlCommandTimeoutTest（sql-core）→ 13 用例全部通过。- 逐行核对
PaimonQueryPlan/PaimonSqlQueryParser/CommonDbConnector/SqlExecuteCommandFunction/DefaultSqlExecutor/JdbcQueryExecutor与三语文档，并与基线c132b49f（merge-base）对比行为。
上轮 18 项复核结论
全部 18 项已修复或脆弱性已消除，无一仍存在或部分残留：
| # | 状态 | 证据 |
|---|---|---|
| 1 | 已修复 | finish() 物化前检查 !sorts.isEmpty() && states.size() > maxBufferedRows（:348-352）；新增 orderedAggregatesAlsoBoundMaterializedGroups（2 行上限 + 3 组，实测通过） |
| 2 | 已修复 | narrow() 抛 literal ... is not representable as ...（:526-538），parse() 的 catch (IllegalArgumentException) throw e;（:96-97）不再包装成语法错误；测试断言消息且 assertFalse(contains("invalid SELECT syntax")) |
| 3 | 已修复（引出 N1） | SqlExecuteCommandFunction.java:42 转入共享路径，超时生效并有测试 |
| 4 | 已修复（文档建议有误 → L2） | mysql 三语文档补充流式读取超时范围 |
| 5 | 已修复 | paimon 三语 maxBufferedRows 行与正文均写明聚合+排序受两个上限约束 |
| 6 | 已修复 | 见 #1 |
| 7 | 已修复 | PaimonSqlQueryParserTest:117-129 |
| 8 | 已修复 | CommonDbConnector:1161-1173 ThreadLocal 守卫 + delegatingLegacyOverrideFailsClearlyInsteadOfOverflowingTheStack |
| 9 | 已修复 | ClassValue OVERRIDDEN + 排除 private/bridge/synthetic（:1181-1204） |
| 10 | 已修复 | 删除 -(Double) 与 convertLiteral 的 DECIMAL 分支（我核实两条确为不可达：typedLiteral 对 DecimalType 提前返回） |
| 11 | 已修复 | normalizeZero 同时用于 groupKey 与 State.add，保留 Float 类型，测试断言 +0.0 |
| 12 | 已修复 | orderIndex 校验表限定符（:143-151） |
| 13 | 已修复 | 每 1024 次比较采样，& 1023 对负值仍正确 |
| 14 | 已修复 | <release>${java.version}</release>(8) + <testRelease>11</testRelease>，Compiler Plugin 3.7.0 支持 |
| 15 | 已修复 | 三语"限制"改为"不支持 CDC/流式读取，只读查询用 executeQuery" |
| 16 | 已修复 | zh_CN 补 0..2147483647（与 JDBC 上界一致） |
| 17 | 已修复 | 构造移入 try，finally 判空 |
| 18 | 已修复（新断言无效 → L1） | 不再绑定 tick 精确值，但 assertTrue(ticks.get() > 0) 恒真 |
本轮新发现（7 项）
- [中]
SELECT *遇复杂列由"返回整行"变为整条命令失败：星号投影逐列调用column()，ARRAY/MAP/MULTISET/ROW直接抛错（:73→:177）。基线executeQuery走convertInternalRowToMap，同一行里的标量列原本是可用的（复杂列本身经default: row.getString(pos)得到的是无意义值，并非正确结构——两位 reviewer 的"基线可以处理这些列"表述略过头，但"查询从返回变为失败"成立）。文档已写限制，但这是兼容性行为变更，且无任何测试覆盖该拒绝路径。 - [中] 命令路径绕过只覆写 5 参
execute的DefaultSqlExecutor子类：6 参直接执行 JDBC（:52-60），命令路径调 6 参；仓库内无此类子类，外部子类的自定义行转换会静默失效。 - [中] 废弃
executeCommand改走流式路径，回调契约变了：JdbcQueryExecutor:58的if (!batch.isEmpty())意味着空结果集一次回调都不发（旧版必回调一次空列表）；结果还按batchSize拆分、多结果集全部推送、废弃 2 参execute覆写被绕过。新测试deprecatedCommandAlsoPassesTimeoutToJdbc只走了 DML 路径，空 SELECT/多批次均未覆盖。 - [低] 超时测试仍未验证"排序中检查"：
PaimonQueryLimits构造即clock.getAsLong()，ticks > 0恒真；我按 +400ms 推演，即便删掉比较器里的检查，排序后的limits.check()也会在第 4 次读取（1.2s）抛SQLTimeoutException，用例照样通过。 - [低] MySQL 文档给出的限流参数有误：
socketTimeout是 Connector/J 客户端连接属性而非服务端；net_read_timeout约束服务端读客户端；限制服务端推送流式结果的是net_write_timeout（Connector/J 的netTimeoutForStreamingResults即调它）。三语皆需更正。 - [低] zh_TW 把"整型字面量只接受整数值"写进了 ORDER BY 段，en_US/zh_CN 都放在 WHERE 段。
- [低] ThreadLocal 递归守卫会误伤合法嵌套查询：只覆写 3 参的子类在自己的方法里用 4 参（超时 0）执行另一条查询会被判为递归并抛
IllegalStateException，报错信息误导排查。
被驳回的发现
无。两位 reviewer 的具体意见均复核成立（仅对"基线能正确处理复杂列"这一理由做了修正，结论不变）。GitHub 上未见晚于基线 head 的新人工评论；此前评论提到的复杂列兼容性问题由上面第 1 条覆盖。
下一步
- 对含复杂列的表，
SELECT *改为"标量列照旧、复杂列报错或省略"的可选策略（或至少补一条拒绝路径测试并在文档写明迁移），消除兼容性回归。 DefaultSqlExecutor/ 废弃入口采用与CommonDbConnector相同的覆写探测：超时 0 走旧路径，正值明确报错；并用一条空 SELECT 用例把"恰好一次回调"的契约固定下来。- 让超时用例真正验证"排序中取消"（在比较器采样那次到期，或断言
SortCancelled路径走过）。 - 修正 MySQL 三语文档的
socketTimeout/net_read_timeout表述，把 zh_TW 的整型字面量规则移到 WHERE 段，并在递归守卫处写明契约。
FINAL_VERDICT: BLOCK
===FINDINGS===
[中] connectors/paimon-plus-connector/src/main/java/io/tapdata/connector/paimon/service/PaimonQueryPlan.java:73
【上轮漏报】SELECT * 遇复杂列由返回整行变为整条命令失败
星号投影对每列调用 column()，而 column() 在 177-178 行拒绝 ARRAY/MAP/MULTISET/ROW，因此含任一复杂列的表执行 SELECT * 会整条失败。基线 executeQuery 走 convertInternalRowToMap：复杂列本身是用 default: row.getString(pos) 转出的无意义值，但同一行里的标量列是可用的；本次改动把"部分可用"变成"完全不可用"，属于兼容性行为变更。三语文档虽已写明该限制，但没有任何测试覆盖这条拒绝路径，也没有给出参数化的兼容通道。建议：星号投影只物化标量列并对复杂列给出按列可选的错误/省略策略（或提供 params 开关回到旧行为），同时补一条 "SELECT * 含 ARRAY 列" 的解析测试，并在三语文档写明迁移步骤与错误信息。
[中] connectors-common/sql-core/src/main/java/io/tapdata/common/DefaultSqlExecutor.java:52
【上轮漏报】只覆写 5 参 execute 的 DefaultSqlExecutor 子类会被命令路径绕过
6 参方法直接调用 JdbcQueryExecutor.execute（52-60 行），不会分派到 5 参方法；5 参（44-47 行）只是委派到 6 参。命令路径（SqlExecuteCommandFunction 第 59 行）调用的是 6 参，因此外部子类若只覆写 5 参（本 PR 之前唯一的可覆写入口），其自定义行读取/转换逻辑会被静默绕过。仓库内未发现此类子类，但废弃入口本轮也转到了这条路径（见下一条），影响面比上轮更大。建议：套用 CommonDbConnector 的覆写探测——子类只覆写 5 参且超时为 0 时回调 5 参，超时为正值时抛 UnsupportedOperationException，并补一条"只覆写 5 参的子类仍生效"的测试。
[中] connectors-common/sql-core/src/main/java/io/tapdata/common/SqlExecuteCommandFunction.java:42
废弃 executeCommand 改走流式路径，结果回调契约变了（空结果集不再回调）
改动前：sqlExecutor.execute(sql, connectionSupplier)，每条命令恰好回调一次——SELECT 回调一次完整 List<DataMap>（空结果为空列表），出错回调 error 结果。改动后走 6 参流式路径：①结果按 batchSize（默认 1000）拆成多次回调；②JdbcQueryExecutor 第 58 行的 if (!batch.isEmpty()) 使得空 SELECT 一次回调都不发，等待"恰好一个结果"的旧调用方会挂起或误判为无返回；③多结果集全部推出，原先只取第一个；④自定义 SqlExecutor 里覆写的废弃 2 参 execute 被绕过。仓库内 Hive/Hive1/Hudi/Databend/Snowflake 都走 aliveSupplier 重载不受影响，受影响的是外部或遗留调用方。新测试 deprecatedCommandAlsoPassesTimeoutToJdbc 只走了 DML（statement.execute 返回 false）路径，空 SELECT 和多批次均未覆盖。建议：废弃入口保留单结果路径，超时为 0 时行为完全不变，为正值时在旧路径里调用 setQueryTimeout 或明确报错；并补空结果集与多批次两条回归测试。
[低] connectors/paimon-plus-connector/src/test/java/io/tapdata/connector/paimon/service/PaimonQueryPlanTest.java:263
超时用例的新断言测不出"排序中检查"
PaimonQueryLimits 构造函数里 started = clock.getAsLong() 已经读过一次时钟，所以 assertTrue(ticks.get() > 0) 恒真。按 +400ms 推演：finish 入口 check（0.4s）、pre-sort check（0.8s）后进入排序，比较器采样那次读到 1.2s 才到期；但即便删掉比较器里的检查，排序结束后的 limits.check() 同样会在 1.2s 抛 SQLTimeoutException，用例照样通过——用例名声称的"排序内检查截止时间"没有被验证。建议：让超时恰好在比较器采样那次到期（例如前几次 check 不推进时钟），并断言异常发生在排序完成之前，或断言 SortCancelled 路径确实被走过。
[低] connectors/mysql-connector/src/main/resources/docs/mysql_en_US.md:87
MySQL 文档建议的超时兜底参数不正确
文档写"配合服务端 socketTimeout/net_read_timeout"来限制逐行读取，但 socketTimeout 是 Connector/J 的客户端连接属性，不是服务端设置；net_read_timeout 控制服务端从客户端读数据，与推送流式结果无关；真正相关的是 net_write_timeout（Connector/J 的 netTimeoutForStreamingResults 即调它）。batchSize 也不提供时间上限。建议：三语文档改为"socketTimeout（Connector/J 客户端属性）/ net_write_timeout（服务端写超时，或连接串 netTimeoutForStreamingResults）"，并明确 batchSize 只影响回调粒度。
[低] connectors/paimon-plus-connector/src/main/resources/docs/paimon_zh_TW.md:145
zh_TW 把整型字面量规则放错了段落
"整型欄位只接受欄位範圍內的整數字面量…不會誤報為語法錯誤"被写进了 ORDER BY 那段，而 en_US（:148）与 zh_CN（:161）都放在 WHERE 段。建议：把该句移到下面"WHERE 支援…"那段，使三语结构与内容一致。
[低] connectors-common/sql-core/src/main/java/io/tapdata/common/CommonDbConnector.java:1163
递归守卫会误伤合法的嵌套查询
守卫是线程级标志（1167 行置位、1171 行 remove()）。只覆写 3 参的子类若在自己的方法里用 4 参（超时 0）执行另一条查询（例如会话预置语句或预查询），会被判为递归并抛 IllegalStateException，报错信息"must not delegate back"会误导排查方向。建议：把守卫改为"同一 SQL/同一调用栈帧"判定或使用重入计数并在超限时才报错，至少在该方法与 README 中写明"3 参覆写内不得再调用 4 参"的契约。
===END===
标记：❌ 不可通行
| if (item instanceof AllColumns && select.getSelectItems().size() == 1) { | ||
| plan.star = true; | ||
| for (int i = 0; i < rowType.getFieldCount(); i++) { | ||
| plan.items.add(new Item(rowType.getFieldNames().get(i), plan.column(i), null, rowType.getFieldNames().get(i))); |
There was a problem hiding this comment.
[中] 【上轮漏报】SELECT * 遇复杂列由返回整行变为整条命令失败
星号投影对每列调用 column()，而 column() 在 177-178 行拒绝 ARRAY/MAP/MULTISET/ROW，因此含任一复杂列的表执行 SELECT * 会整条失败。基线 executeQuery 走 convertInternalRowToMap：复杂列本身是用 default: row.getString(pos) 转出的无意义值，但同一行里的标量列是可用的；本次改动把"部分可用"变成"完全不可用"，属于兼容性行为变更。三语文档虽已写明该限制，但没有任何测试覆盖这条拒绝路径，也没有给出参数化的兼容通道。建议：星号投影只物化标量列并对复杂列给出按列可选的错误/省略策略（或提供 params 开关回到旧行为），同时补一条 "SELECT * 含 ARRAY 列" 的解析测试，并在三语文档写明迁移步骤与错误信息。
| @Override | ||
| public void execute(String sql, TapSupplier<Connection> connectionSupplier, Consumer<Object> consumer, | ||
| Supplier<Boolean> aliveSupplier, int batchSize, int queryTimeoutSeconds) throws Throwable { | ||
| JdbcQueryExecutor.execute(sql, connectionSupplier, consumer, aliveSupplier, batchSize, queryTimeoutSeconds, |
There was a problem hiding this comment.
[中] 【上轮漏报】只覆写 5 参 execute 的 DefaultSqlExecutor 子类会被命令路径绕过
6 参方法直接调用 JdbcQueryExecutor.execute（52-60 行），不会分派到 5 参方法；5 参（44-47 行）只是委派到 6 参。命令路径（SqlExecuteCommandFunction 第 59 行）调用的是 6 参，因此外部子类若只覆写 5 参（本 PR 之前唯一的可覆写入口），其自定义行读取/转换逻辑会被静默绕过。仓库内未发现此类子类，但废弃入口本轮也转到了这条路径（见下一条），影响面比上轮更大。建议：套用 CommonDbConnector 的覆写探测——子类只覆写 5 参且超时为 0 时回调 5 参，超时为正值时抛 UnsupportedOperationException，并补一条"只覆写 5 参的子类仍生效"的测试。
| executeResultConsumer.accept(executeResult); | ||
| // Keep the deprecated entry point on the same code path so queryTimeoutSeconds and batchSize | ||
| // are honored; legacy callers have no cancellation supplier, so the command never cancels. | ||
| executeCommand(sqlExecutor, tapExecuteCommand, connectionSupplier, () -> true, executeResultConsumer); |
There was a problem hiding this comment.
[中] 废弃 executeCommand 改走流式路径，结果回调契约变了（空结果集不再回调）
改动前：sqlExecutor.execute(sql, connectionSupplier)，每条命令恰好回调一次——SELECT 回调一次完整 List<DataMap>（空结果为空列表），出错回调 error 结果。改动后走 6 参流式路径：①结果按 batchSize（默认 1000）拆成多次回调；②JdbcQueryExecutor 第 58 行的 if (!batch.isEmpty()) 使得空 SELECT 一次回调都不发，等待"恰好一个结果"的旧调用方会挂起或误判为无返回；③多结果集全部推出，原先只取第一个；④自定义 SqlExecutor 里覆写的废弃 2 参 execute 被绕过。仓库内 Hive/Hive1/Hudi/Databend/Snowflake 都走 aliveSupplier 重载不受影响，受影响的是外部或遗留调用方。新测试 deprecatedCommandAlsoPassesTimeoutToJdbc 只走了 DML（statement.execute 返回 false）路径，空 SELECT 和多批次均未覆盖。建议：废弃入口保留单结果路径，超时为 0 时行为完全不变，为正值时在旧路径里调用 setQueryTimeout 或明确报错；并补空结果集与多批次两条回归测试。
| assertTrue(failure.getMessage().contains("queryTimeoutSeconds=1")); | ||
| // The exact number of clock reads is an implementation detail (the sort samples the | ||
| // deadline); only require that the deadline was actually polled. | ||
| assertTrue(ticks.get() > 0); |
There was a problem hiding this comment.
[低] 超时用例的新断言测不出"排序中检查"
PaimonQueryLimits 构造函数里 started = clock.getAsLong() 已经读过一次时钟，所以 assertTrue(ticks.get() > 0) 恒真。按 +400ms 推演：finish 入口 check（0.4s）、pre-sort check（0.8s）后进入排序，比较器采样那次读到 1.2s 才到期；但即便删掉比较器里的检查，排序结束后的 limits.check() 同样会在 1.2s 抛 SQLTimeoutException，用例照样通过——用例名声称的"排序内检查截止时间"没有被验证。建议：让超时恰好在比较器采样那次到期（例如前几次 check 不推进时钟），并断言异常发生在排序完成之前，或断言 SortCancelled 路径确实被走过。
|
|
||
| ### Custom SQL command timeout | ||
|
|
||
| `execute` / `executeQuery` command parameters: `sql` is the SQL text; `batchSize` is a whole int (default 1000; zero or negative values use the default); `queryTimeoutSeconds` is whole seconds in 0..2147483647. Omitted or 0 means no statement timeout; negative, fractional and overflowing values are rejected. Positive values use JDBC `Statement.setQueryTimeout`. A driver that cannot enforce it returns an explicit error; set 0 only if an unlimited query is acceptable. MySQL command queries read the result set in streaming mode (`fetchSize=Integer.MIN_VALUE`); Connector/J ties the cancel timer to statement execution, so the timeout bounds `execute()` but not the subsequent row-by-row fetch. Use `batchSize`, cancellation and a server-side `socketTimeout`/`net_read_timeout` if row reading itself must be bounded. |
There was a problem hiding this comment.
[低] MySQL 文档建议的超时兜底参数不正确
文档写"配合服务端 socketTimeout/net_read_timeout"来限制逐行读取，但 socketTimeout 是 Connector/J 的客户端连接属性，不是服务端设置；net_read_timeout 控制服务端从客户端读数据，与推送流式结果无关；真正相关的是 net_write_timeout（Connector/J 的 netTimeoutForStreamingResults 即调它）。batchSize 也不提供时间上限。建议：三语文档改为"socketTimeout（Connector/J 客户端属性）/ net_write_timeout（服务端写超时，或连接串 netTimeoutForStreamingResults）"，并明确 batchSize 只影响回调粒度。
|
|
||
| ### SQL 邊界與回傳類型 | ||
|
|
||
| 支援 `SELECT *`、標量欄位投影與別名、`COUNT(*)`、`COUNT(1)`、`COUNT(column)`、`SUM`、`AVG`、`MIN`、`MAX`、多欄位 GROUP BY，以及依已選擇的表達式、別名或序號 ORDER BY。`CAST(column AS DATE)` 僅接受 DATE 或無時區 TIMESTAMP，直接取日曆日期，不隱式轉換時區；不支援 DATE(column)。ASC 預設 NULLS FIRST，DESC 預設 NULLS LAST，也可明確指定。排序需將結果行快取在記憶體，超過 `maxBufferedRows` 即整次失敗；整型欄位只接受欄位範圍內的整數字面量，小數或超範圍字面量會以 `literal ... is not representable as ...` 明確報錯，不會靜默截斷，也不會誤報為語法錯誤。ORDER BY 未加引號的別名忽略大小寫，帶引號的別名精確匹配。目前 JSqlParser 4.4 不接受識別字內部雙寫雙引號的語法。 |
There was a problem hiding this comment.
[低] zh_TW 把整型字面量规则放错了段落
"整型欄位只接受欄位範圍內的整數字面量…不會誤報為語法錯誤"被写进了 ORDER BY 那段，而 en_US（:148）与 zh_CN（:161）都放在 WHERE 段。建议：把该句移到下面"WHERE 支援…"那段，使三语结构与内容一致。
| } | ||
| // Preserve a subclass that only customizes the three-argument overload. A subclass whose | ||
| // three-argument override delegates back here would recurse forever, so fail loudly. | ||
| if (Boolean.TRUE.equals(LEGACY_DELEGATION.get())) { |
There was a problem hiding this comment.
[低] 递归守卫会误伤合法的嵌套查询
守卫是线程级标志（1167 行置位、1171 行 remove()）。只覆写 3 参的子类若在自己的方法里用 4 参（超时 0）执行另一条查询（例如会话预置语句或预查询），会被判为递归并抛 IllegalStateException，报错信息"must not delegate back"会误导排查方向。建议：把守卫改为"同一 SQL/同一调用栈帧"判定或使用重入计数并在超限时才报错，至少在该方法与 README 中写明"3 参覆写内不得再调用 4 参"的契约。
Summary
queryTimeoutSecondstoexecuteQuerycommands.Statement.setQueryTimeout()when the value is positive; absent or non-positive values preserve existing behavior.SqlExecutormethod compatible with implementations that only implement the original signature.Validation
git diff --checkpassed.