sundapeng commented on code in PR #8728:
URL: https://github.com/apache/paimon/pull/8728#discussion_r3610936843
##########
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`）的集成测试，验证模式匹配端点的完整往返。
##########
paimon-core/src/main/java/org/apache/paimon/utils/PartitionPathUtils.java:
##########
@@ -330,8 +455,9 @@ public static List<Pair<LinkedHashMap<String, String>,
Path>> searchPartSpecAndP
part.getPath(), partitionKeys),
part.getPath()));
} else {
- LinkedHashMap<String, String> spec =
extractPartitionSpecFromPath(part.getPath());
- if (spec.size() != partitionKeys.size()) {
+ LinkedHashMap<String, String> spec =
+ extractPartitionSpecFromPath(part.getPath(),
partitionKeys);
Review Comment:
🟡 **[minor] migration — 严格 key 匹配可能静默隐藏已有分区**
searchPartSpecAndPaths 现在使用带 partitionKeys 参数的
extractPartitionSpecFromPath，要求每个路径组件的 key 与声明的分区键精确匹配。旧代码使用无参版本，接受任意 k=v
组件仅检查数量。
受影响的场景：
1. 通过 ALTER TABLE 重命名分区键后，旧目录保留旧 key 名
2. 外部 format table 指向 Hive 风格目录，key 大小写不同（如 `DT=20240101` vs 声明的 `dt`）
3. 外部工具创建的目录使用缩写 key 名
这些分区以前可见（虽然 spec 值可能不正确），现在变为不可见，导致查询结果中**静默数据丢失**。
**建议**: 当目录因 key 不匹配被跳过时记录 WARN 日志（类似 managed table 的 corrupt-partition
警告），以便运维人员发现数据不可见问题。考虑大小写不敏感的 key 匹配以对齐 Hive 语义。在文档中说明此变更后分区目录必须精确匹配声明的 key 名。
--
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]
