This is an automated email from the ASF dual-hosted git repository.
zykkk pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 4b5173144c6 [improvement](jdbc catalog) Delete unnecessary schema and
optimize insert logic (#37244)
4b5173144c6 is described below
commit 4b5173144c60cc308a0deaf7d71aae5ec5c02ef1
Author: zy-kkk <zhongy...@gmail.com>
AuthorDate: Thu Jul 4 17:55:02 2024 +0800
[improvement](jdbc catalog) Delete unnecessary schema and optimize insert
logic (#37244)
pick (#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 75e32ae0947..9625db3ea5c 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
@@ -633,7 +633,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());
@@ -644,6 +647,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);
}
@@ -665,6 +683,19 @@ public class NativeInsertStmt extends InsertStmt {
analyzeRow(analyzer, targetColumns, rows, 0,
origColIdxsForExtendCols, realTargetColumnNames);
// 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 30a02cdf6e3..455c9bbc4eb 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));
}
@@ -457,7 +456,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;
@@ -471,8 +469,6 @@ public abstract class JdbcClient {
// because for utf8 encoding, a Chinese character takes up 3 bytes
protected int charOctetLength;
protected boolean isAllowNull;
- protected boolean isAutoincrement;
- 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 ed39c890f7a..44fdd80ecee 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;
@@ -125,7 +123,7 @@ 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();
// if isLowerCaseTableNames == true, tableName is lower case
// but databaseMetaData.getColumns() is case sensitive
String currentDbName = dbName;
@@ -140,7 +138,6 @@ public class JdbcMySQLClient extends JdbcClient {
DatabaseMetaData databaseMetaData = conn.getMetaData();
String catalogName = getCatalogName(conn);
rs = getColumns(databaseMetaData, catalogName, finalDbName,
finalTableName);
- List<String> primaryKeys = getPrimaryKeys(databaseMetaData,
catalogName, finalDbName, finalTableName);
Map<String, String> mapFieldtoType = null;
while (rs.next()) {
JdbcFieldSchema field = new JdbcFieldSchema();
@@ -156,7 +153,6 @@ public class JdbcMySQLClient extends JdbcClient {
mapFieldtoType = getColumnsDataTypeUseQuery(finalDbName,
finalTableName);
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"));
@@ -169,9 +165,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.setAutoincrement("YES".equalsIgnoreCase(isAutoincrement));
- field.setDefaultValue(rs.getString("COLUMN_DEF"));
tableSchema.add(field);
}
} catch (SQLException e) {
@@ -183,47 +176,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.isAutoincrement(),
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 c0debeee601..67b605ef369 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
@@ -137,7 +137,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 f81195948cf..ec726fddc10 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
@@ -254,9 +254,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
@@ -446,3 +443,9 @@ year SMALLINT Yes false \N NONE
-- !sql --
1
+
+-- !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 a0c69fa9923..13359b02775 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; """
@@ -544,6 +543,38 @@ suite("test_mysql_jdbc_catalog",
"p0,external,mysql,external_docker,external_doc
sql """drop catalog if exists mysql_rename2;"""
+ // 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
