You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "somiljain2006 (via GitHub)" <gi...@apache.org> on 2026/06/01 12:05:08 UTC
[PR] Fix PK extraction for batch inserts with SQL functions [incubator-seata]
somiljain2006 opened a new pull request, #8124:
URL: https://github.com/apache/incubator-seata/pull/8124
<!-- Please make sure you have read and understood the contributing guidelines -->
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [ ] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
Track placeholders inside SQL method expressions when resolving PreparedStatement parameter indexes for INSERT statements. This prevents incorrect primary key extraction when functions such as ST_GeomFromText(CONCAT(...)) contain embedded placeholders.
### Ⅱ. Does this pull request fix one issue?
Fixes #6941
### Ⅲ. Why don't you add test cases (unit test/integration test)?
Add regression tests covering SQL method expressions with nested placeholders.
### Ⅳ. Describe how to verify it
mvn clean test
### Ⅴ. Special notes for reviews
N/A
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] Fix PK extraction for batch inserts with SQL functions [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8124:
URL: https://github.com/apache/incubator-seata/pull/8124#discussion_r3340104425
##########
sqlparser/seata-sqlparser-druid/src/main/java/org/apache/seata/sqlparser/druid/mysql/MySQLInsertRecognizer.java:
##########
@@ -126,7 +127,15 @@ public List<List<Object>> getInsertRows(Collection<Integer> primaryKeyIndex) {
} else if (expr instanceof SQLVariantRefExpr) {
row.add(((SQLVariantRefExpr) expr).getName());
} else if (expr instanceof SQLMethodInvokeExpr) {
- row.add(SqlMethodExpr.get());
+ final int[] placeholderCount = {0};
+ expr.accept(new SQLASTVisitorAdapter() {
+ @Override
+ public boolean visit(SQLVariantRefExpr x) {
+ placeholderCount[0]++;
+ return true;
+ }
+ });
Review Comment:
Inside SQLMethodInvokeExpr placeholder tracking, the visitor currently counts every SQLVariantRefExpr, including MySQL user/system variables (e.g. @var, @@version) that are not JDBC placeholders. This can over-count placeholders and miscalculate PreparedStatement parameter indexes when such variables appear inside SQL functions.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix PK extraction for batch inserts with SQL functions [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #8124:
URL: https://github.com/apache/incubator-seata/pull/8124
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] Fix PK extraction for batch inserts with SQL functions [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8124:
URL: https://github.com/apache/incubator-seata/pull/8124#issuecomment-4592784762
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `72.97297%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 72.62%. Comparing base ([`225d182`](https://app.codecov.io/gh/apache/incubator-seata/commit/225d182d6a55691d6b4695340131bd39c69bca5f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`87dd67a`](https://app.codecov.io/gh/apache/incubator-seata/commit/87dd67a901db3d90642c6727cdf77ff71ab6d9aa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/seata/rm/datasource/exec/BaseInsertExecutor.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fexec%2FBaseInsertExecutor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9leGVjL0Jhc2VJbnNlcnRFeGVjdXRvci5qYXZh) | 68.42% | [4 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/apache/seata/sqlparser/struct/SqlMethodExpr.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&filepath=sqlparser%2Fseata-sqlparser-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fsqlparser%2Fstruct%2FSqlMethodExpr.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3FscGFyc2VyL3NlYXRhLXNxbHBhcnNlci1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zcWxwYXJzZXIvc3RydWN0L1NxbE1ldGhvZEV4cHIuamF2YQ==) | 69.23% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8124 +/- ##
============================================
+ Coverage 72.59% 72.62% +0.02%
Complexity 883 883
============================================
Files 1327 1327
Lines 50719 50741 +22
Branches 6049 6054 +5
============================================
+ Hits 36821 36849 +28
+ Misses 10879 10876 -3
+ Partials 3019 3016 -3
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...a/sqlparser/druid/mysql/MySQLInsertRecognizer.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&filepath=sqlparser%2Fseata-sqlparser-druid%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fsqlparser%2Fdruid%2Fmysql%2FMySQLInsertRecognizer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3FscGFyc2VyL3NlYXRhLXNxbHBhcnNlci1kcnVpZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvc3FscGFyc2VyL2RydWlkL215c3FsL015U1FMSW5zZXJ0UmVjb2duaXplci5qYXZh) | `80.28% <100.00%> (+1.49%)` | :arrow_up: |
| [...g/apache/seata/sqlparser/struct/SqlMethodExpr.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&filepath=sqlparser%2Fseata-sqlparser-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fsqlparser%2Fstruct%2FSqlMethodExpr.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3FscGFyc2VyL3NlYXRhLXNxbHBhcnNlci1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zcWxwYXJzZXIvc3RydWN0L1NxbE1ldGhvZEV4cHIuamF2YQ==) | `71.42% <69.23%> (-28.58%)` | :arrow_down: |
| [...e/seata/rm/datasource/exec/BaseInsertExecutor.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fexec%2FBaseInsertExecutor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9leGVjL0Jhc2VJbnNlcnRFeGVjdXRvci5qYXZh) | `63.11% <68.42%> (-0.82%)` | :arrow_down: |
... and [12 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8124/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] Fix PK extraction for batch inserts with SQL functions [incubator-seata]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #8124:
URL: https://github.com/apache/incubator-seata/pull/8124#issuecomment-4600528618
@funky-eyes @slievrly Can you review this pr?
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org