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

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


The following commit(s) were added to refs/heads/master by this push:
     new 076a11f3f1f [improvement](jdbc catalog) Delete unnecessary schema and 
optimize insert logic (#30880)
076a11f3f1f is described below

commit 076a11f3f1f495016c01ff554eb3baec27ae74fb
Author: zy-kkk <zhongy...@gmail.com>
AuthorDate: Sun Feb 18 15:13:42 2024 +0800

    [improvement](jdbc catalog) Delete unnecessary schema and optimize insert 
logic (#30880)
    
    In the previous design, we were compatible with MySQL's auto-increment 
column and default value to bypass the null value check when writing back Jdbc 
External Table. However, because MySQL's default value is not completely 
unified with Doris, this resulted in The unsuitable default value is wrong. In 
response to this situation, I made the following optimizations
    1. For JDBC External Table, we always allow certain columns to be missing 
during insertion. Even if these columns are not allowed to be empty at the 
source end, the error should be generated by the source end, not Doris herself.
    2. When the target column is non-nullable and the insertion is done via 
`INSERT INTO tbl VALUES()` or `INSERT INTO tbl SELECT constants`, Doris should 
verify any inconsistency between them and throw an exception. This check is not 
applied for `INSERT INTO tbl SELECT ... FROM tbl` operations.
---
 .../apache/doris/analysis/NativeInsertStmt.java    | 33 +++++++++++++-
 .../doris/datasource/jdbc/client/JdbcClient.java   |  6 +--
 .../datasource/jdbc/client/JdbcMySQLClient.java    | 50 +---------------------
 .../datasource/jdbc/client/JdbcOracleClient.java   |  1 -
 .../jdbc/test_mysql_jdbc_catalog.out               |  9 ++--
 .../jdbc/test_mysql_jdbc_catalog.groovy            | 33 +++++++++++++-
 6 files changed, 72 insertions(+), 60 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/NativeInsertStmt.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/NativeInsertStmt.java
index 717f592c458..843ca198abe 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/NativeInsertStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/NativeInsertStmt.java
@@ -664,7 +664,10 @@ public class NativeInsertStmt extends InsertStmt {
         }
 
         // Check if all columns mentioned is enough
-        checkColumnCoverage(mentionedColumns, targetTable.getBaseSchema());
+        // For JdbcTable, it is allowed to insert without specifying all 
columns and without checking
+        if (!(targetTable instanceof JdbcTable)) {
+            checkColumnCoverage(mentionedColumns, targetTable.getBaseSchema());
+        }
 
         realTargetColumnNames = 
targetColumns.stream().map(Column::getName).collect(Collectors.toList());
 
@@ -675,6 +678,21 @@ public class NativeInsertStmt extends InsertStmt {
                 // INSERT INTO VALUES(...)
                 List<ArrayList<Expr>> rows = 
selectStmt.getValueList().getRows();
                 for (int rowIdx = 0; rowIdx < rows.size(); ++rowIdx) {
+                    // Only check for JdbcTable
+                    if (targetTable instanceof JdbcTable) {
+                        // Check for NULL values in not-nullable columns
+                        for (int colIdx = 0; colIdx < targetColumns.size(); 
++colIdx) {
+                            Column column = targetColumns.get(colIdx);
+                            // Ensure rows.get(rowIdx) has enough columns to 
match targetColumns
+                            if (colIdx < rows.get(rowIdx).size()) {
+                                Expr expr = rows.get(rowIdx).get(colIdx);
+                                if (!column.isAllowNull() && expr instanceof 
NullLiteral) {
+                                    throw new AnalysisException("Column `" + 
column.getName()
+                                            + "` is not nullable, but the 
inserted value is nullable.");
+                                }
+                            }
+                        }
+                    }
                     analyzeRow(analyzer, targetColumns, rows, rowIdx, 
origColIdxsForExtendCols, realTargetColumnNames,
                             skipCheck);
                 }
@@ -698,6 +716,19 @@ public class NativeInsertStmt extends InsertStmt {
                         skipCheck);
                 // rows may be changed in analyzeRow(), so rebuild the result 
exprs
                 selectStmt.getResultExprs().clear();
+
+                // For JdbcTable, need to check whether there is a NULL value 
inserted into the NOT NULL column
+                if (targetTable instanceof JdbcTable) {
+                    for (int colIdx = 0; colIdx < targetColumns.size(); 
++colIdx) {
+                        Column column = targetColumns.get(colIdx);
+                        Expr expr = rows.get(0).get(colIdx);
+                        if (!column.isAllowNull() && expr instanceof 
NullLiteral) {
+                            throw new AnalysisException("Column `" + 
column.getName()
+                                    + "` is not nullable, but the inserted 
value is nullable.");
+                        }
+                    }
+                }
+
                 for (Expr expr : rows.get(0)) {
                     selectStmt.getResultExprs().add(expr);
                 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
index 07706ace822..6112398a096 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
@@ -325,7 +325,6 @@ public abstract class JdbcClient {
                    We used this method to retrieve the key column of the JDBC 
table, but since we only tested mysql,
                    we kept the default key behavior in the parent class and 
only overwrite it in the mysql subclass
                 */
-                field.setKey(true);
                 field.setColumnSize(rs.getInt("COLUMN_SIZE"));
                 field.setDecimalDigits(rs.getInt("DECIMAL_DIGITS"));
                 field.setNumPrecRadix(rs.getInt("NUM_PREC_RADIX"));
@@ -354,7 +353,7 @@ public abstract class JdbcClient {
         List<Column> dorisTableSchema = 
Lists.newArrayListWithCapacity(jdbcTableSchema.size());
         for (JdbcFieldSchema field : jdbcTableSchema) {
             dorisTableSchema.add(new Column(field.getColumnName(),
-                    jdbcTypeToDoris(field), field.isKey, null,
+                    jdbcTypeToDoris(field), true, null,
                     field.isAllowNull(), field.getRemarks(),
                     true, -1));
         }
@@ -503,7 +502,6 @@ public abstract class JdbcClient {
         protected int dataType;
         // The SQL type of the corresponding java.sql.types (Type Name)
         protected String dataTypeName;
-        protected boolean isKey;
         // For CHAR/DATA, columnSize means the maximum number of chars.
         // For NUMERIC/DECIMAL, columnSize means precision.
         protected int columnSize;
@@ -517,8 +515,6 @@ public abstract class JdbcClient {
         // because for utf8 encoding, a Chinese character takes up 3 bytes
         protected int charOctetLength;
         protected boolean isAllowNull;
-        protected long autoIncInitValue;
-        protected String defaultValue;
     }
 
     protected abstract Type jdbcTypeToDoris(JdbcFieldSchema fieldSchema);
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
index 3a1807ce24f..ace29c4198f 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
@@ -17,9 +17,7 @@
 
 package org.apache.doris.datasource.jdbc.client;
 
-import org.apache.doris.analysis.DefaultValueExprDef;
 import org.apache.doris.catalog.ArrayType;
-import org.apache.doris.catalog.Column;
 import org.apache.doris.catalog.PrimitiveType;
 import org.apache.doris.catalog.ScalarType;
 import org.apache.doris.catalog.Type;
@@ -117,14 +115,13 @@ public class JdbcMySQLClient extends JdbcClient {
     public List<JdbcFieldSchema> getJdbcColumnsInfo(String dbName, String 
tableName) {
         Connection conn = getConnection();
         ResultSet rs = null;
-        List<JdbcFieldSchema> tableSchema = 
com.google.common.collect.Lists.newArrayList();
+        List<JdbcFieldSchema> tableSchema = Lists.newArrayList();
         String finalDbName = getRealDatabaseName(dbName);
         String finalTableName = getRealTableName(dbName, tableName);
         try {
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             rs = getColumns(databaseMetaData, catalogName, finalDbName, 
finalTableName);
-            List<String> primaryKeys = getPrimaryKeys(databaseMetaData, 
catalogName, dbName, tableName);
             Map<String, String> mapFieldtoType = null;
             while (rs.next()) {
                 lowerColumnToRealColumn.putIfAbsent(finalDbName, new 
ConcurrentHashMap<>());
@@ -150,7 +147,6 @@ public class JdbcMySQLClient extends JdbcClient {
                     mapFieldtoType = getColumnsDataTypeUseQuery(dbName, 
tableName);
                     
field.setDataTypeName(mapFieldtoType.get(rs.getString("COLUMN_NAME")));
                 }
-                field.setKey(primaryKeys.contains(field.getColumnName()));
                 field.setColumnSize(rs.getInt("COLUMN_SIZE"));
                 field.setDecimalDigits(rs.getInt("DECIMAL_DIGITS"));
                 field.setNumPrecRadix(rs.getInt("NUM_PREC_RADIX"));
@@ -163,9 +159,6 @@ public class JdbcMySQLClient extends JdbcClient {
                 field.setAllowNull(rs.getInt("NULLABLE") != 0);
                 field.setRemarks(rs.getString("REMARKS"));
                 field.setCharOctetLength(rs.getInt("CHAR_OCTET_LENGTH"));
-                String isAutoincrement = rs.getString("IS_AUTOINCREMENT");
-                
field.setAutoIncInitValue("YES".equalsIgnoreCase(isAutoincrement) ? 1 : -1);
-                field.setDefaultValue(rs.getString("COLUMN_DEF"));
                 tableSchema.add(field);
             }
         } catch (SQLException e) {
@@ -177,47 +170,6 @@ public class JdbcMySQLClient extends JdbcClient {
         return tableSchema;
     }
 
-    @Override
-    public List<Column> getColumnsFromJdbc(String dbName, String tableName) {
-        List<JdbcFieldSchema> jdbcTableSchema = getJdbcColumnsInfo(dbName, 
tableName);
-        List<Column> dorisTableSchema = 
Lists.newArrayListWithCapacity(jdbcTableSchema.size());
-        for (JdbcFieldSchema field : jdbcTableSchema) {
-            DefaultValueExprDef defaultValueExprDef = null;
-            if (field.getDefaultValue() != null) {
-                String colDefaultValue = field.getDefaultValue().toLowerCase();
-                // current_timestamp()
-                if (colDefaultValue.startsWith("current_timestamp")) {
-                    long precision = 0;
-                    if (colDefaultValue.contains("(")) {
-                        String substring = colDefaultValue.substring(18, 
colDefaultValue.length() - 1).trim();
-                        precision = substring.isEmpty() ? 0 : 
Long.parseLong(substring);
-                    }
-                    defaultValueExprDef = new DefaultValueExprDef("now", 
precision);
-                }
-            }
-            dorisTableSchema.add(new Column(field.getColumnName(),
-                    jdbcTypeToDoris(field), field.isKey(), null,
-                    field.isAllowNull(), field.getAutoIncInitValue(), 
field.getDefaultValue(), field.getRemarks(),
-                    true, defaultValueExprDef, -1, null));
-        }
-        return dorisTableSchema;
-    }
-
-    protected List<String> getPrimaryKeys(DatabaseMetaData databaseMetaData, 
String catalogName,
-                                          String dbName, String tableName) 
throws SQLException {
-        ResultSet rs = null;
-        List<String> primaryKeys = Lists.newArrayList();
-
-        rs = databaseMetaData.getPrimaryKeys(dbName, null, tableName);
-        while (rs.next()) {
-            String columnName = rs.getString("COLUMN_NAME");
-            primaryKeys.add(columnName);
-        }
-        rs.close();
-
-        return primaryKeys;
-    }
-
     @Override
     protected Type jdbcTypeToDoris(JdbcFieldSchema fieldSchema) {
         // For Doris type
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
index 270b5b4bdc8..a5d2929605a 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
@@ -134,7 +134,6 @@ public class JdbcOracleClient extends JdbcClient {
                    We used this method to retrieve the key column of the JDBC 
table, but since we only tested mysql,
                    we kept the default key behavior in the parent class and 
only overwrite it in the mysql subclass
                 */
-                field.setKey(true);
                 field.setColumnSize(rs.getInt("COLUMN_SIZE"));
                 field.setDecimalDigits(rs.getInt("DECIMAL_DIGITS"));
                 field.setNumPrecRadix(rs.getInt("NUM_PREC_RADIX"));
diff --git 
a/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out 
b/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
index dea68a6b71f..8aa7c81d763 100644
--- a/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
+++ b/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
@@ -220,9 +220,6 @@ triggers
 user_privileges
 views
 
--- !auto_default_t --
-0
-
 -- !dt --
 2023-06-17T10:00       2023-06-17T10:00:01.100 2023-06-17T10:00:02.220 
2023-06-17T10:00:03.333 2023-06-17T10:00:04.444400      
2023-06-17T10:00:05.555550      2023-06-17T10:00:06.666666
 
@@ -410,3 +407,9 @@ year        SMALLINT        Yes     false   \N      NONE
 -- !date_sub_sec --
 2      2022-01-01
 
+-- !auto_default_t1 --
+0
+
+-- !auto_default_t2 --
+0
+
diff --git 
a/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy 
b/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
index 71391ed21ea..473c418b027 100644
--- 
a/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
+++ 
b/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
@@ -175,7 +175,6 @@ suite("test_mysql_jdbc_catalog", 
"p0,external,mysql,external_docker,external_doc
             order_qt_ex_tb21_7  """ select (`key` +1) as k, `id` from 
${ex_tb21} having abs(k) = 2 order by id;"""
             order_qt_ex_tb21_8  """ select `key` as k, `id` from ${ex_tb21} 
having abs(k) = 2 order by id;"""
             order_qt_information_schema """ show tables from 
information_schema; """
-            order_qt_auto_default_t """insert into ${auto_default_t}(name) 
values('a'); """
             order_qt_dt """select * from ${dt}; """
             order_qt_dt_null """select * from ${dt_null} order by 1; """
             order_qt_test_dz """select * from ${test_zd} order by 1; """
@@ -518,6 +517,38 @@ suite("test_mysql_jdbc_catalog", 
"p0,external,mysql,external_docker,external_doc
                        }
                }
         sql """ drop catalog if exists mysql_fun_push_catalog; """
+
+        // test insert null
+
+        sql """drop catalog if exists ${catalog_name} """
+
+        sql """create catalog if not exists ${catalog_name} properties(
+            "type"="jdbc",
+            "user"="root",
+            "password"="123456",
+            "jdbc_url" = 
"jdbc:mysql://${externalEnvIp}:${mysql_port}/doris_test?useSSL=false",
+            "driver_url" = "${driver_url}",
+            "driver_class" = "com.mysql.cj.jdbc.Driver"
+        );"""
+
+        sql """switch ${catalog_name}"""
+        sql """ use ${ex_db_name}"""
+
+        order_qt_auto_default_t1 """insert into ${auto_default_t}(name) 
values('a'); """
+        test {
+            sql "insert into ${auto_default_t}(name,dt) values('a', null);"
+            exception "Column `dt` is not nullable, but the inserted value is 
nullable."
+        }
+        test {
+            sql "insert into ${auto_default_t}(name,dt) select '1', null;"
+            exception "Column `dt` is not nullable, but the inserted value is 
nullable."
+        }
+        explain {
+            sql "insert into ${auto_default_t}(name,dt) select col1,col12 from 
ex_tb15;"
+            contains "PreparedStatement SQL: INSERT INTO 
`doris_test`.`auto_default_t`(`name`,`dt`) VALUES (?, ?)"
+        }
+        order_qt_auto_default_t2 """insert into ${auto_default_t}(name,dt) 
select col1, coalesce(col12,'2022-01-01 00:00:00') from ex_tb15 limit 1;"""
+        sql """drop catalog if exists ${catalog_name} """
     }
 }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to