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 自包含。
##########
paimon-core/src/main/java/org/apache/paimon/utils/PartitionPathUtils.java:
##########
@@ -109,13 +109,105 @@ public static String generatePartitionPathUtil(
suffixBuf.append(escapePathName(e.getKey()));
suffixBuf.append('=');
}
- suffixBuf.append(escapePathName(e.getValue()));
+ String value = e.getValue();
+ validatePartitionValueForPath(value, onlyValue);
+ suffixBuf.append(escapePathName(value));
i++;
}
suffixBuf.append(Path.SEPARATOR);
return suffixBuf.toString();
}
+ /**
+ * Generate a partition path without the trailing separator, e.g. {@code
dt=20250101/hr=01}.
+ * This is the canonical partition name used when talking to a
partition-managing catalog.
+ */
+ public static String generatePartitionName(
+ LinkedHashMap<String, String> partitionSpec, boolean onlyValue) {
+ String path = generatePartitionPathUtil(partitionSpec, onlyValue);
+ return path.endsWith(Path.SEPARATOR)
+ ? path.substring(0, path.length() - Path.SEPARATOR.length())
+ : path;
+ }
+
+ /**
+ * Validate that a partition value is safe for the configured path layout.
In a key-value
+ * layout, values such as {@code "."} are part of a component such as
{@code "pt=."} and are
+ * safe. In a value-only layout, {@code "."} and {@code ".."} are complete
path components and
+ * would resolve to a different directory.
+ */
+ public static void validatePartitionValueForPath(String value, boolean
onlyValueInPath) {
+ if (value == null
+ || value.isEmpty()
+ || (onlyValueInPath && (".".equals(value) ||
"..".equals(value)))) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Partition value '%s' cannot be used as a
partition path component.",
+ value));
+ }
+ }
+
+ /** Conservatively validate a value when the physical partition layout is
unknown. */
+ public static void validatePartitionValueForPath(String value) {
+ validatePartitionValueForPath(value, true);
+ }
+
+ /** Validate every value of a partition spec for the configured path
layout. */
+ public static void validatePartitionSpecForPath(
+ Map<String, String> partitionSpec, boolean onlyValueInPath) {
+ for (String value : partitionSpec.values()) {
+ validatePartitionValueForPath(value, onlyValueInPath);
+ }
+ }
+
+ /** Conservatively validate a spec when the physical partition layout is
unknown. */
+ public static void validatePartitionSpecForPath(Map<String, String>
partitionSpec) {
+ validatePartitionSpecForPath(partitionSpec, true);
+ }
+
+ /**
+ * Build the partition-name prefix pattern pushed down to a
partition-managing catalog from the
+ * leading equality prefix of a partition predicate.
+ *
+ * <p>Pattern contract (shared by every engine talking to the catalog):
partition names are the
+ * escaped {@code key=value} form joined by {@code '/'}; {@code '%'} is
the only wildcard and
+ * there is no escape sequence for it ({@code '_'} stays a literal). A
complete spec matches the
+ * exact partition name; an incomplete prefix is suffixed with {@code '%'}.
+ *
+ * <p>Returns {@code null} whenever pushdown must be skipped and the
caller should list all
+ * partitions instead: the equality prefix is empty, a prefix value is
blank, or the escaped
+ * prefix contains a literal {@code '%'} that the contract cannot express.
+ */
+ @Nullable
+ public static String buildPartitionNamePrefixPattern(
+ List<String> partitionKeys, Map<String, String> equalityPrefix) {
+ if (equalityPrefix.isEmpty()) {
+ return null;
+ }
+ LinkedHashMap<String, String> orderedPrefix = new LinkedHashMap<>();
+ for (String partitionKey : partitionKeys) {
+ if (!equalityPrefix.containsKey(partitionKey)) {
+ break;
+ }
+ String value = equalityPrefix.get(partitionKey);
+ if (StringUtils.isNullOrWhitespaceOnly(value)) {
+ return null;
+ }
+ orderedPrefix.put(partitionKey, value);
+ }
+ if (orderedPrefix.isEmpty()) {
+ return null;
+ }
+ String escapedPrefix = generatePartitionPath(orderedPrefix);
Review Comment:
🟠 **[major] logic — 转义/未转义不匹配导致特殊字符分区静默返回空结果**
buildPartitionNamePrefixPattern() 通过 generatePartitionPath(orderedPrefix)（第
201 行）构建前缀模式，该方法对每个值执行 escapePathName()（如 `a:b` → `a%3Ab`）。因此生成的 LIKE
模式是**转义形式**（如 `dt=a%3Ab/%`）。
然而 catalog 端匹配使用的分区名由 PartitionUtils.buildPartitionName(spec)
构建（PartitionUtils.java:90-99），直接拼接 RAW key=value，**不做转义**（如 `dt=a:b`）。REST
端点（RESTCatalogServer.getPagedKey → matchNamePattern/sqlPatternToRegex）也使用未转义形式。
**后果**: 对于前导等值分区谓词中包含 CHAR_TO_ESCAPE 字符（`:`、` `、`=`、`%`、`/`
等）的值，转义后的模式无法匹配未转义的分区名，listPartitionsPaged 返回零分区。由于 managed scan 完全信任 catalog
可见性（无文件系统回退），查询会**静默返回空结果集**。
**建议**: 统一 pattern 和 catalog 分区名的转义约定。方案 (a)：用 RAW
分区名（PartitionUtils.buildPartitionName）构建前缀模式；方案 (b)：若 REST 契约确实要求转义名称，则修改
PartitionUtils.buildPartitionName / 服务端 getPagedKey 进行转义。并添加包含特殊字符分区值（如
`a:b`）的集成测试，验证模式匹配端点的完整往返。
--
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]
