-
Notifications
You must be signed in to change notification settings - Fork 8.9k
feature: add SQL Server composite primary keys (#8041) #8050
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.x
Are you sure you want to change the base?
Changes from 7 commits
64b219a
551737d
b631c7a
4117050
bc3962b
9dcfcc4
f721c5f
324bf3f
065180f
9a5a513
dc8dac9
8a27069
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| <!-- | ||
| <!-- | ||
| Licensed to the Apache Software Foundation (ASF) under one or more | ||
| contributor license agreements. See the NOTICE file distributed with | ||
| this work for additional information regarding copyright ownership. | ||
|
|
@@ -30,6 +30,7 @@ Add changes here for all PR submitted to the 2.x branch. | |
| - [[#8002](https://github.com/apache/incubator-seata/pull/8002)] add Grafana dashboard JSON for NamingServer metrics | ||
| - [[#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 | ||
|
Comment on lines
33
to
36
|
||
|
|
||
| ### bugfix: | ||
|
|
@@ -96,6 +97,7 @@ Thanks to these contributors for their code commits. Please report an unintended | |
| - [WangzJi](https://github.com/WangzJi) | ||
| - [somiljain](https://github.com/somiljain) | ||
| - [xuxiaowei-com-cn](https://github.com/xuxiaowei-com-cn) | ||
| - [UokyI](https://github.com/UokyI) | ||
|
|
||
|
|
||
| Also, we receive many valuable issues, questions and advices from our community. Thanks for you all. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |
| - [[#8002](https://github.com/apache/incubator-seata/pull/8002)] 为namingserver指标增加Grafana dashboard JSON | ||
| - [[#8020](https://github.com/apache/incubator-seata/pull/8020)] 新增 UnregisterRM 协议,在客户端销毁时通知服务端 | ||
| - [[#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 | ||
|
Comment on lines
35
to
37
|
||
|
|
||
| ### bugfix: | ||
|
|
@@ -99,5 +100,6 @@ | |
| - [xiaoxiangyeyu0](https://github.com/xiaoxiangyeyu0) | ||
| - [WangzJi](https://github.com/WangzJi) | ||
| - [xuxiaowei-com-cn](https://github.com/xuxiaowei-com-cn) | ||
| - [UokyI](https://github.com/UokyI) | ||
|
|
||
| 同时,我们收到了社区反馈的很多有价值的issue和建议,非常感谢大家。 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
|
|
||
| import org.apache.seata.rm.datasource.sql.struct.Field; | ||
| import org.apache.seata.sqlparser.util.ColumnUtils; | ||
| import org.apache.seata.sqlparser.util.JdbcConstants; | ||
|
|
||
| import java.sql.PreparedStatement; | ||
| import java.sql.SQLException; | ||
|
|
@@ -78,6 +79,11 @@ public static List<WhereSql> buildWhereConditionListByPKs(List<String> pkNameLis | |
| */ | ||
| public static List<WhereSql> buildWhereConditionListByPKs( | ||
| List<String> pkNameList, int rowSize, String dbType, int maxInSize) { | ||
| // SQL Server does not support tuple IN syntax: (col1,col2) IN ((?,?),(?,?)) | ||
| // Use AND/OR syntax instead | ||
| if (JdbcConstants.SQLSERVER.equalsIgnoreCase(dbType) && pkNameList.size() > 1) { | ||
| return buildWhereConditionListByPKsForSqlServer(pkNameList, rowSize, maxInSize, dbType); | ||
| } | ||
| List<WhereSql> whereSqls = new ArrayList<>(); | ||
| // we must consider the situation of composite primary key | ||
| int batchSize = rowSize % maxInSize == 0 ? rowSize / maxInSize : (rowSize / maxInSize) + 1; | ||
|
|
@@ -115,6 +121,45 @@ public static List<WhereSql> buildWhereConditionListByPKs( | |
| return whereSqls; | ||
| } | ||
|
|
||
| /** | ||
| * 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<>(); | ||
|
Comment on lines
+124
to
+137
|
||
| int batchSize = rowSize % maxInSize == 0 ? rowSize / maxInSize : (rowSize / maxInSize) + 1; | ||
| for (int batch = 0; batch < batchSize; batch++) { | ||
| StringBuilder whereStr = new StringBuilder(); | ||
| int eachSize = | ||
| (batch == batchSize - 1) ? (rowSize % maxInSize == 0 ? maxInSize : rowSize % maxInSize) : maxInSize; | ||
|
|
||
| for (int i = 0; i < eachSize; i++) { | ||
| if (i > 0) { | ||
| whereStr.append(" OR "); | ||
| } | ||
| whereStr.append("("); | ||
| for (int x = 0; x < pkNameList.size(); x++) { | ||
| if (x > 0) { | ||
| whereStr.append(" AND "); | ||
| } | ||
| whereStr.append(ColumnUtils.addEscape(pkNameList.get(x), dbType)); | ||
| whereStr.append("=?"); | ||
| } | ||
| whereStr.append(")"); | ||
| } | ||
| whereSqls.add(new WhereSql(whereStr.toString(), eachSize, pkNameList.size())); | ||
| } | ||
|
|
||
| return whereSqls; | ||
| } | ||
|
|
||
| /** | ||
| * set parameter for PreparedStatement, this is only used in pk sql. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ | |
| import java.sql.Statement; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -69,7 +70,8 @@ public SqlServerInsertExecutor( | |
|
|
||
| @Override | ||
| public Map<String, List<Object>> getPkValues() throws SQLException { | ||
| Map<String, List<Object>> pkValuesMap; | ||
| // Fix: Initialize pkValuesMap to support SQL Server composite primary keys. | ||
| Map<String, List<Object>> pkValuesMap = new HashMap<>(); | ||
| boolean isContainsPk = containsPK(); | ||
| List<String> pkColumnNameList = getTableMeta().getPrimaryKeyOnlyName(); | ||
|
|
||
|
|
@@ -84,8 +86,44 @@ public Map<String, List<Object>> getPkValues() throws SQLException { | |
| pkValuesMap = getPkValuesWithNoColumn(); | ||
| } | ||
| } else { | ||
| // when there is a composite primary key | ||
| throw new NotSupportYetException("composite primary key is not supported in sqlserver"); | ||
| // 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 { | ||
|
Comment on lines
+89
to
+128
|
||
| throw new NotSupportYetException( | ||
| "composite primary key with non-autoincrement column not in INSERT is not supported in sqlserver: " | ||
| + pkColumnName); | ||
| } | ||
| } | ||
|
Comment on lines
+89
to
+133
|
||
| } | ||
| } | ||
|
|
||
| return pkValuesMap; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -209,4 +209,114 @@ private void mockStatementInsertRows() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rows.add(Arrays.asList(Null.get(), "xx", "xx", "xx")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| when(sqlInsertRecognizer.getInsertRows(pkIndexMap.values())).thenReturn(rows); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void testGetPkValues_compositePrimaryKey_withAllPkInInsert() throws Exception { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Mock composite primary key: id + user_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> compositePkList = Arrays.asList(ID_COLUMN, USER_ID_COLUMN); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| when(tableMeta.getPrimaryKeyOnlyName()).thenReturn(compositePkList); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doReturn(tableMeta).when(insertExecutor).getTableMeta(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doReturn(pkIndexMap).when(insertExecutor).getPkIndex(); // PK columns are in INSERT statement | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Mock getPkValuesByColumn to return expected values directly | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, List<Object>> expectedPkValues = new HashMap<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expectedPkValues.put(ID_COLUMN, Arrays.asList(1, 2)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expectedPkValues.put(USER_ID_COLUMN, Arrays.asList("user1", "user2")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doReturn(expectedPkValues).when(insertExecutor).getPkValuesByColumn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, List<Object>> pkValues = insertExecutor.getPkValues(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify composite primary key values are correctly retrieved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Assertions.assertNotNull(pkValues); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Assertions.assertEquals(expectedPkValues, pkValues); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify that getPkValuesByColumn was called, confirming the code path for composite keys with manual values | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| verify(insertExecutor).getPkValuesByColumn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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_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)); |
Copilot
AI
Apr 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Copilot
AI
Apr 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.