gyang94 commented on code in PR #1760:
URL: https://github.com/apache/fluss/pull/1760#discussion_r2380809019
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -308,52 +308,62 @@ public void alterTableProperties(
TablePath tablePath,
TablePropertyChanges tablePropertyChanges,
boolean ignoreIfNotExists) {
- if (!databaseExists(tablePath.getDatabaseName())) {
- throw new DatabaseNotExistException(
- "Database " + tablePath.getDatabaseName() + " does not
exist.");
- }
- if (!tableExists(tablePath)) {
- if (ignoreIfNotExists) {
- return;
- } else {
- throw new TableNotExistException("Table " + tablePath + " does
not exists.");
- }
- }
-
try {
- TableRegistration updatedTableRegistration =
- getUpdatedTableRegistration(tablePath,
tablePropertyChanges);
- if (updatedTableRegistration != null) {
+ // it throws TableNotExistException if the table or database not
exists
+ TableRegistration tableReg = getTableRegistration(tablePath);
+ SchemaInfo schemaInfo = getLatestSchema(tablePath);
+ // we can't use MetadataManager#getTable here, because it will add
the default
+ // lake options to the table properties, which may cause the
validation failure
Review Comment:
A small question: now it looks those default properties won't affect the
`validateAlterTableProperties` validation below.
When will those default values make validation failure? Default values
should be exactly what takes effect to a table.
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/FlinkConnectorOptions.java:
##########
@@ -132,6 +133,13 @@ public class FlinkConnectorOptions {
public static final List<ConfigOption<?>> CLIENT_OPTIONS =
FlinkConversions.toFlinkOptions(FlussConfigUtils.CLIENT_OPTIONS.values());
+ //
--------------------------------------------------------------------------------------------
+ // modification disallowed connector options
+ //
--------------------------------------------------------------------------------------------
+
+ public static final List<String> ALTER_DISALLOW_OPTIONS =
+ Arrays.asList(BUCKET_NUMBER.key(), BUCKET_KEY.key(),
BOOTSTRAP_SERVERS.key());
Review Comment:
Should fluss server side also keeps a disallow list like this?
--
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]
