wuchong commented on code in PR #1760:
URL: https://github.com/apache/fluss/pull/1760#discussion_r2381202280
##########
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:
These properties are flink specific options which will be translated into
Fluss `TableDescriptor.TableDistribution` structure. So when supporting
altering these properties, we need to convert to a `TableDistribution` change.
Because Fluss only understand `TableDistribution`, not the `bucket.number` and
`bucket.key` options. These options are custom properties to Fluss server. So
we only validate them in flink connector side, not in server side. Server side
should only handle the `TableDistribution`.
##########
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:
The default datalake properties like `table.datalake.paimon.warehouse=...`.
This is not included in
`org.apache.fluss.config.FlussConfigUtils#TABLE_OPTIONS` and will fail when
running
`org.apache.fluss.server.utils.TableDescriptorValidation#validateTableDescriptor`
with this config. So we exclude the default lake properties for validation,
just like how a table is validated when table is created.
--
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]
