github-actions[bot] commented on code in PR #68998:
URL: https://github.com/apache/doris/pull/68998#discussion_r4256045503
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiConnectorMetadata.java:
##########
@@ -922,6 +928,52 @@ private List<ConnectorPartitionInfo>
restrictToPin(HudiTableHandle handle, List<
return atPin;
}
+ private List<String> hmsPartitionPaths(HudiTableHandle handle,
List<String> names) {
+ List<String> encodedNames = names.stream().filter(name ->
name.indexOf('%') >= 0)
+ .collect(Collectors.toList());
+ if (encodedNames.isEmpty()) {
+ return names;
+ }
+ // Hive sync can register an already escaped Hudi value, which HMS
escapes again in its name.
+ // The storage location identifies the physical path regardless of the
sync extractor's decoding
+ // policy. Decode that path only in parsePartitionValues, preserving
literal percent sequences.
+ // Name and object caches expire independently. A deleted encoded
partition must not fail
+ // a query before pruning; surviving objects must be associated by
values, not list position.
+ List<HmsPartitionInfo> partitions = hmsClient.getExistingPartitions(
+ handle.getDbName(), handle.getTableName(), encodedNames);
+ Map<List<String>, String> namesByValues = new HashMap<>();
+ for (String name : encodedNames) {
+ List<String> values = Arrays.stream(name.split("/"))
+ .map(part ->
HudiScanPlanProvider.unescapePathName(part.substring(part.indexOf('=') + 1)))
+ .collect(Collectors.toList());
+ namesByValues.put(values, name);
+ }
+ Map<String, String> paths = new HashMap<>();
+ for (HmsPartitionInfo partition : partitions) {
+ String name = namesByValues.get(partition.getValues());
+ paths.put(name, relativePartitionPath(handle.getBasePath(),
partition.getLocation()));
+ }
+ return names.stream().filter(name -> name.indexOf('%') < 0 ||
paths.containsKey(name))
+ .map(name -> paths.getOrDefault(name,
name)).collect(Collectors.toList());
+ }
+
+ private static String relativePartitionPath(String basePath, String
location) {
+ if (location == null || location.isEmpty()) {
+ throw new DorisConnectorException("Hudi HMS partition has no
storage location");
+ }
+ // Path quotes literal percent signs when creating its URI; getPath
retains the Hudi path's
+ // own escapes. URI.create(location).getPath() would incorrectly
consume that escaping layer.
+ URI base = new Path(basePath).toUri();
+ URI partition = new Path(location).toUri();
+ String prefix = base.getPath().endsWith("/") ? base.getPath() :
base.getPath() + "/";
+ if (!Objects.equals(base.getScheme(), partition.getScheme())
Review Comment:
[P2] Accept equivalent storage schemes in Hudi partition locations
Normal Hive sync can produce a table at `s3a://b/t` and live partitions at
`s3://b/t/...`: [Hudi 1.0.2 keeps an existing table when only the scheme
changes](https://github.com/apache/hudi/blob/release-1.0.2/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java#L224-L235),
while [new partition locations use the current sync base
path](https://github.com/apache/hudi/blob/release-1.0.2/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HMSDDLExecutor.java#L179-L193).
Doris also maps both schemes to `S3AFileSystem`. With
`use_hive_sync_partition=true`, an encoded partition such as `dt=a%253Ab` whose
location is `s3://b/t/dt=a%3Ab` now throws "outside the table base path"
against `s3a://b/t`. Because location resolution precedes pruning, even `WHERE
dt='ok'` on an existing plain partition fails; the previous path pruned that
HMS name without this check. Compare supported equivalent scheme identities
while retaining the bucket and path containment ch
ecks, and cover mixed-scheme live partitions in both listing and pruning tests.
##########
fe/fe-filesystem/fe-filesystem-spi/src/main/java/org/apache/doris/filesystem/spi/GlobPattern.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.filesystem.spi;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Deque;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/** Numeric-range normalization shared by object storage and Hadoop glob
listing. */
+public final class GlobPattern {
+ private static final int MAX_EXPANSIONS = 10000;
+ private static final Pattern RANGE =
Pattern.compile("(-?\\d+)\\.\\.(-?\\d+)");
+ private static final Pattern SIMPLE_RANGE =
Pattern.compile("\\d+\\.\\.\\d+");
+
+ private GlobPattern() {
+ }
+
+ /**
+ * Expands {@code {N..M}} numeric range syntax in a glob pattern to the
equivalent
+ * comma-separated alternation {@code {N,N+1,...,M}} understood by Hadoop
and object-storage globs.
+ * Supports simple non-negative ranges like {@code {1..3}}, reverse ranges
like {@code {3..1}},
+ * and mixed comma-separated patterns like {@code {1..2,3,1..3}} or {@code
{Refrain,1..3}}.
+ * For simple (no-comma) brace groups, only non-negative ranges are
expanded;
+ * negative-start ranges like {@code {-1..1}} are left unchanged.
+ * Duplicate values are removed. For example:
+ * <ul>
+ * <li>{@code data_{1..3}.csv} → {@code data_{1,2,3}.csv}</li>
+ * <li>{@code data_{1..2,3,1..3}.csv} → {@code data_{1,2,3}.csv}</li>
+ * <li>{@code data_{Refrain,1..3}.csv} → {@code
data_{Refrain,1,2,3}.csv}</li>
+ * <li>{@code data_{-1..1}.csv} → unchanged (no expansion)</li>
+ * </ul>
+ */
+ public static String expandNumericRanges(String pattern) {
+ StringBuilder result = new StringBuilder();
+ Deque<BraceGroup> groups = new ArrayDeque<>();
+ ExpansionBudget budget = new ExpansionBudget();
+ boolean characterClass = false;
+ for (int i = 0; i < pattern.length(); i++) {
+ char c = pattern.charAt(i);
+ StringBuilder output = groups.isEmpty() ? result :
groups.peek().segment;
+ // Escaped braces and commas are literal characters, even inside
an alternative.
+ // Character classes likewise must not change the surrounding
brace structure.
+ if (c == '\\') {
+ output.append(c);
+ if (i + 1 < pattern.length()) {
+ output.append(pattern.charAt(++i));
+ }
+ } else if (characterClass) {
+ output.append(c);
+ if (c == ']') {
+ characterClass = false;
+ }
+ } else if (c == '[') {
+ characterClass = true;
+ output.append(c);
+ } else if (c == '{') {
+ groups.push(new BraceGroup());
+ } else if (c == ',' && !groups.isEmpty()) {
+ groups.peek().finishSegment();
+ } else if (c == '}' && !groups.isEmpty()) {
+ BraceGroup group = groups.pop();
+ group.finishSegment();
+ String expanded = group.expand(budget);
+ (groups.isEmpty() ? result :
groups.peek().segment).append('{').append(expanded).append('}');
+ } else {
+ output.append(c);
+ }
+ }
+ while (!groups.isEmpty()) {
+ BraceGroup group = groups.pop();
+ group.finishSegment();
+ (groups.isEmpty() ? result : groups.peek().segment)
+ .append('{').append(String.join(",", group.segments));
+ }
+ return result.toString();
+ }
+
+ private static final class ExpansionBudget {
+ private long combinations = 1;
+ private long expandedValues;
+ }
+
+ private static final class BraceGroup {
+ private final List<String> segments = new ArrayList<>();
+ private final StringBuilder segment = new StringBuilder();
+
+ private void finishSegment() {
+ segments.add(segment.toString());
+ segment.setLength(0);
+ }
+
+ private String expand(ExpansionBudget budget) {
+ if (segments.size() == 1 &&
!SIMPLE_RANGE.matcher(segments.get(0)).matches()) {
+ return segments.get(0);
+ }
+ Set<String> values = new LinkedHashSet<>();
+ boolean expanded = false;
+ for (String alternative : segments) {
+ Matcher range = RANGE.matcher(alternative.trim());
+ if (range.matches()) {
+ // Count before materializing, and use long arithmetic so
an int endpoint
+ // cannot wrap the loop or turn a huge requested range
into a small count.
+ long from = Integer.parseInt(range.group(1));
+ long to = Integer.parseInt(range.group(2));
+ budget.expandedValues += Math.abs(to - from) + 1;
+ checkLimit(budget.expandedValues);
+ long step = from <= to ? 1 : -1;
+ for (long value = from; step > 0 ? value <= to : value >=
to; value += step) {
+ values.add(Long.toString(value));
+ }
+ expanded = true;
+ } else {
+ values.add(alternative.trim());
+ }
+ }
+ if (expanded) {
+ budget.combinations *= values.size();
Review Comment:
[P2] Count mutually exclusive ranges as alternatives
For `hdfs://namenode:8020/data_{a{1..101},b{1..101}}.csv`, the two inner
ranges form 202 alternatives, but this shared product becomes `101 * 101 =
10201` and throws `IllegalArgumentException` even though only 202 numeric
values are generated. Both `DFSFileSystem.listFiles` and `globListWithLimit`
call this normalizer before `globStatus`, so discovery fails for a small valid
nested glob. [Hadoop 3.4.2 supports nested brace
alternatives](https://github.com/apache/hadoop/blob/rel/release-3.4.2/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/GlobPattern.java#L102-L114).
Track cardinality per brace expression, summing alternative branches and
multiplying only independent concatenated groups, while retaining the
allocation limit. Add a nested-alternative case alongside the existing
Cartesian-product limit test.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
