sundapeng commented on code in PR #8728:
URL: https://github.com/apache/paimon/pull/8728#discussion_r3610936528
##########
paimon-core/src/main/java/org/apache/paimon/table/FormatTable.java:
##########
@@ -265,6 +308,27 @@ public FileIO fileIO() {
public FormatTable copy(Map<String, String> dynamicOptions) {
Map<String, String> newOptions = new HashMap<>(options);
newOptions.putAll(dynamicOptions);
+
+ CoreOptions coreOptions = CoreOptions.fromMap(options);
+ CoreOptions copiedCoreOptions = CoreOptions.fromMap(newOptions);
+ boolean managed = coreOptions.partitionedTableInMetastore();
+ boolean copiedManaged =
copiedCoreOptions.partitionedTableInMetastore();
+ if (managed != copiedManaged) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Dynamic option '%s' cannot change whether
Format Table partitions are catalog-managed.",
+
CoreOptions.METASTORE_PARTITIONED_TABLE.key()));
+ }
+ if (managed
+ && coreOptions.formatTablePartitionOnlyValueInPath()
+ !=
copiedCoreOptions.formatTablePartitionOnlyValueInPath()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Dynamic option '%s' cannot change the
physical partition layout of a catalog-managed Format Table.",
+
CoreOptions.FORMAT_TABLE_PARTITION_ONLY_VALUE_IN_PATH.key()));
+ }
+ CatalogUtils.validateManagedFormatTableOptions(newOptions);
Review Comment:
🟡 **[minor] architecture — 包级循环依赖**
FormatTable(公共表 API,带 @Public 注解)现在向上依赖
CatalogUtils,形成包级循环依赖:FormatTable.FormatTableImpl.copy() 调用
CatalogUtils.validateManagedFormatTableOptions(newOptions),而 CatalogUtils 已经
import 了 FormatTable 和 FormatTableCatalogProvider。
table 层是公共 API 表面,不应反向依赖 catalog 层的工具类做校验。该校验本身(检查
METASTORE_PARTITIONED_TABLE=true 不能与 format-table.implementation=engine
组合)是纯选项级约束。
**建议**: 将 validateManagedFormatTableOptions 移到 CoreOptions(两层都已依赖)或
table.format 包内的小型 validator 中,打破循环依赖,保持公共表 API 自包含。
--
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]