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