fsk119 commented on PR #21577:
URL: https://github.com/apache/flink/pull/21577#issuecomment-1369338527
Thanks for the review!
>Since the MODIFY operation is split into a list of changes, I would like to
know whether the order matters and how the external catalog copes with them.
For example, does API expose a requirement that it should be an atomic
operation, or ? does the external catalog decide it?
Yes, the order matters. The catalog developer needs to make sure the
execution in order and the execution is atomic. The logic may looks like(the
idea is from the Spark JdbcTablecatalog#alterTable)
```
default void alterTable(
ObjectPath tablePath,
CatalogBaseTable newTable,
List<TableChange> tableChanges,
boolean ignoreIfNotExists)
throws TableNotExistException, CatalogException {
// make sure all table changes are supported.
validate(tableChanges);
// jdbc catalog can work like
conn.setAutoCommit(false);
Statement statement = conn.createStatement()
try {
statement.setQueryTimeout(options.queryTimeout);
for (String sql = dialect.alterTable(tableName, changes,
metaData.getDatabaseMajorVersion)) {
statement.executeUpdate(sql)
}
conn.commit()
} catch (Exception e) {
if (conn != null) {
conn.rollback();
}
throw e;
} finally {
statement.close();
conn.setAutoCommit(true);
}
}
```
> The "oldColumn" and "originColumn" are interchangeably used across the
changelog, and can we align them to improve the readability?
Thanks for pointing it out. I agree with you we should align the term in the
codes. I am prone to use the term "oldColumn" instead because we already have
used this in the FLIP-273. WDYT?
> SchemaConverter#updatePositionAndCollectColumnChange and
OperationConverterUtils#buildColumnChange are very similar. Do we have a plan
to do some refactor work?
Yes. I think we can refactor this after all parts finish. WDYT?
--
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]
