[GitHub] [shardingsphere] sandynz commented on a change in pull request #14688: For #14604：Fix "Data truncated for column" error occurs during the migration of the year type field

[GitBox <gi...@apache.org>]

sandynz commented on a change in pull request #14688:
URL: https://github.com/apache/shardingsphere/pull/14688#discussion_r781997107



##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/execute/result/query/impl/driver/jdbc/type/memory/JDBCRowsLoader.java
##########
@@ -37,6 +37,10 @@
 @NoArgsConstructor(access = AccessLevel.PRIVATE)
 public final class JDBCRowsLoader {
     
+    private static final String YEAR_DATA_TYPE = "YEAR";
+    
+    private static final String YEAR_DATA_TYPE_SHORT = "java.lang.Short";

Review comment:
       "java.lang.Short" could be replaced to `Short.class.getName()`

##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/execute/result/query/impl/driver/jdbc/type/memory/JDBCRowsLoader.java
##########
@@ -108,4 +116,20 @@ private static Object loadRowValue(final ResultSet resultSet, final int columnIn
                 return resultSet.getObject(columnIndex);
         }
     }
+    
+    private static boolean isYearDataType(final String columnDataTypeName) {
+        return YEAR_DATA_TYPE.equalsIgnoreCase(columnDataTypeName);
+    }
+    
+    private static boolean isYearShortDataType(final String columnClassName) {
+        return YEAR_DATA_TYPE_SHORT.equalsIgnoreCase(columnClassName);
+    }
+    
+    private static Object getYearDateData(final ResultSet resultSet, final int index) throws SQLException {
+        return resultSet.getObject(index) == null ? null : resultSet.getDate(index);
+    }
+    
+    private static Object getYearShortData(final ResultSet resultSet, final int index) throws SQLException {
+        return resultSet.getObject(index) == null ? null : resultSet.getShort(index);
+    }

Review comment:
       It looks a little strange. If `getColumnClassName` could return Date or Short, could `resultSet.getObject` just return the right one instance?

##########
File path: shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/DataConsistencyCheckerImpl.java
##########
@@ -136,6 +138,12 @@ private long count(final DataSource dataSource, final String table, final Databa
         PipelineDataSourceConfiguration targetDataSourceConfig = PipelineDataSourceConfigurationFactory.newInstance(
                 jobContext.getJobConfig().getPipelineConfig().getTarget().getType(), jobContext.getJobConfig().getPipelineConfig().getTarget().getParameter());
         checkDatabaseTypeSupportedOrNot(supportedDatabaseTypes, targetDataSourceConfig.getDatabaseType().getName());
+        if (sourceDataSourceConfig.getDatabaseType().getName().equalsIgnoreCase(new MySQLDatabaseType().getName())) {
+            Properties queryProps = new Properties();
+            queryProps.setProperty("yearIsDateType", Boolean.FALSE.toString());
+            sourceDataSourceConfig.appendJDBCQueryProperties(queryProps);
+            targetDataSourceConfig.appendJDBCQueryProperties(queryProps);
+        }

Review comment:
       Could be extracted to a new method.

##########
File path: shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-dialect/shardingsphere-data-pipeline-mysql/src/main/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/MySQLInventoryDumper.java
##########
@@ -42,7 +44,9 @@ public MySQLInventoryDumper(final InventoryDumperConfiguration inventoryDumperCo
     
     @Override
     public Object readValue(final ResultSet resultSet, final int index) throws SQLException {
-        if (isDateTimeValue(resultSet.getMetaData().getColumnType(index))) {
+        if (isYearDataType(resultSet.getMetaData().getColumnTypeName(index))) {
+            return resultSet.getObject(index) == null ? null : resultSet.getShort(index);
+        } else if (isDateTimeValue(resultSet.getMetaData().getColumnType(index))) {
             return resultSet.getString(index);
         } else {

Review comment:
       `if (isYearDataType(resultSet.getMetaData().getColumnTypeName(index)))` could be put after `if (isDateTimeValue(resultSet.getMetaData().getColumnType(index)))`, to keep the original code is not changed.

##########
File path: shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-dialect/shardingsphere-data-pipeline-mysql/src/main/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/MySQLInventoryDumper.java
##########
@@ -42,7 +44,9 @@ public MySQLInventoryDumper(final InventoryDumperConfiguration inventoryDumperCo
     
     @Override
     public Object readValue(final ResultSet resultSet, final int index) throws SQLException {
-        if (isDateTimeValue(resultSet.getMetaData().getColumnType(index))) {
+        if (isYearDataType(resultSet.getMetaData().getColumnTypeName(index))) {
+            return resultSet.getObject(index) == null ? null : resultSet.getShort(index);

Review comment:
       Could `resultSet.getObject(index) == null ? null : resultSet.getShort(index)` be simplified to `resultSet.getShort(index)` and `resultSet.wasNull()`?
   
   If not, `resultSet.getObject(index) == null` should be `null == resultSet.getObject(index)`.




-- 
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@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org