sundapeng commented on code in PR #8728:
URL: https://github.com/apache/paimon/pull/8728#discussion_r3610937663
##########
paimon-core/src/main/java/org/apache/paimon/table/format/FormatTableCommit.java:
##########
@@ -192,12 +247,68 @@ private Method getHiveCreatePartitionsInHmsMethod()
throws NoSuchMethodException
private LinkedHashMap<String, String> extractPartitionSpecFromPath(
Review Comment:
🟢 **[suggestion] architecture — 分区 spec 提取逻辑重复**
FormatTableCommit.extractPartitionSpecFromPath(private,约 65 行)重新实现了与
PartitionPathUtils.extractPartitionSpecFromPath(Path,
List<String>)(package-private,同一 MR
中添加)相同的尾部组件遍历算法。两者都向后遍历路径组件、匹配声明的分区键、反转义值、生成 LinkedHashMap。Commit 版本额外处理
value-only 模式并抛出描述性异常而非返回 null。
核心算法(向后遍历、key 校验、反转义)重复,如果任一路径解析契约变更将独立漂移。
**建议**: 将共享的向后遍历-校验算法提取到 PartitionPathUtils 的单一方法中,接受错误处理策略(return-null vs
throw)和 onlyValueInPath 标志。FormatTableCommit 委托给它,通过包装器添加表标识符上下文到错误消息中。
##########
paimon-core/src/main/java/org/apache/paimon/catalog/CatalogUtils.java:
##########
@@ -173,6 +179,7 @@ private static void validateFormatTableOptions(Options
options, boolean dataToke
options.get(PRIMARY_KEY) == null,
"Cannot define %s for format table.",
PRIMARY_KEY.key());
+ validateManagedFormatTableOptions(options);
Review Comment:
🟡 **[minor] [migration]** `validateCreateTable` 现在对所有 catalog 都拒绝了此前无害的
`metastore.partitioned-table=true` + `format-table.implementation=engine` 组合
`validateManagedFormatTableOptions` 被加入
`validateFormatTableOptions`(CatalogUtils.java:182),而该方法在 `validateCreateTable`
中执行 —— 被 AbstractCatalog.createTable(filesystem/hive 基类)、JdbcCatalog 和
RESTCatalog 共同使用。本 MR 之前,在任何 catalog 中同时设置 `metastore.partitioned-table=true` 和
`format-table.implementation=engine` 都会被接受且完全惰性(本 MR 新增文档自己也说明其他 catalog "treat
the option as inert")。本 MR 之后,同样的 CREATE TABLE 语句在**所有** catalog 中都抛
IllegalArgumentException,会打断携带该无害组合的存量 DDL 脚本 / IaC 模板。对 REST managed
场景做硬失败是合理的加固,但对选项仍然惰性的 catalog 一并拒绝是一个向后不兼容的行为变更。
**证据**:CatalogUtils.java:182 在 `validateFormatTableOptions` 中无条件调用
`validateManagedFormatTableOptions(options)`;`validateCreateTable` 被
AbstractCatalog.createTable(AbstractCatalog.java:429)、JdbcCatalog.createTable(JdbcCatalog.java:376)和
RESTCatalog.createTable(RESTCatalog.java:577)调用。
**建议**:将 engine 组合的拒绝限定在实际支持 managed partitions 的 catalog 上(把校验移入
`validateManagedFormatTableCatalog`,或基于
`catalog.supportsManagedFormatTablePartitions()` 门控);或者在 migration/release
notes 中显式说明该组合从"惰性接受"变为"拒绝"。
##########
paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java:
##########
@@ -567,8 +575,13 @@ public void createTable(Identifier identifier, Schema
schema, boolean ignoreIfEx
checkNotBranch(identifier, "createTable");
checkNotSystemTable(identifier, "createTable");
validateCreateTable(schema, dataTokenEnabled);
- createExternalTablePathIfNotExist(schema);
tableDefaultOptions.forEach(schema.options()::putIfAbsent);
Review Comment:
🟡 **[minor] [migration]** createTable 顺序交换:`table-default.path` 现在会参与
external-path 建目录与 managed-table 内外部判定
diff 把 `tableDefaultOptions.forEach(schema.options()::putIfAbsent)` 移到了
`createExternalTablePathIfNotExist` **之前**(此前在其后)。两个副作用:(1) 若 catalog 配置了
`table-default.path`,`createExternalTablePathIfNotExist` 现在会为**每个**建表请求 mkdir
这个默认路径——此前 defaults 对它不可见;(2) `validateManagedFormatTableCatalog` 的 `isExternal
= schema.options().containsKey(PATH.key())` 在 defaults 合并**之后**计算,所以在配置了
`table-default.path` 的 catalog 中创建的内部 managed format table 会被误判为 external
而被拒绝("Managed format table must be an internal table")。两者都是对存量 CREATE TABLE
流程的行为变更,仅在该 catalog 配置下显现,但共享 catalog 配置 `table-default.path` 是合理场景。
**证据**:RESTCatalog.java:578-584:defaults 在 578 合并,externality 在 583 由合并后的
options 推导,`createExternalTablePathIfNotExist` 在
584;`createExternalTablePathIfNotExist`(1373-1383 行)会 mkdir
`options.get(PATH.key())`。父提交中 `createExternalTablePathIfNotExist` 在 defaults
应用之前执行。
**建议**:用用户显式指定的 options(合并 tableDefaultOptions 之前快照 option map)计算
externality;`createExternalTablePathIfNotExist` 也保持在合并 defaults 之前的 schema
上执行,或显式排除把 `table-default.path` 当作 external 标记。
--
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]