feature: add SQL Server composite primary keys (#8041)#8050
Conversation
funky-eyes
added
mode: AT
funky-eyes
left a comment
funky-eyes
left a comment
There was a problem hiding this comment.
Run 'mvn spotless:apply' to fix these violations.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #8050 +/- ##
============================================
+ Coverage 72.10% 72.27% +0.17%
- Complexity 872 876 +4
============================================
Files 1320 1320
Lines 50550 50590 +40
Branches 6026 6040 +14
============================================
+ Hits 36447 36566 +119
+ Misses 11108 11039 -69
+ Partials 2995 2985 -10
... and 41 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds SQL Server support for composite primary keys in Seata AT mode, primarily by generating SQL Server-compatible PK WHERE clauses and enhancing SQL Server INSERT PK value extraction.
Changes:
- Adjusts SQL Server INSERT
PreparedStatementcreation to useStatement.RETURN_GENERATED_KEYSinstead of column-name arrays. - Adds SQL Server-specific composite-PK WHERE clause generation using AND/OR syntax instead of tuple
IN. - Extends
SqlServerInsertExecutorand unit tests to support composite primary keys.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rm-datasource/src/main/java/org/apache/seata/rm/datasource/AbstractConnectionProxy.java | Uses SQL Server-compatible generated-keys mode for INSERT prepared statements. |
| rm-datasource/src/main/java/org/apache/seata/rm/datasource/SqlGenerateUtils.java | Adds SQL Server PK WHERE generation via AND/OR instead of tuple IN. |
| rm-datasource/src/main/java/org/apache/seata/rm/datasource/exec/sqlserver/SqlServerInsertExecutor.java | Implements composite PK handling for SQL Server INSERT PK extraction. |
| rm-datasource/src/test/java/org/apache/seata/rm/datasource/SqlGenerateUtilsTest.java | Adds unit tests for SQL Server composite/single PK WHERE clause generation. |
| rm-datasource/src/test/java/org/apache/seata/rm/datasource/exec/SqlServerInsertExecutorTest.java | Adds unit tests for SQL Server composite PK PK-value retrieval scenarios. |
| changes/en-us/2.x.md | Adds changelog entry for SQL Server composite PK support. |
| changes/zh-cn/2.x.md | Adds changelog entry for SQL Server composite PK support (Chinese). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - [[#8041](https://github.com/apache/incubator-seata/pull/8041)] 新增SQL Server 多主键支持 | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] 添加了 fastjson2 和 jackson3 | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line introduced after the new #8041 entry, which leaves unnecessary vertical spacing in the rendered changelog. Consider removing the additional empty line to keep formatting consistent with surrounding entries.
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(USER_ID_COLUMN)); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
The new composite-PK tests cover (1) all PK columns provided and (2) all PK columns marked autoincrement, but they don't cover the common mixed case where some PK columns are provided in the INSERT and the remaining PK column is identity/auto-generated. Adding a test for that scenario would catch the current behavior where getPkValues() throws instead of merging manual + generated PK values.
| @Test | |
| @Test | |
| public void testGetPkValues_compositePrimaryKey_withPartialPkInInsertAndAutoIncrement() throws Exception { | |
| // Mock composite primary key where one PK is provided in INSERT and the other is auto-increment | |
| List<String> compositePkList = Arrays.asList(ID_COLUMN, USER_ID_COLUMN); | |
| when(tableMeta.getPrimaryKeyOnlyName()).thenReturn(compositePkList); | |
| Map<String, ColumnMeta> pkMap = new HashMap<>(); | |
| ColumnMeta idMeta = mock(ColumnMeta.class); | |
| when(idMeta.isAutoincrement()).thenReturn(false); // Provided in INSERT | |
| pkMap.put(ID_COLUMN, idMeta); | |
| ColumnMeta userIdMeta = mock(ColumnMeta.class); | |
| when(userIdMeta.isAutoincrement()).thenReturn(true); // Generated by database | |
| pkMap.put(USER_ID_COLUMN, userIdMeta); | |
| when(tableMeta.getPrimaryKeyMap()).thenReturn(pkMap); | |
| mockParametersForCompositePk(); | |
| doReturn(tableMeta).when(insertExecutor).getTableMeta(); | |
| doReturn(true).when(insertExecutor).containsPK(); // One primary key column is present in INSERT | |
| doReturn(Arrays.asList(PK_VALUE)).when(insertExecutor).getGeneratedKeys(); | |
| Map<String, List<Object>> pkValues = insertExecutor.getPkValues(); | |
| // Verify manual and generated PK values are merged correctly | |
| Assertions.assertEquals(Arrays.asList(1), pkValues.get(ID_COLUMN)); | |
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(USER_ID_COLUMN)); | |
| } | |
| @Test |
| // when there is a composite primary key - Fix: Support SQL Server composite primary keys. | ||
| if (isContainsPk) { | ||
| // All primary key columns are manually assigned in the INSERT statement. | ||
| pkValuesMap = getPkValuesByColumn(); | ||
| } else { | ||
| // Some or all primary key columns are auto-generated. | ||
| // Get metadata for all primary key columns. | ||
| Map<String, ColumnMeta> primaryKeyMap = getTableMeta().getPrimaryKeyMap(); | ||
|
|
||
| // Iterate over each primary key column. | ||
| for (String pkColumnName : pkColumnNameList) { | ||
| ColumnMeta pkMeta = primaryKeyMap.get(pkColumnName); | ||
| if (pkMeta.isAutoincrement()) { | ||
| // Auto-increment column: get from generated keys. | ||
| pkValuesMap.put(pkColumnName, getGeneratedKeys()); | ||
| } else { | ||
| // Non-auto-increment column: requires manual assignment, throw exception if not supported. | ||
| throw new NotSupportYetException( | ||
| "composite primary key with non-autoincrement column is not supported in sqlserver: " | ||
| + pkColumnName); | ||
| } | ||
| } |
There was a problem hiding this comment.
getPkValues() for SQL Server composite PKs incorrectly assumes that when containsPK() is false, none of the PK columns are provided. For a common composite PK case where one PK column is provided in the INSERT and another is identity/auto-generated, this branch will throw for the provided non-autoincrement column instead of merging manual values with generated keys. Consider following the MySQLInsertExecutor pattern: start from getPkValuesByColumn() (for any PKs present) and then fill any missing PK columns that are autoincrement from getGeneratedKeys(); only throw if a required PK column is still missing and not autoincrement.
| // Use AND/OR syntax instead | ||
| if (JdbcConstants.SQLSERVER.equalsIgnoreCase(dbType)) { |
There was a problem hiding this comment.
The SQL Server special-case in buildWhereConditionListByPKs triggers for all PK sizes, including single-column PKs. This changes SQL Server WHERE generation from the existing tuple-IN form to an OR chain (e.g. (id=?) OR (id=?)), which is longer and can be less efficient for large batches. If the limitation only applies to composite PKs, gate this branch on pkNameList.size() > 1 and let the existing implementation handle single-column PKs.
| // Use AND/OR syntax instead | |
| if (JdbcConstants.SQLSERVER.equalsIgnoreCase(dbType)) { | |
| // Use AND/OR syntax instead for composite primary keys | |
| if (JdbcConstants.SQLSERVER.equalsIgnoreCase(dbType) && pkNameList.size() > 1) { |
| String[] pkNameArray = | ||
| new String[tableMeta.getPrimaryKeyOnlyName().size()]; | ||
| tableMeta.getPrimaryKeyOnlyName().toArray(pkNameArray); | ||
| targetPreparedStatement = getTargetConnection().prepareStatement(sql, pkNameArray); | ||
| // Fix: SQL Server does not support array of column names for getGeneratedKeys, use | ||
| // RETURN_GENERATED_KEYS instead. | ||
| if (JdbcConstants.SQLSERVER.equalsIgnoreCase(dbType)) { | ||
| targetPreparedStatement = | ||
| getTargetConnection().prepareStatement(sql, Statement.RETURN_GENERATED_KEYS); | ||
| } else { | ||
| targetPreparedStatement = getTargetConnection().prepareStatement(sql, pkNameArray); | ||
| } |
There was a problem hiding this comment.
pkNameArray is built unconditionally, but for SQL Server the code path now ignores it and uses Statement.RETURN_GENERATED_KEYS. Consider moving the pkNameArray allocation/population into the non-SQLServer branch to avoid unnecessary work on SQL Server inserts.
|
|
||
| private void mockParametersForCompositePk() { | ||
| Map<Integer, ArrayList<Object>> parameters = new HashMap<>(4); | ||
| ArrayList<Object> arrayList0 = new ArrayList<>(); | ||
| arrayList0.add(1); // id value | ||
| ArrayList<Object> arrayList1 = new ArrayList<>(); | ||
| arrayList1.add("userId1"); | ||
| ArrayList<Object> arrayList2 = new ArrayList<>(); | ||
| arrayList2.add("userName1"); | ||
| ArrayList<Object> arrayList3 = new ArrayList<>(); | ||
| arrayList3.add("userStatus1"); | ||
| parameters.put(1, arrayList0); | ||
| parameters.put(2, arrayList1); | ||
| parameters.put(3, arrayList2); | ||
| parameters.put(4, arrayList3); | ||
| PreparedStatementProxy psp = (PreparedStatementProxy) this.statementProxy; | ||
| when(psp.getParameters()).thenReturn(parameters); | ||
|
|
||
| List<List<Object>> rows = new ArrayList<>(); | ||
| rows.add(Arrays.asList("?", "?", "?", "?")); | ||
| when(sqlInsertRecognizer.getInsertRows(pkIndexMap.values())).thenReturn(rows); | ||
| } |
There was a problem hiding this comment.
mockParametersForCompositePk() is added but never called in this test class. Unused helpers make the test harder to maintain; either use it in the composite PK tests or remove it.
| private void mockParametersForCompositePk() { | |
| Map<Integer, ArrayList<Object>> parameters = new HashMap<>(4); | |
| ArrayList<Object> arrayList0 = new ArrayList<>(); | |
| arrayList0.add(1); // id value | |
| ArrayList<Object> arrayList1 = new ArrayList<>(); | |
| arrayList1.add("userId1"); | |
| ArrayList<Object> arrayList2 = new ArrayList<>(); | |
| arrayList2.add("userName1"); | |
| ArrayList<Object> arrayList3 = new ArrayList<>(); | |
| arrayList3.add("userStatus1"); | |
| parameters.put(1, arrayList0); | |
| parameters.put(2, arrayList1); | |
| parameters.put(3, arrayList2); | |
| parameters.put(4, arrayList3); | |
| PreparedStatementProxy psp = (PreparedStatementProxy) this.statementProxy; | |
| when(psp.getParameters()).thenReturn(parameters); | |
| List<List<Object>> rows = new ArrayList<>(); | |
| rows.add(Arrays.asList("?", "?", "?", "?")); | |
| when(sqlInsertRecognizer.getInsertRows(pkIndexMap.values())).thenReturn(rows); | |
| } |
| - [[#8041](https://github.com/apache/incubator-seata/pull/8041)] add SQL Server composite primary keys | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] add fastjson2 and jackson3 | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an extra blank line introduced after the new #8041 entry, which leaves an empty list item spacing in the rendered changelog. Consider removing the additional empty line to keep formatting consistent with surrounding entries.
| - [[#8041](https://github.com/apache/incubator-seata/pull/8041)] add SQL Server composite primary keys | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] add fastjson2 and jackson3 | ||
|
|
||
|
|
There was a problem hiding this comment.
Please add your GitHub information to the contributors list at the bottom. Thank you.
| - [[#8041](https://github.com/apache/incubator-seata/pull/8041)] 新增SQL Server 多主键支持 | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] 添加了 fastjson2 和 jackson3 | ||
|
|
||
|
|
There was a problem hiding this comment.
Please add your GitHub information to the contributors list at the bottom. Thank you.
…pache#8050) - Fix SqlServerInsertExecutor getPkValues() to handle mixed PK scenario where some PK columns are in INSERT and others are auto-increment - Gate SqlGenerateUtils SQL Server special branch on pkNameList.size() > 1 to avoid affecting single PK scenarios - Move pkNameArray allocation into non-SQLServer branch in AbstractConnectionProxy to avoid unnecessary overhead - Update unit tests to match new implementation logic and add mixed PK test case - Fix changelog PR link from apache#8041 (issue) to apache#8050 (PR) - Add UokyI to contributors list
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <!-- | ||
| Licensed to the Apache Software Foundation (ASF) under one or more | ||
| contributor license agreements. See the NOTICE file distributed with |
There was a problem hiding this comment.
The very first character on this file appears to be a UTF-8 BOM / invisible character (line starts with "\uFEFF<!--"). This can create noisy diffs and occasionally breaks tooling; please remove the BOM so the file starts with a plain "<!--" like the other changelog files.
| /** | ||
| * Build where condition list by PKs for SQL Server. | ||
| * SQL Server does not support tuple IN syntax: (col1,col2) IN ((?,?),(?,?)) | ||
| * Use AND/OR syntax instead: (col1=? AND col2=?) OR (col1=? AND col2=?) | ||
| * | ||
| * @param pkNameList pk column name list | ||
| * @param rowSize the row size of records | ||
| * @param maxInSize the max in size | ||
| * @return where condition sql list for SQL Server | ||
| */ | ||
| private static List<WhereSql> buildWhereConditionListByPKsForSqlServer( | ||
| List<String> pkNameList, int rowSize, int maxInSize, String dbType) { | ||
| List<WhereSql> whereSqls = new ArrayList<>(); |
There was a problem hiding this comment.
Javadoc for buildWhereConditionListByPKsForSqlServer() is missing an @param entry for the dbType argument, even though it is part of the signature and used for escaping. Please either document dbType or remove it from the signature (since this method is SQL Server-specific).
| // when there is a composite primary key - Fix: Support SQL Server composite primary keys. | ||
| // SQL Server allows only one IDENTITY column per table. | ||
| // So composite PK can have at most one auto-increment column. | ||
| // Strategy: parse PK values from INSERT columns, then fill missing auto-increment PK from generated keys. | ||
| if (!getPkIndex().isEmpty()) { | ||
| // At least one PK column is in the INSERT statement. | ||
| pkValuesMap = getPkValuesByColumn(); | ||
| Map<String, ColumnMeta> primaryKeyMap = getTableMeta().getPrimaryKeyMap(); | ||
|
|
||
| // Fill any missing auto-increment PK columns from generated keys. | ||
| for (String pkColumnName : pkColumnNameList) { | ||
| if (!pkValuesMap.containsKey(pkColumnName)) { | ||
| ColumnMeta pkMeta = primaryKeyMap.get(pkColumnName); | ||
| if (pkMeta.isAutoincrement()) { | ||
| pkValuesMap.put(pkColumnName, getGeneratedKeys()); | ||
| } else { | ||
| throw new NotSupportYetException( | ||
| "composite primary key with non-autoincrement column not in INSERT is not supported in sqlserver: " | ||
| + pkColumnName); | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| // No PK columns in INSERT statement. | ||
| // For composite PK, this means all PK columns must have values from elsewhere. | ||
| // Since SQL Server only supports one IDENTITY column, non-identity PK columns would fail. | ||
| Map<String, ColumnMeta> primaryKeyMap = getTableMeta().getPrimaryKeyMap(); | ||
| for (String pkColumnName : pkColumnNameList) { | ||
| ColumnMeta pkMeta = primaryKeyMap.get(pkColumnName); | ||
| if (pkMeta.isAutoincrement()) { | ||
| pkValuesMap.put(pkColumnName, getGeneratedKeys()); | ||
| } else { |
There was a problem hiding this comment.
In composite-PK branches, this calls getGeneratedKeys() inside a loop and may invoke it multiple times (e.g., when more than one PK column is missing or marked autoincrement). SqlServerInsertExecutor.getGeneratedKeys() consumes the generated-keys ResultSet and relies on beforeFirst(), which may fail on some drivers; repeated calls can therefore return empty and throw NotSupportYetException or produce inconsistent results. Consider fetching generated keys once and reusing the list, and/or explicitly enforcing that at most one PK column can be autoincrement (otherwise fail fast).
| public void testGetPkValues_compositePrimaryKey_withAutoIncrement() throws Exception { | ||
| // Mock composite primary key with auto-increment columns | ||
| List<String> compositePkList = Arrays.asList(ID_COLUMN, USER_ID_COLUMN); | ||
| when(tableMeta.getPrimaryKeyOnlyName()).thenReturn(compositePkList); | ||
|
|
||
| Map<String, ColumnMeta> pkMap = new HashMap<>(); | ||
| ColumnMeta idMeta = mock(ColumnMeta.class); | ||
| when(idMeta.isAutoincrement()).thenReturn(true); // Auto-increment | ||
| pkMap.put(ID_COLUMN, idMeta); | ||
|
|
||
| ColumnMeta userIdMeta = mock(ColumnMeta.class); | ||
| when(userIdMeta.isAutoincrement()).thenReturn(true); // Auto-increment | ||
| pkMap.put(USER_ID_COLUMN, userIdMeta); | ||
| when(tableMeta.getPrimaryKeyMap()).thenReturn(pkMap); | ||
|
|
||
| doReturn(tableMeta).when(insertExecutor).getTableMeta(); | ||
| doReturn(new HashMap<String, Integer>()).when(insertExecutor).getPkIndex(); // No PK columns in INSERT | ||
| doReturn(Arrays.asList(PK_VALUE)).when(insertExecutor).getGeneratedKeys(); | ||
|
|
||
| Map<String, List<Object>> pkValues = insertExecutor.getPkValues(); | ||
|
|
||
| // Verify auto-increment column gets value from generated keys | ||
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(ID_COLUMN)); | ||
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(USER_ID_COLUMN)); |
There was a problem hiding this comment.
This test models a composite PK where both columns are autoincrement and expects the same generated key to be applied to both. SQL Server allows only one IDENTITY column per table (and the production code comment states that), so this scenario is not valid and could mask real behavior. Please adjust the test to a realistic composite-PK case (e.g., one provided PK + one IDENTITY, which you already test) or assert that multiple autoincrement PK columns are rejected.
| public void testGetPkValues_compositePrimaryKey_withAutoIncrement() throws Exception { | |
| // Mock composite primary key with auto-increment columns | |
| List<String> compositePkList = Arrays.asList(ID_COLUMN, USER_ID_COLUMN); | |
| when(tableMeta.getPrimaryKeyOnlyName()).thenReturn(compositePkList); | |
| Map<String, ColumnMeta> pkMap = new HashMap<>(); | |
| ColumnMeta idMeta = mock(ColumnMeta.class); | |
| when(idMeta.isAutoincrement()).thenReturn(true); // Auto-increment | |
| pkMap.put(ID_COLUMN, idMeta); | |
| ColumnMeta userIdMeta = mock(ColumnMeta.class); | |
| when(userIdMeta.isAutoincrement()).thenReturn(true); // Auto-increment | |
| pkMap.put(USER_ID_COLUMN, userIdMeta); | |
| when(tableMeta.getPrimaryKeyMap()).thenReturn(pkMap); | |
| doReturn(tableMeta).when(insertExecutor).getTableMeta(); | |
| doReturn(new HashMap<String, Integer>()).when(insertExecutor).getPkIndex(); // No PK columns in INSERT | |
| doReturn(Arrays.asList(PK_VALUE)).when(insertExecutor).getGeneratedKeys(); | |
| Map<String, List<Object>> pkValues = insertExecutor.getPkValues(); | |
| // Verify auto-increment column gets value from generated keys | |
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(ID_COLUMN)); | |
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(USER_ID_COLUMN)); | |
| public void testGetPkValues_compositePrimaryKey_withOneAutoIncrementAndOnePkInInsert() throws Exception { | |
| // Mock realistic SQL Server composite primary key: one IDENTITY column + one provided PK column | |
| List<String> compositePkList = Arrays.asList(ID_COLUMN, USER_ID_COLUMN); | |
| when(tableMeta.getPrimaryKeyOnlyName()).thenReturn(compositePkList); | |
| Map<String, ColumnMeta> pkMap = new HashMap<>(); | |
| ColumnMeta idMeta = mock(ColumnMeta.class); | |
| when(idMeta.isAutoincrement()).thenReturn(true); | |
| pkMap.put(ID_COLUMN, idMeta); | |
| ColumnMeta userIdMeta = mock(ColumnMeta.class); | |
| when(userIdMeta.isAutoincrement()).thenReturn(false); | |
| pkMap.put(USER_ID_COLUMN, userIdMeta); | |
| when(tableMeta.getPrimaryKeyMap()).thenReturn(pkMap); | |
| doReturn(tableMeta).when(insertExecutor).getTableMeta(); | |
| Map<String, Integer> pkIndex = new HashMap<>(); | |
| pkIndex.put(USER_ID_COLUMN, 0); | |
| doReturn(pkIndex).when(insertExecutor).getPkIndex(); | |
| Map<String, List<Object>> pkValuesByColumn = new HashMap<>(); | |
| pkValuesByColumn.put(USER_ID_COLUMN, Arrays.asList("user1")); | |
| doReturn(pkValuesByColumn).when(insertExecutor).getPkValuesByColumn(); | |
| doReturn(Arrays.asList(PK_VALUE)).when(insertExecutor).getGeneratedKeys(); | |
| Map<String, List<Object>> pkValues = insertExecutor.getPkValues(); | |
| Assertions.assertEquals(Arrays.asList(PK_VALUE), pkValues.get(ID_COLUMN)); | |
| Assertions.assertEquals(Arrays.asList("user1"), pkValues.get(USER_ID_COLUMN)); |
| - [[#8044](https://github.com/apache/incubator-seata/pull/8044)] 为 UnregisterRM 协议添加 protobuf 序列化支持 | ||
| - [[#8050](https://github.com/apache/incubator-seata/pull/8050)] 新增SQL Server 多主键支持 | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] 添加了 fastjson2 和 jackson3 |
There was a problem hiding this comment.
There was a problem hiding this comment.
@funky-eyes 我这个地址，是应该写 [#8050] ？我看其他人的格式都是这样写的，还是按照Copilot AI的反馈，使用 [#8041] ?
| - [[#8020](https://github.com/apache/incubator-seata/pull/8020)] add UnregisterRM protocol to notify server on client destroy | ||
| - [[#8044](https://github.com/apache/incubator-seata/pull/8044)] add protobuf serialization support for UnregisterRM protocol | ||
| - [[#8050](https://github.com/apache/incubator-seata/pull/8050)] add SQL Server composite primary keys | ||
| - [[#8046](https://github.com/apache/incubator-seata/pull/8046)] add fastjson2 and jackson3 |
There was a problem hiding this comment.
|
Hi, could you please take a look at the comment Copilot left above? |
… primary keys (apache#8041) - Add @param dbType to buildWhereConditionListByPKsForSqlServer Javadoc - Cache getGeneratedKeys() result before loop to avoid repeated ResultSet consumption - Fix unrealistic test: replace dual-autoincrement composite PK test with single IDENTITY + non-autoincrement scenario - Remove UTF-8 BOM from changes/en-us/2.x.md
I've checked the Copilot comment about the changelog entry links. The AI suggested changing #8050 to #8041, but looking at the existing entries in the changelog (e.g., #8020, #8044, #8046), they all use the PR number rather than the issue number. So #8050 follows the project convention. This appears to be a false positive from the AI review. |
Ⅰ. Describe what this PR did
This PR adds support for SQL Server composite primary keys in Seata. The changes include:
AbstractConnectionProxy.java: Fixed SQL Server compatibility issue - SQL Server does not support array of column names for
getGeneratedKeys, now usesStatement.RETURN_GENERATED_KEYSinstead.SqlGenerateUtils.java: Added
buildWhereConditionListByPKsForSqlServer()method to handle composite primary keys. SQL Server does not support tuple IN syntax(col1,col2) IN ((?,?),(?,?)), so it now uses AND/OR syntax(col1=? AND col2=?) OR (col1=? AND col2=?).SqlServerInsertExecutor.java: Implemented composite primary key support for SQL Server INSERT operations. Handles both manually assigned primary keys and auto-increment columns.
Unit Tests: Added test cases for SQL Server composite primary key scenarios in
SqlGenerateUtilsTestandSqlServerInsertExecutorTest.Ⅱ. Does this pull request fix one issue?
fixes #8041
Ⅲ. Why don't you add test cases (unit test/integration test)?
Tests are included in this PR.
SqlGenerateUtilsTest.testBuildWhereConditionListByPKsForSqlServer()- Tests composite PK WHERE clause generationSqlGenerateUtilsTest.testBuildWhereConditionListByPKsForSqlServerWithSinglePk()- Tests single PK WHERE clause generationSqlServerInsertExecutorTest.testGetPkValues_compositePrimaryKey_withAllPkInInsert()- Tests composite PK with manual valuesSqlServerInsertExecutorTest.testGetPkValues_compositePrimaryKey_withAutoIncrement()- Tests composite PK with auto-incrementSqlServerInsertExecutorTest.testGetPkValues_compositePrimaryKey_nonAutoIncrementThrowsException()- Tests exception handlingⅣ. Describe how to verify it
Ⅴ. Special notes for reviews