coolderli commented on code in PR #4097:
URL: https://github.com/apache/gravitino/pull/4097#discussion_r1688006583
##########
flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/BaseCatalog.java:
##########
@@ -273,10 +275,70 @@ public void createTable(ObjectPath tablePath,
CatalogBaseTable table, boolean ig
}
}
+ /**
+ * The method only is used to change the properties and comments. To alter
columns, use the other
+ * alterTable API and provide a list of TableChange's.
+ *
+ * @param tablePath path of the table or view to be modified
+ * @param newTable the new table definition
+ * @param ignoreIfNotExists flag to specify behavior when the table or view
does not exist: if set
+ * to false, throw an exception, if set to true, do nothing.
+ * @throws TableNotExistException if the table not exists.
+ * @throws CatalogException in case of any runtime exception.
+ */
@Override
- public void alterTable(ObjectPath objectPath, CatalogBaseTable
catalogBaseTable, boolean b)
+ public void alterTable(ObjectPath tablePath, CatalogBaseTable newTable,
boolean ignoreIfNotExists)
throws TableNotExistException, CatalogException {
- throw new UnsupportedOperationException();
+ CatalogBaseTable existingTable;
+
+ try {
+ existingTable = this.getTable(tablePath);
+ } catch (TableNotExistException e) {
+ if (!ignoreIfNotExists) {
+ throw e;
+ }
+ return;
+ }
+
+ if (existingTable.getTableKind() != newTable.getTableKind()) {
+ throw new CatalogException(
+ String.format(
+ "Table types don't match. Existing table is '%s' and new table
is '%s'.",
+ existingTable.getTableKind(), newTable.getTableKind()));
+ }
+
+ NameIdentifier identifier =
+ NameIdentifier.of(tablePath.getDatabaseName(),
tablePath.getObjectName());
+ catalog().asTableCatalog().alterTable(identifier,
getTableChanges(existingTable, newTable));
+ }
+
+ @Override
+ public void alterTable(
+ ObjectPath tablePath,
+ CatalogBaseTable newTable,
+ List<org.apache.flink.table.catalog.TableChange> tableChanges,
+ boolean ignoreIfNotExists)
+ throws TableNotExistException, CatalogException {
+ CatalogBaseTable existingTable;
+ try {
+ existingTable = this.getTable(tablePath);
Review Comment:
I think it is. It's an interface for the user. Users may alter a table to a
view. But it is forbidden in Flink. We can find the details in the description.
```
/**
* Modifies an existing table or view. Note that the new and old {@link
CatalogBaseTable} must
* be of the same kind. For example, this doesn't allow altering a
regular table to partitioned
* table, or altering a view to a table, and vice versa.
*
* <p>The framework will make sure to call this method with fully
validated {@link
* ResolvedCatalogTable} or {@link ResolvedCatalogView}. Those instances
are easy to serialize
* for a durable catalog implementation.
*
* @param tablePath path of the table or view to be modified
* @param newTable the new table definition
* @param ignoreIfNotExists flag to specify behavior when the table or
view does not exist: if
* set to false, throw an exception, if set to true, do nothing.
* @throws TableNotExistException if the table does not exist
* @throws CatalogException in case of any runtime exception
*/
void alterTable(ObjectPath tablePath, CatalogBaseTable newTable, boolean
ignoreIfNotExists)
throws TableNotExistException, CatalogException;
```
--
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]
