wuchong commented on code in PR #1503:
URL: https://github.com/apache/fluss/pull/1503#discussion_r2265296349
##########
fluss-server/src/main/java/com/alibaba/fluss/server/coordinator/CoordinatorService.java:
##########
@@ -650,4 +657,36 @@ private static List<BucketMetadata>
getBucketMetadataFromContext(
});
return bucketMetadataList;
}
+
+ /**
+ * Validates whether the table creation is allowed based on the table type
and configuration.
+ *
+ * @param tableDescriptor the table descriptor to validate
+ * @param tablePath the table path for error reporting
+ * @throws InvalidTableException if table creation is not allowed
+ */
+ private void validateTableCreationPermission(
+ TableDescriptor tableDescriptor, TablePath tablePath) {
+ boolean hasPrimaryKey = tableDescriptor.hasPrimaryKey();
+
+ if (hasPrimaryKey) {
+ // This is a KV table (Primary Key Table)
+ if (!kvTableAllowCreation) {
+ throw new InvalidTableException(
+ String.format(
+ "Creation of KV tables (primary key tables) is
not allowed. "
+ + "Table: %s. Please set '%s' to true
to enable KV table creation.",
+ tablePath,
ConfigOptions.KV_TABLE_ALLOW_CREATION.key()));
+ }
+ } else {
+ // This is a Log table
+ if (!logTableAllowCreation) {
+ throw new InvalidTableException(
+ String.format(
+ "Creation of Log tables is not allowed. "
+ + "Table: %s. Please set '%s' to true
to enable Log table creation.",
+ tablePath,
ConfigOptions.LOG_TABLE_ALLOW_CREATION.key()));
Review Comment:
ditto
```suggestion
throw new InvalidTableException(
"Creation of Log Tables is disallowed in the
cluster.");
```
##########
fluss-server/src/main/java/com/alibaba/fluss/server/coordinator/CoordinatorService.java:
##########
@@ -650,4 +657,36 @@ private static List<BucketMetadata>
getBucketMetadataFromContext(
});
return bucketMetadataList;
}
+
+ /**
+ * Validates whether the table creation is allowed based on the table type
and configuration.
+ *
+ * @param tableDescriptor the table descriptor to validate
+ * @param tablePath the table path for error reporting
+ * @throws InvalidTableException if table creation is not allowed
+ */
+ private void validateTableCreationPermission(
+ TableDescriptor tableDescriptor, TablePath tablePath) {
+ boolean hasPrimaryKey = tableDescriptor.hasPrimaryKey();
+
+ if (hasPrimaryKey) {
+ // This is a KV table (Primary Key Table)
+ if (!kvTableAllowCreation) {
+ throw new InvalidTableException(
+ String.format(
+ "Creation of KV tables (primary key tables) is
not allowed. "
+ + "Table: %s. Please set '%s' to true
to enable KV table creation.",
+ tablePath,
ConfigOptions.KV_TABLE_ALLOW_CREATION.key()));
Review Comment:
```suggestion
"Creation of Primary Key Tables is disallowed in the
cluster.");
```
1. Use `Primary Key Table` for user-facing messages, we don't have the
concept of KV table actually.
2. remove the cluster config from the exception message, because user
shouldn't be aware of the cluster config and usually don't have the permission
to access cluster configs (so it's not necessary to expose them).
##########
fluss-common/src/main/java/com/alibaba/fluss/config/ConfigOptions.java:
##########
@@ -1309,6 +1309,24 @@ public class ConfigOptions {
"The column name of the version column for the
`versioned` merge engine. "
+ "If the merge engine is set to
`versioned`, the version column must be set.");
+ public static final ConfigOption<Boolean> LOG_TABLE_ALLOW_CREATION =
+ key("log.table.allow-creation")
Review Comment:
How about `allow.create.log.tables` and `allow.create.kv.tables`? This makes
sure both of them be able to be together on docs when we support auto generate
docs in the future.
And please put the ConfigOptions after `AUTO_PARTITION_CHECK_INTERVAL`?
Because `table.*` are table-level configs, not server-side configs.
--
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]
