Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
jerryshao merged PR #5265: URL: https://github.com/apache/gravitino/pull/5265 -- 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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
jerryshao closed pull request #5265: [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog URL: https://github.com/apache/gravitino/pull/5265 -- 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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades merged PR #5228: URL: https://github.com/apache/gravitino/pull/5228 -- 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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815955854
##
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##
@@ -72,39 +160,497 @@ protected boolean getAutoIncrementInfo(ResultSet
resultSet) throws SQLException
@Override
protected Map 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() {
+ {
+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())) {
+ tableBuilder.withComment(
+ tableBuilder.properties().getOrDefault(COMMENT,
tableBuilder.comment()));
+}
}
@Override
protected String generatePurgeTableSql(String tableName) {
return String.format("TRUNCATE TABLE `%s`", tableName);
}
+ /**
+ * OceanBase does not support some multiple changes in one statement, So
rewrite this method, one
+ * by one to apply TableChange to the table.
+ *
+ * @param databaseName The name of the database.
+ * @param tableName The name of the table.
+ * @param changes The changes to apply to the table.
+ */
@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);
+try (Connection connection = getConnection(databaseName)) {
+ for (TableChange change : changes) {
+String sql = generateAlterTableSql(databaseName, tableName, change);
+if (StringUtils.isEmpty(sql)) {
+ LOG.info("No changes to alter table {} from database {}", tableName,
databaseName);
+ return;
+}
+JdbcConnectorUtils.executeUpdate(connection, sql);
+ }
+ LOG.info("Alter table {} from database {}", tableName, databaseName);
+} catch (final SQLException se) {
+ throw this.exceptionMapper.toGravitinoException(se);
+}
}
@Override
protected String generateAlterTableSql(
String databaseName, String tableName, TableChange... changes) {
-throw new UnsupportedOperationException("Not implemented yet.");
+// Not all operations require the original table information, so lazy
loading is used here
+JdbcTable lazyLoadTable = null;
+TableChange.UpdateComment updateComment = null;
+List setProperties = new ArrayList<>();
+List alterSql = new ArrayList<>();
+for (TableChange change : changes) {
+ if (change instanceof TableChange.UpdateComment) {
+updateComment = (TableChange.UpdateComment) change;
+ } else if (change instanceof TableChange.SetProperty) {
+// The set attribute needs to be added at the end.
+setProperties.add(((TableChange.SetProperty) change));
+ } else if (change instanceof TableChange.RemoveProperty) {
+// OceanBase does not support deleting table attributes, it can be
replaced by Set Property
+throw new IllegalArgumentException("Remove property is not supported
yet");
+ } else if (change instanceof TableChange.AddColumn) {
+TableChange.AddColumn addColumn = (TableChange.AddColumn) change;
+lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+alterSql.add(addColumnFieldDefinition(addColumn));
+ } else if (change instanceof TableChange.RenameColumn)
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815955854
##
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##
@@ -72,39 +160,497 @@ protected boolean getAutoIncrementInfo(ResultSet
resultSet) throws SQLException
@Override
protected Map 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() {
+ {
+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())) {
+ tableBuilder.withComment(
+ tableBuilder.properties().getOrDefault(COMMENT,
tableBuilder.comment()));
+}
}
@Override
protected String generatePurgeTableSql(String tableName) {
return String.format("TRUNCATE TABLE `%s`", tableName);
}
+ /**
+ * OceanBase does not support some multiple changes in one statement, So
rewrite this method, one
+ * by one to apply TableChange to the table.
+ *
+ * @param databaseName The name of the database.
+ * @param tableName The name of the table.
+ * @param changes The changes to apply to the table.
+ */
@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);
+try (Connection connection = getConnection(databaseName)) {
+ for (TableChange change : changes) {
+String sql = generateAlterTableSql(databaseName, tableName, change);
+if (StringUtils.isEmpty(sql)) {
+ LOG.info("No changes to alter table {} from database {}", tableName,
databaseName);
+ return;
+}
+JdbcConnectorUtils.executeUpdate(connection, sql);
+ }
+ LOG.info("Alter table {} from database {}", tableName, databaseName);
+} catch (final SQLException se) {
+ throw this.exceptionMapper.toGravitinoException(se);
+}
}
@Override
protected String generateAlterTableSql(
String databaseName, String tableName, TableChange... changes) {
-throw new UnsupportedOperationException("Not implemented yet.");
+// Not all operations require the original table information, so lazy
loading is used here
+JdbcTable lazyLoadTable = null;
+TableChange.UpdateComment updateComment = null;
+List setProperties = new ArrayList<>();
+List alterSql = new ArrayList<>();
+for (TableChange change : changes) {
+ if (change instanceof TableChange.UpdateComment) {
+updateComment = (TableChange.UpdateComment) change;
+ } else if (change instanceof TableChange.SetProperty) {
+// The set attribute needs to be added at the end.
+setProperties.add(((TableChange.SetProperty) change));
+ } else if (change instanceof TableChange.RemoveProperty) {
+// OceanBase does not support deleting table attributes, it can be
replaced by Set Property
+throw new IllegalArgumentException("Remove property is not supported
yet");
+ } else if (change instanceof TableChange.AddColumn) {
+TableChange.AddColumn addColumn = (TableChange.AddColumn) change;
+lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+alterSql.add(addColumnFieldDefinition(addColumn));
+ } else if (change instanceof TableChange.RenameColumn)
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815924753
##
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:
Refactored.
##
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 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 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
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on PR #5228: URL: https://github.com/apache/gravitino/pull/5228#issuecomment-2436766547 Overall LGTM. plz fix the CI -- 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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815925397
##
catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java:
##
@@ -0,0 +1,997 @@
+/*
+ * 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.catalog.oceanbase.operation;
+
+import java.time.LocalDateTime;
+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.stream.Collectors;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.expressions.transforms.Transforms;
+import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Decimal;
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.utils.RandomNameUtils;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag("gravitino-docker-test")
+public class TestOceanBaseTableOperations extends TestOceanBase {
+ private static Type VARCHAR = Types.VarCharType.of(255);
+ private static Type INT = Types.IntegerType.get();
Review Comment:
Also Fixed in TestMysqlTableOperations.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815924965
##
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:
Added.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815924129
##
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:
Also fixed in MysqlTableOperations.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815924382
##
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 listTables(String databaseName) throws
NoSuchSchemaException {
Review Comment:
Fixed.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815899843
##
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:
ok, plz add comment for the code
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1815883261
##
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:
Yes, OB does not support some multiple changes in one statement, such as the
following:
```
ALTER TABLE test
ADD COLUMN pro_id INT PRIMARY key,
ADD COLUMN order_id int;
ALTER TABLE `auto_increment_table`
ADD COLUMN `col_6` bigint AUTO_INCREMENT NOT NULL COMMENT 'id' ,
ADD PRIMARY KEY (`col_6`);
```
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1814456642
##
catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java:
##
@@ -0,0 +1,997 @@
+/*
+ * 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.catalog.oceanbase.operation;
+
+import java.time.LocalDateTime;
+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.stream.Collectors;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.expressions.transforms.Transforms;
+import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Decimal;
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.utils.RandomNameUtils;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag("gravitino-docker-test")
+public class TestOceanBaseTableOperations extends TestOceanBase {
+ private static Type VARCHAR = Types.VarCharType.of(255);
+ private static Type INT = Types.IntegerType.get();
Review Comment:
plz make them final
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1814389378
##
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 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() {
+ {
+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:
Currently OB is mainly compatible with MySQL 5.7.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1814386933
##
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 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() {
+ {
+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:
Yes, I tested several versions of ob.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
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 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() {
+ {
+put(COMMENT, resultSet.getString(COMMENT));
+String autoIncrement =
resultSet.getString("AUTO_INCREMENT");
+if
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1812406241
##
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##
@@ -58,10 +93,130 @@ 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");
+}
Review Comment:
Supports indexes, but not support distribution.
A check will be added for OceanBase does not support distribution.
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
mchades commented on code in PR #5228:
URL: https://github.com/apache/gravitino/pull/5228#discussion_r1812371020
##
catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java:
##
@@ -58,10 +93,130 @@ 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");
+}
Review Comment:
Also do not support distribution and indexes, right?
--
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]
Re: [PR] [#5227] feat(oceanbase-catalog): Support table operations for OceanBase JDBC catalog [gravitino]
yuanoOo commented on PR #5228: URL: https://github.com/apache/gravitino/pull/5228#issuecomment-2431214380 @mchades PTAL. > could you plz add integration tests to cover the new feature? I plan to put it into the next subtask. -- 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]
