mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1814271776
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -18,34 +18,70 @@
*/
package org.apache.gravitino.catalog.oceanbase.operation;
+import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import java.sql.Connection;
+import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
import org.apache.gravitino.catalog.jdbc.operation.JdbcTableOperations;
-import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils;
+import org.apache.gravitino.exceptions.NoSuchColumnException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.transforms.Transform;
import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Types;
/** Table operations for OceanBase. */
public class OceanBaseTableOperations extends JdbcTableOperations {
+ public static final String BACK_QUOTE = "`";
+ public static final String OCEANBASE_AUTO_INCREMENT = "AUTO_INCREMENT";
Review Comment:
private is enough?
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -58,10 +94,134 @@ protected String generateCreateTableSql(
Distribution distribution,
Index[] indexes) {
if (ArrayUtils.isNotEmpty(partitioning)) {
- throw new UnsupportedOperationException("Currently not support Partition
tables.");
+ throw new UnsupportedOperationException(
+ "Currently we do not support Partitioning in oceanbase");
+ }
+
+ if (!Distributions.NONE.equals(distribution)) {
+ throw new UnsupportedOperationException("OceanBase does not support
distribution");
+ }
+
+ validateIncrementCol(columns, indexes);
Review Comment:
Exactly the same as MySQL, it is recommended to be placed in the super class.
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -93,18 +274,447 @@ protected String generatePurgeTableSql(String tableName) {
@Override
public void alterTable(String databaseName, String tableName, TableChange...
changes)
throws NoSuchTableException {
- throw new UnsupportedOperationException("Not implemented yet.");
+ LOG.info("Attempting to alter table {} from database {}", tableName,
databaseName);
Review Comment:
why need override this method? Can't OBD perform multiple changes in one
statement?
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -72,17 +232,38 @@ protected boolean getAutoIncrementInfo(ResultSet
resultSet) throws SQLException
@Override
protected Map<String, String> getTableProperties(Connection connection,
String tableName)
throws SQLException {
- throw new UnsupportedOperationException("Not implemented yet.");
- }
+ try (PreparedStatement statement = connection.prepareStatement("SHOW TABLE
STATUS LIKE ?")) {
+ statement.setString(1, tableName);
+ try (ResultSet resultSet = statement.executeQuery()) {
+ while (resultSet.next()) {
+ String name = resultSet.getString("NAME");
+ if (Objects.equals(name, tableName)) {
+ return Collections.unmodifiableMap(
+ new HashMap<String, String>() {
+ {
+ put(COMMENT, resultSet.getString(COMMENT));
+ String autoIncrement =
resultSet.getString("AUTO_INCREMENT");
+ if (StringUtils.isNotEmpty(autoIncrement)) {
+ put("AUTO_INCREMENT", autoIncrement);
+ }
+ }
+ });
+ }
+ }
- @Override
- protected String generateRenameTableSql(String oldTableName, String
newTableName) {
- return String.format("RENAME TABLE `%s` TO `%s`", oldTableName,
newTableName);
+ throw new NoSuchTableException(
+ "Table %s does not exist in %s.", tableName,
connection.getCatalog());
+ }
+ }
}
- @Override
- protected String generateDropTableSql(String tableName) {
- return String.format("DROP TABLE `%s`", tableName);
+ protected void correctJdbcTableFields(
+ Connection connection, String databaseName, String tableName,
JdbcTable.Builder tableBuilder)
+ throws SQLException {
+ if (StringUtils.isEmpty(tableBuilder.comment())) {
Review Comment:
Does OBD not put the table comment into the driver's results also?
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -18,34 +18,70 @@
*/
package org.apache.gravitino.catalog.oceanbase.operation;
+import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import java.sql.Connection;
+import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
import org.apache.gravitino.catalog.jdbc.operation.JdbcTableOperations;
-import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils;
+import org.apache.gravitino.exceptions.NoSuchColumnException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.transforms.Transform;
import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Types;
/** Table operations for OceanBase. */
public class OceanBaseTableOperations extends JdbcTableOperations {
+ public static final String BACK_QUOTE = "`";
+ public static final String OCEANBASE_AUTO_INCREMENT = "AUTO_INCREMENT";
+ private static final String OCEANBASE_NOT_SUPPORT_NESTED_COLUMN_MSG =
+ "OceanBase does not support nested column names.";
+
@Override
public List<String> listTables(String databaseName) throws
NoSuchSchemaException {
Review Comment:
Suggest using MySQL as the default implementation, because Doris and OBD are
the same.
##########
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##########
@@ -58,10 +94,134 @@ protected String generateCreateTableSql(
Distribution distribution,
Index[] indexes) {
if (ArrayUtils.isNotEmpty(partitioning)) {
- throw new UnsupportedOperationException("Currently not support Partition
tables.");
+ throw new UnsupportedOperationException(
+ "Currently we do not support Partitioning in oceanbase");
+ }
+
+ if (!Distributions.NONE.equals(distribution)) {
+ throw new UnsupportedOperationException("OceanBase does not support
distribution");
+ }
+
+ validateIncrementCol(columns, indexes);
+ StringBuilder sqlBuilder = new StringBuilder();
+ sqlBuilder.append(String.format("CREATE TABLE `%s` (\n", tableName));
+
+ // Add columns
+ for (int i = 0; i < columns.length; i++) {
+ JdbcColumn column = columns[i];
+ sqlBuilder
+ .append(SPACE)
+ .append(SPACE)
+ .append(BACK_QUOTE)
+ .append(column.name())
+ .append(BACK_QUOTE);
+
+ appendColumnDefinition(column, sqlBuilder);
+ // Add a comma for the next column, unless it's the last one
+ if (i < columns.length - 1) {
+ sqlBuilder.append(",\n");
+ }
+ }
+
+ appendIndexesSql(indexes, sqlBuilder);
+
+ sqlBuilder.append("\n)");
+
+ // Add table comment if specified
+ if (StringUtils.isNotEmpty(comment)) {
+ sqlBuilder.append(" COMMENT='").append(comment).append("'");
+ }
+
+ // Add table properties
+ if (MapUtils.isNotEmpty(properties)) {
+ sqlBuilder.append(
+ properties.entrySet().stream()
+ .map(entry -> String.format("%s = %s", entry.getKey(),
entry.getValue()))
+ .collect(Collectors.joining(",\n", "\n", "")));
+ }
+
+ // Return the generated SQL statement
+ String result = sqlBuilder.append(";").toString();
+
+ LOG.info("Generated create table:{} sql: {}", tableName, result);
+ return result;
+ }
+
+ /**
+ * The auto-increment column will be verified. There can only be one
auto-increment column and it
+ * must be the primary key or unique index.
+ *
+ * @param columns jdbc column
+ * @param indexes table indexes
+ */
+ private static void validateIncrementCol(JdbcColumn[] columns, Index[]
indexes) {
+ // Check auto increment column
+ List<JdbcColumn> autoIncrementCols =
+
Arrays.stream(columns).filter(Column::autoIncrement).collect(Collectors.toList());
+ String autoIncrementColsStr =
+
autoIncrementCols.stream().map(JdbcColumn::name).collect(Collectors.joining(",",
"[", "]"));
+ Preconditions.checkArgument(
+ autoIncrementCols.size() <= 1,
+ "Only one column can be auto-incremented. There are multiple
auto-increment columns in your table: "
+ + autoIncrementColsStr);
+ if (!autoIncrementCols.isEmpty()) {
+ Optional<Index> existAutoIncrementColIndexOptional =
+ Arrays.stream(indexes)
+ .filter(
+ index ->
+ Arrays.stream(index.fieldNames())
+ .flatMap(Arrays::stream)
+ .anyMatch(
+ s ->
+
StringUtils.equalsIgnoreCase(autoIncrementCols.get(0).name(), s)))
+ .filter(
+ index ->
+ index.type() == Index.IndexType.PRIMARY_KEY
+ || index.type() == Index.IndexType.UNIQUE_KEY)
+ .findAny();
+ Preconditions.checkArgument(
+ existAutoIncrementColIndexOptional.isPresent(),
+ "Incorrect table definition; there can be only one auto column and
it must be defined as a key");
+ }
+ }
+
+ public static void appendIndexesSql(Index[] indexes, StringBuilder
sqlBuilder) {
+ for (Index index : indexes) {
+ String fieldStr = getIndexFieldStr(index.fieldNames());
+ sqlBuilder.append(",\n");
+ switch (index.type()) {
+ case PRIMARY_KEY:
+ if (null != index.name()
+ && !StringUtils.equalsIgnoreCase(
+ index.name(), Indexes.DEFAULT_MYSQL_PRIMARY_KEY_NAME)) {
+ throw new IllegalArgumentException("Primary key name must be
PRIMARY in OceanBase");
+ }
+ sqlBuilder.append("CONSTRAINT ").append("PRIMARY KEY
(").append(fieldStr).append(")");
+ break;
+ case UNIQUE_KEY:
+ sqlBuilder.append("CONSTRAINT ");
+ if (null != index.name()) {
+
sqlBuilder.append(BACK_QUOTE).append(index.name()).append(BACK_QUOTE);
+ }
+ sqlBuilder.append(" UNIQUE (").append(fieldStr).append(")");
+ break;
+ default:
+ throw new IllegalArgumentException("OceanBase doesn't support index
: " + index.type());
+ }
}
+ }
- throw new UnsupportedOperationException("Not implemented yet.");
+ private static String getIndexFieldStr(String[][] fieldNames) {
Review Comment:
also here
--
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]
