You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by pa...@apache.org on 2022/02/17 08:08:11 UTC

[shardingsphere] branch master updated: Fix NPE caused by GeneratedKeysResultSet not return columnName in read-write splitting (#15464)

This is an automated email from the ASF dual-hosted git repository.

panjuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git


The following commit(s) were added to refs/heads/master by this push:
     new d37738d  Fix NPE caused by GeneratedKeysResultSet not return columnName in read-write splitting (#15464)
d37738d is described below

commit d37738ddb0a89c83fee9a1782a693366c5743350
Author: Zhengqiang Duan <du...@apache.org>
AuthorDate: Thu Feb 17 16:07:00 2022 +0800

    Fix NPE caused by GeneratedKeysResultSet not return columnName in read-write splitting (#15464)
    
    * Fix NPE caused by GeneratedKeysResultSet not return columnName in read-write splitting
    
    * Add unit test for getGeneratedKeys method
---
 .../loader/dialect/H2TableMetaDataLoader.java      |  3 +--
 .../statement/ShardingSpherePreparedStatement.java |  3 ++-
 .../core/statement/ShardingSphereStatement.java    |  3 ++-
 .../driver/jdbc/base/AbstractSQLTest.java          |  2 +-
 .../ReadwriteSplittingPreparedStatementTest.java   | 25 +++++++++++++++++++++-
 .../statement/ReadwriteSplittingStatementTest.java | 21 ++++++++++++++++++
 .../src/test/resources/sql/jdbc_init.sql           |  2 +-
 7 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/loader/dialect/H2TableMetaDataLoader.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/loader/dialect/H2TableMetaDataLoader.java
index 2a8b525..b94ca50 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/loader/dialect/H2TableMetaDataLoader.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/loader/dialect/H2TableMetaDataLoader.java
@@ -104,8 +104,7 @@ public final class H2TableMetaDataLoader implements DialectTableMetaDataLoader {
         String columnName = resultSet.getString("COLUMN_NAME");
         String typeName = resultSet.getString("TYPE_NAME");
         boolean primaryKey = primaryKeys.contains(columnName);
-        // tableGenerated.getOrDefault(columnName, Boolean.FALSE);
-        boolean generated = false;
+        boolean generated = tableGenerated.getOrDefault(columnName, Boolean.FALSE);
         // H2 database case sensitive is always true
         return new ColumnMetaData(columnName, dataTypeMap.get(typeName), primaryKey, generated, true);
     }
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java
index 8de29fe..a24622a 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java
@@ -494,7 +494,8 @@ public final class ShardingSpherePreparedStatement extends AbstractPreparedState
                 generatedValues.add((Comparable<?>) resultSet.getObject(1));
             }
         }
-        return new GeneratedKeysResultSet(null, generatedValues.iterator(), this);
+        String columnName = generatedKey.map(GeneratedKeyContext::getColumnName).orElse(null);
+        return new GeneratedKeysResultSet(columnName, generatedValues.iterator(), this);
     }
     
     @Override
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java
index a08e19a..89c2930 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java
@@ -549,7 +549,8 @@ public final class ShardingSphereStatement extends AbstractStatementAdapter {
                 generatedValues.add((Comparable<?>) resultSet.getObject(1));
             }
         }
-        return new GeneratedKeysResultSet(null, generatedValues.iterator(), this);
+        String columnName = generatedKey.map(GeneratedKeyContext::getColumnName).orElse(null);
+        return new GeneratedKeysResultSet(columnName, generatedValues.iterator(), this);
     }
     
     private Optional<GeneratedKeyContext> findGeneratedKey() {
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/base/AbstractSQLTest.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/base/AbstractSQLTest.java
index 61d15d3..7f34f60 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/base/AbstractSQLTest.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/base/AbstractSQLTest.java
@@ -32,7 +32,7 @@ import java.util.Objects;
 
 public abstract class AbstractSQLTest {
     
-    private static final List<String> ACTUAL_DATA_SOURCE_NAMES = Arrays.asList("jdbc_0", "jdbc_1", "shadow_jdbc_0", "shadow_jdbc_1", "encrypt", "test_primary_ds", "test_replica_ds");
+    private static final List<String> ACTUAL_DATA_SOURCE_NAMES = Arrays.asList("jdbc_0", "jdbc_1", "shadow_jdbc_0", "shadow_jdbc_1", "encrypt", "test_write_ds", "test_read_ds");
     
     private static final Map<String, DataSource> ACTUAL_DATA_SOURCES = new HashMap<>();
     
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingPreparedStatementTest.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingPreparedStatementTest.java
index d8b3a44..fe04e8f 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingPreparedStatementTest.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingPreparedStatementTest.java
@@ -21,10 +21,15 @@ import org.apache.shardingsphere.driver.jdbc.base.AbstractShardingSphereDataSour
 import org.junit.Test;
 
 import java.sql.PreparedStatement;
+import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Statement;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 public final class ReadwriteSplittingPreparedStatementTest extends AbstractShardingSphereDataSourceForReadwriteSplittingTest {
     
@@ -44,8 +49,26 @@ public final class ReadwriteSplittingPreparedStatementTest extends AbstractShard
     
     @Test
     public void assertGetParameterMetaData() throws SQLException {
-        try (PreparedStatement preparedStatement = getReadwriteSplittingDataSource().getConnection().prepareStatement("SELECT * FROM t_global where id = ?")) {
+        try (PreparedStatement preparedStatement = getReadwriteSplittingDataSource().getConnection().prepareStatement("SELECT * FROM t_config where id = ?")) {
             assertThat(preparedStatement.getParameterMetaData().getParameterCount(), is(1));
         }
     }
+    
+    @Test
+    public void assertGetGeneratedKeys() throws SQLException {
+        try (PreparedStatement preparedStatement = getReadwriteSplittingDataSource()
+                .getConnection().prepareStatement("INSERT INTO t_config(status) VALUES(?);", Statement.RETURN_GENERATED_KEYS)) {
+            preparedStatement.setString(1, "OK");
+            preparedStatement.executeUpdate();
+            ResultSet generatedKeys = preparedStatement.getGeneratedKeys();
+            assertTrue(generatedKeys.next());
+            int columnCount = generatedKeys.getMetaData().getColumnCount();
+            for (int index = 0; index < columnCount; index++) {
+                assertNotNull(generatedKeys.getObject(index + 1));
+                assertNotNull(generatedKeys.getMetaData().getColumnLabel(index + 1));
+                assertNotNull(generatedKeys.getMetaData().getColumnName(index + 1));
+            }
+            assertFalse(generatedKeys.next());
+        }
+    }
 }
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingStatementTest.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingStatementTest.java
index d254deb..a4ffb47 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingStatementTest.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ReadwriteSplittingStatementTest.java
@@ -20,9 +20,14 @@ package org.apache.shardingsphere.driver.jdbc.core.statement;
 import org.apache.shardingsphere.driver.jdbc.base.AbstractShardingSphereDataSourceForReadwriteSplittingTest;
 import org.junit.Test;
 
+import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 public final class ReadwriteSplittingStatementTest extends AbstractShardingSphereDataSourceForReadwriteSplittingTest {
     
     @Test(expected = SQLException.class)
@@ -38,4 +43,20 @@ public final class ReadwriteSplittingStatementTest extends AbstractShardingSpher
             statement.executeQuery("");
         }
     }
+    
+    @Test
+    public void assertGetGeneratedKeys() throws SQLException {
+        try (Statement statement = getReadwriteSplittingDataSource().getConnection().createStatement()) {
+            statement.executeUpdate("INSERT INTO t_config(status) VALUES('OK');", Statement.RETURN_GENERATED_KEYS);
+            ResultSet generatedKeys = statement.getGeneratedKeys();
+            assertTrue(generatedKeys.next());
+            int columnCount = generatedKeys.getMetaData().getColumnCount();
+            for (int index = 0; index < columnCount; index++) {
+                assertNotNull(generatedKeys.getObject(index + 1));
+                assertNotNull(generatedKeys.getMetaData().getColumnLabel(index + 1));
+                assertNotNull(generatedKeys.getMetaData().getColumnName(index + 1));
+            }
+            assertFalse(generatedKeys.next());
+        }
+    }
 }
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/resources/sql/jdbc_init.sql b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/resources/sql/jdbc_init.sql
index f88819b..28f8537 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/resources/sql/jdbc_init.sql
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/test/resources/sql/jdbc_init.sql
@@ -25,7 +25,7 @@ CREATE TABLE IF NOT EXISTS t_order_auto_1 (order_id INT NOT NULL, user_id INT NO
 CREATE TABLE IF NOT EXISTS t_order_item_auto_0 (item_id INT NOT NULL, order_id INT NOT NULL, user_id INT NOT NULL, status VARCHAR(45) NULL, PRIMARY KEY (item_id));
 CREATE TABLE IF NOT EXISTS t_order_item_auto_1 (item_id INT NOT NULL, order_id INT NOT NULL, user_id INT NOT NULL, status VARCHAR(45) NULL, PRIMARY KEY (item_id));
 
-CREATE TABLE IF NOT EXISTS t_config (id INT NOT NULL, status VARCHAR(45) NULL, PRIMARY KEY (id));
+CREATE TABLE IF NOT EXISTS t_config (id INT NOT NULL AUTO_INCREMENT, status VARCHAR(45) NULL, PRIMARY KEY (id));
 
 CREATE TABLE IF NOT EXISTS t_user_0 (id INT NOT NULL, name VARCHAR(45) NULL, PRIMARY KEY (id));
 CREATE TABLE IF NOT EXISTS t_user_1 (id INT NOT NULL, name VARCHAR(45) NULL, PRIMARY KEY (id));