jiangxt2 commented on code in PR #13493: URL: https://github.com/apache/gravitino/pull/13493#discussion_r4129328841
########## spark-connector/v3.5/spark/src/main/java/org/apache/gravitino/spark/connector/jdbc/doris/DorisPhysicalSchemaValidator35.java: ########## @@ -0,0 +1,364 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.spark.connector.jdbc.doris; + +import com.google.common.collect.ImmutableSet; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Set; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.spark.connector.SparkTypeConverter; +import org.apache.spark.sql.connector.catalog.Identifier; +import org.apache.spark.sql.types.DataType; +import org.apache.spark.sql.types.StructField; +import org.apache.spark.sql.types.StructType; + +/** Validates one authorized Doris table against FE and JDBC physical metadata. */ +final class DorisPhysicalSchemaValidator35 { + + private static final Set<String> ORDINARY_TYPES = + ImmutableSet.of( + "bigint", + "boolean", + "char", + "date", + "datev2", + "datetime", + "datetimev2", + "decimal", + "double", + "float", + "int", + "integer", + "smallint", + "string", + "text", + "tinyint", + "varchar"); + + private DorisPhysicalSchemaValidator35() {} + + static void validate( + Identifier identifier, + Table logicalTable, + StructType sparkPhysicalSchema, + String jdbcUrl, + String jdbcDriver, + String jdbcUser, + String jdbcPassword, + SparkTypeConverter typeConverter) { + try { + Class.forName(jdbcDriver); + try (Connection connection = DriverManager.getConnection(jdbcUrl, jdbcUser, jdbcPassword)) { + List<PhysicalColumn> jdbcColumns = loadJdbcColumns(connection, identifier); + List<PhysicalColumn> feColumns = loadFeColumns(connection, identifier); + validateColumns( + identifier, logicalTable, sparkPhysicalSchema, jdbcColumns, feColumns, typeConverter); + } + } catch (ClassNotFoundException | SQLException | RuntimeException e) { + if (e instanceof IllegalArgumentException + && e.getMessage() != null + && e.getMessage().startsWith("Doris schema mismatch")) { + throw (IllegalArgumentException) e; + } + throw new IllegalArgumentException( + "Doris physical schema validation failed for " + identifier); Review Comment: Schema checks now run outside the metadata-access try/catch, removing message-prefix matching without adding an exception subclass. Physical-access failures report their stage and available safe SQLState/vendor code. I kept raw causes omitted because [Spark 3.5.3’s connection factory](https://github.com/apache/spark/blob/v3.5.3/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L143-L151) can include the JDBC URL in an exception, while #13489 requires connection details to stay out of errors and logs. Driver loading remains delegated to Spark during catalog initialization. The new regression test enters through `GravitinoDorisCatalogSpark35.initialize()` with a nonexistent driver class and asserts the directly propagated `ClassNotFoundException`, the missing class name, and the absence of the JDBC URL and URL/vended password sentinels throughout the printed exception stack. All three catalog unit tests pass. Would you accept this exception boundary and redaction approach? ########## spark-connector/v3.5/spark/src/main/java/org/apache/gravitino/spark/connector/jdbc/doris/DorisPhysicalSchemaValidator35.java: ########## @@ -0,0 +1,364 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.spark.connector.jdbc.doris; + +import com.google.common.collect.ImmutableSet; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Set; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.spark.connector.SparkTypeConverter; +import org.apache.spark.sql.connector.catalog.Identifier; +import org.apache.spark.sql.types.DataType; +import org.apache.spark.sql.types.StructField; +import org.apache.spark.sql.types.StructType; + +/** Validates one authorized Doris table against FE and JDBC physical metadata. */ +final class DorisPhysicalSchemaValidator35 { + + private static final Set<String> ORDINARY_TYPES = Review Comment: The IT now covers all 16 current `ORDINARY_TYPES` spellings: 15 parameterized cases plus a dedicated SMALLINT comparison. Each spelling has NOT NULL and nullable columns, non-null values, and actual NULL values in the nullable column. Aliases may resolve to the same physical metadata type. Plain DATETIME is normalized to precision 0, and DECIMALV3 uses the normalized DECIMAL signature. CHAR and FLOAT remain rejected and documented. The narrowly scoped SMALLINT compatibility rule retains Spark ShortType; its dedicated test compares generic and specialized reads of the same Doris table, including signed bounds, Java Short values, and nullability. Local validation against Doris 3.0.6.2 passed all 22 IT cases. The affected schema-validator suite passed all 14 unit tests, with zero failures, errors, or skips. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
