github-actions[bot] commented on code in PR #69383:
URL: https://github.com/apache/doris/pull/69383#discussion_r4334770927
##########
fe/fe-filesystem/fe-filesystem-azure/src/main/java/org/apache/doris/filesystem/azure/AzureFileSystem.java:
##########
@@ -397,51 +398,62 @@ public GlobListing globListWithLimit(Location path,
String startAfter, long maxB
// base = uri with the key portion stripped, preserving the original
scheme/host syntax.
String base = uri.substring(0, uri.length() - keyPattern.length());
- Pattern matcher = Pattern.compile(globToRegex(keyPattern));
- String listKeyPrefix = longestNonGlobPrefix(keyPattern);
- String listPrefixUri = base + listKeyPrefix;
+ String expandedKeyPattern =
ObjectStorageGlob.expandNumericRanges(keyPattern);
+ Pattern matcher = Pattern.compile(globToRegex(expandedKeyPattern));
+ String listKeyPrefix = longestNonGlobPrefix(expandedKeyPattern);
+ List<String> listKeyPrefixes =
ObjectStorageGlob.expandedGlobListPrefixes(expandedKeyPattern);
Review Comment:
[P2] Fall back when class-range endpoints contain escapes
For `data/[\a-z]/*/file.csv`, the new prefix expander consumes `\a` as a
complete alternative and emits only `data/-/`, `data/a/`, and `data/z/`. The
full matcher emits `[\Qa\E-z]`, an ordinary `a-z` range, so it accepts
`data/m/sub/file.csv`, but none of the remote LIST calls can return that blob.
Escaped upper endpoints such as `[a-\c]` have the same mismatch. Azure
previously listed the conservative `data/` prefix and found these matches;
importing the existing S3 expander introduces silent omissions here. Parse
escaped range endpoints consistently with the matcher, or retain a conservative
prefix for these classes.
##########
fe/fe-filesystem/fe-filesystem-azure/src/main/java/org/apache/doris/filesystem/azure/AzureFileSystem.java:
##########
@@ -397,51 +398,62 @@ public GlobListing globListWithLimit(Location path,
String startAfter, long maxB
// base = uri with the key portion stripped, preserving the original
scheme/host syntax.
String base = uri.substring(0, uri.length() - keyPattern.length());
- Pattern matcher = Pattern.compile(globToRegex(keyPattern));
- String listKeyPrefix = longestNonGlobPrefix(keyPattern);
- String listPrefixUri = base + listKeyPrefix;
+ String expandedKeyPattern =
ObjectStorageGlob.expandNumericRanges(keyPattern);
+ Pattern matcher = Pattern.compile(globToRegex(expandedKeyPattern));
+ String listKeyPrefix = longestNonGlobPrefix(expandedKeyPattern);
+ List<String> listKeyPrefixes =
ObjectStorageGlob.expandedGlobListPrefixes(expandedKeyPattern);
List<FileEntry> files = new ArrayList<>();
long totalSize = 0L;
- String maxFile = "";
- String continuationToken = null;
+ String nextMatchAfterLimit = "";
+ String lastMatchedKey = "";
boolean reachLimit = false;
- outer:
- do {
- RemoteObjects page = objStorage.listObjects(listPrefixUri,
continuationToken);
- for (RemoteObject obj : page.getObjectList()) {
- String key = obj.getKey();
- if (key.endsWith(DIR_MARKER_SUFFIX)) {
- continue;
- }
- if (startAfter != null && !startAfter.isEmpty() &&
key.compareTo(startAfter) <= 0) {
- continue;
- }
- if (!matcher.matcher(key).matches()) {
- continue;
- }
- if ((maxFiles > 0 && files.size() >= maxFiles)
- || (maxBytes > 0 && totalSize >= maxBytes)) {
- maxFile = key;
- reachLimit = true;
- break outer;
+ for (String currentListKeyPrefix : listKeyPrefixes) {
+ String continuationToken = null;
+ String listPrefixUri = base + currentListKeyPrefix;
Review Comment:
[P2] Encode expanded prefixes before rebuilding the Azure URI
A valid glob such as `data/[$-&]/*/file.csv` expands to prefixes containing
`$`, `%`, and `&`. This concatenation sends the generated `data/%/` through
`AzureObjStorage.listObjects`, which reparses it with `AzureUri.parse` and
`URLDecoder.decode`; the bare percent escape throws `IllegalArgumentException`,
aborting the listing even if the container only has ordinary matching `$`/`&`
keys. The previous conservative `data/` scan succeeded. Generated `+` and `#`
prefixes are likewise changed into spaces or truncated. Encode the raw prefix
while preserving path separators, or pass the raw prefix directly to the
listing API. The storage mocks in the new tests bypass this second parse.
##########
fe/fe-filesystem/fe-filesystem-azure/src/main/java/org/apache/doris/filesystem/azure/AzureFileSystem.java:
##########
@@ -397,51 +398,62 @@ public GlobListing globListWithLimit(Location path,
String startAfter, long maxB
// base = uri with the key portion stripped, preserving the original
scheme/host syntax.
String base = uri.substring(0, uri.length() - keyPattern.length());
- Pattern matcher = Pattern.compile(globToRegex(keyPattern));
- String listKeyPrefix = longestNonGlobPrefix(keyPattern);
- String listPrefixUri = base + listKeyPrefix;
+ String expandedKeyPattern =
ObjectStorageGlob.expandNumericRanges(keyPattern);
+ Pattern matcher = Pattern.compile(globToRegex(expandedKeyPattern));
+ String listKeyPrefix = longestNonGlobPrefix(expandedKeyPattern);
+ List<String> listKeyPrefixes =
ObjectStorageGlob.expandedGlobListPrefixes(expandedKeyPattern);
List<FileEntry> files = new ArrayList<>();
long totalSize = 0L;
- String maxFile = "";
- String continuationToken = null;
+ String nextMatchAfterLimit = "";
+ String lastMatchedKey = "";
boolean reachLimit = false;
- outer:
- do {
- RemoteObjects page = objStorage.listObjects(listPrefixUri,
continuationToken);
- for (RemoteObject obj : page.getObjectList()) {
- String key = obj.getKey();
- if (key.endsWith(DIR_MARKER_SUFFIX)) {
- continue;
- }
- if (startAfter != null && !startAfter.isEmpty() &&
key.compareTo(startAfter) <= 0) {
- continue;
- }
- if (!matcher.matcher(key).matches()) {
- continue;
- }
- if ((maxFiles > 0 && files.size() >= maxFiles)
- || (maxBytes > 0 && totalSize >= maxBytes)) {
- maxFile = key;
- reachLimit = true;
- break outer;
+ for (String currentListKeyPrefix : listKeyPrefixes) {
+ String continuationToken = null;
+ String listPrefixUri = base + currentListKeyPrefix;
+ do {
+ RemoteObjects page = objStorage.listObjects(listPrefixUri,
continuationToken);
+ for (RemoteObject obj : page.getObjectList()) {
+ String key = obj.getKey();
+ if (key.endsWith(DIR_MARKER_SUFFIX)) {
+ continue;
+ }
+ if (startAfter != null && !startAfter.isEmpty()
+ && ObjectStorageGlob.compareUtf8Binary(key,
startAfter) <= 0) {
Review Comment:
[P2] Apply resume cursors in the Azure endpoint listing order
The configurable Azure endpoint can use Microsoft's Azurite, whose [flat
listing
implementation](https://github.com/Azure/Azurite/blob/main/src/blob/persistence/LokiBlobMetadataStore.ts)
sorts names with JavaScript string comparisons (UTF-16 ordinal). For
`data/*.csv`, that order puts `data/😀.csv` before `data/Ａ.csv` (U+FF21). With
`maxFiles=1`, the first call returns the emoji key; resuming from that returned
key now discards the unconsumed fullwidth-A key because it sorts before the
emoji in UTF-8. The former `String.compareTo` retained it. Keep prefix
ordering, remote enumeration and cursor filtering consistent with the endpoint
order, and cover a limited no-brace listing in ordinal order. This
counterexample is established from the official emulator source; live Azure
service ordering was not exercised.
##########
fe/fe-filesystem/fe-filesystem-spi/src/main/java/org/apache/doris/filesystem/spi/ObjectStorageGlob.java:
##########
@@ -0,0 +1,725 @@
+// 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.math.BigInteger;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Provider-neutral glob helpers for object-storage keys.
+ */
+public final class ObjectStorageGlob {
+
+ private static final int MAX_EXPANDED_GLOB_LIST_PREFIXES = 256;
+ private static final Comparator<String> UTF8_BINARY_ORDER =
ObjectStorageGlob::compareUtf8Binary;
+ private static final Pattern NUMERIC_RANGE_PATTERN =
Pattern.compile("-?\\d+\\.\\.-?\\d+");
+ private static final Pattern NUMERIC_RANGE_ALTERNATIVE =
Pattern.compile("(-?\\d+)\\.\\.(-?\\d+)");
+
+ private ObjectStorageGlob() {
+ }
+
+ /**
+ * Returns the longest key prefix that contains no glob metacharacters.
+ */
+ public static String longestNonGlobPrefix(String globPattern) {
+ int earliest = globPattern.length();
+ for (char c : new char[]{'*', '?', '[', '{', '\\'}) {
+ int idx = globPattern.indexOf(c);
+ if (idx >= 0 && idx < earliest) {
+ earliest = idx;
+ }
+ }
+ return globPattern.substring(0, earliest);
+ }
+
+ /**
+ * Returns object-store list prefixes that are safe to push down for a
glob pattern.
+ *
+ * <p>Unlike {@link #longestNonGlobPrefix(String)}, this expands bounded
glob constructs
+ * ({@code {...}} alternation and positive {@code [...]} character
classes) before the first
+ * unbounded wildcard. For example,
+ * {@code date=2025-{0[3-9],1[0-2]}-01/mp_id=8/*} becomes concrete date
prefixes instead
+ * of one broad {@code date=2025-} scan. If expansion would be too large
or unsafe, it
+ * falls back to the conservative longest static prefix.
+ */
+ public static List<String> expandedGlobListPrefixes(String globPattern) {
+ List<String> prefixes = expandGlobListPrefixes(globPattern, true);
+ return prefixes == null ? List.of(longestNonGlobPrefix(globPattern)) :
prefixes;
+ }
+
+ private static List<String> expandGlobListPrefixes(String globPattern,
boolean allowPartialPrefix) {
+ List<String> prefixes = new ArrayList<>();
+ prefixes.add("");
+ int i = 0;
+ while (i < globPattern.length()) {
+ char c = globPattern.charAt(i);
+ if (c == '*' || c == '?') {
+ return allowPartialPrefix ? compactPrefixes(prefixes) : null;
+ }
+ if (c == '\\') {
+ if (i + 1 < globPattern.length()) {
+ appendLiteral(prefixes, globPattern.charAt(i + 1));
+ i += 2;
+ } else {
+ appendLiteral(prefixes, c);
+ i++;
+ }
+ continue;
+ }
+ if (c == '[') {
+ PrefixExpansion charClass = expandCharacterClass(globPattern,
i);
+ if (charClass == null) {
+ return allowPartialPrefix ? compactPrefixes(prefixes) :
null;
+ }
+ prefixes = appendAlternatives(prefixes, charClass.values);
+ if (prefixes == null) {
+ return null;
+ }
+ i = charClass.nextIndex;
+ continue;
+ }
+ if (c == '{') {
+ PrefixExpansion brace = expandBraceGroup(globPattern, i);
+ if (brace == null) {
+ return allowPartialPrefix ? compactPrefixes(prefixes) :
null;
+ }
+ prefixes = appendAlternatives(prefixes, brace.values);
+ if (prefixes == null) {
+ return null;
+ }
+ i = brace.nextIndex;
+ continue;
+ }
+ appendLiteral(prefixes, c);
+ i++;
+ }
+ // Complete brace arms must retain distinct values until the
surrounding suffix is appended.
+ return allowPartialPrefix ? compactPrefixes(prefixes) : prefixes;
+ }
+
+ private static void appendLiteral(List<String> prefixes, char c) {
+ for (int i = 0; i < prefixes.size(); i++) {
+ prefixes.set(i, prefixes.get(i) + c);
+ }
+ }
+
+ private static List<String> appendAlternatives(List<String> prefixes,
List<String> alternatives) {
+ long expandedSize = (long) prefixes.size() * alternatives.size();
+ if (expandedSize > MAX_EXPANDED_GLOB_LIST_PREFIXES) {
+ return null;
+ }
+ List<String> expanded = new ArrayList<>((int) expandedSize);
+ for (String prefix : prefixes) {
+ for (String alternative : alternatives) {
+ expanded.add(prefix + alternative);
+ }
+ }
+ return expanded;
+ }
+
+ private static PrefixExpansion expandCharacterClass(String globPattern,
int openIndex) {
+ int closeIndex = findClosingBracket(globPattern, openIndex);
+ if (closeIndex < 0 || closeIndex == openIndex + 1) {
+ return null;
+ }
+ int i = openIndex + 1;
+ char first = globPattern.charAt(i);
+ if (first == '!' || first == '^') {
+ return null;
+ }
+ if (containsSurrogate(globPattern, i, closeIndex)) {
+ return null;
+ }
+ List<String> values = new ArrayList<>();
+ while (i < closeIndex) {
+ char current = globPattern.charAt(i);
+ if (current == '\\') {
+ if (i + 1 >= closeIndex) {
+ return null;
+ }
+ values.add(String.valueOf(globPattern.charAt(i + 1)));
+ i += 2;
+ continue;
+ }
+ if (i + 2 < closeIndex && globPattern.charAt(i + 1) == '-') {
+ char rangeEnd = globPattern.charAt(i + 2);
+ int step = current <= rangeEnd ? 1 : -1;
+ for (char ch = current; step > 0 ? ch <= rangeEnd : ch >=
rangeEnd; ch += step) {
+ values.add(String.valueOf(ch));
+ if (values.size() > MAX_EXPANDED_GLOB_LIST_PREFIXES) {
+ return null;
+ }
+ }
+ i += 3;
+ continue;
+ }
+ values.add(String.valueOf(current));
+ i++;
+ }
+ return new PrefixExpansion(values, closeIndex + 1);
+ }
+
+ private static int findClosingBracket(String globPattern, int openIndex) {
+ for (int i = openIndex + 1; i < globPattern.length(); i++) {
+ char c = globPattern.charAt(i);
+ if (c == '\\') {
+ i++;
+ continue;
+ }
+ if (c == ']') {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ private static boolean containsSurrogate(String text, int start, int end) {
+ for (int i = start; i < end; i++) {
+ if (Character.isSurrogate(text.charAt(i))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static PrefixExpansion expandBraceGroup(String globPattern, int
openIndex) {
+ int closeIndex = findClosingBrace(globPattern, openIndex);
+ if (closeIndex < 0) {
+ return null;
+ }
+ List<String> alternatives = splitBraceAlternatives(
+ globPattern.substring(openIndex + 1, closeIndex));
+ if (alternatives.isEmpty()) {
+ return null;
+ }
+ List<String> values = new ArrayList<>();
+ for (String alternative : alternatives) {
+ if (containsNumericRange(alternative)) {
+ return null;
+ }
+ List<String> expandedAlternative =
expandGlobListPrefixes(alternative, false);
+ if (expandedAlternative == null) {
+ return null;
+ }
+ values.addAll(expandedAlternative);
+ if (values.size() > MAX_EXPANDED_GLOB_LIST_PREFIXES) {
+ return null;
+ }
+ }
+ return new PrefixExpansion(values, closeIndex + 1);
+ }
+
+ private static int findClosingBrace(String globPattern, int openIndex) {
+ int depth = 0;
+ for (int i = openIndex; i < globPattern.length(); i++) {
+ char c = globPattern.charAt(i);
+ if (c == '\\') {
+ i++;
+ continue;
+ }
+ if (c == '[') {
+ int closeBracket = findClosingBracket(globPattern, i);
+ if (closeBracket >= 0) {
+ i = closeBracket;
+ continue;
+ }
+ }
+ if (c == '{') {
+ depth++;
+ } else if (c == '}') {
+ depth--;
+ if (depth == 0) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private static List<String> splitBraceAlternatives(String content) {
+ List<String> alternatives = new ArrayList<>();
+ int depth = 0;
+ int start = 0;
+ for (int i = 0; i < content.length(); i++) {
+ char c = content.charAt(i);
+ if (c == '\\') {
+ i++;
+ continue;
+ }
+ if (c == '[') {
+ int closeBracket = findClosingBracket(content, i);
+ if (closeBracket >= 0) {
+ i = closeBracket;
+ continue;
+ }
+ }
+ if (c == '{') {
+ depth++;
+ } else if (c == '}') {
+ if (depth == 0) {
+ return Collections.emptyList();
+ }
+ depth--;
+ } else if (c == ',' && depth == 0) {
+ alternatives.add(content.substring(start, i));
+ start = i + 1;
+ }
+ }
+ if (depth != 0) {
+ return Collections.emptyList();
+ }
+ alternatives.add(content.substring(start));
+ return alternatives;
+ }
+
+ private static List<String> compactPrefixes(List<String> prefixes) {
+ List<String> sorted = new ArrayList<>(prefixes);
+ sorted.sort(UTF8_BINARY_ORDER);
+ List<String> compact = new ArrayList<>();
+ for (String prefix : sorted) {
+ if (prefix.isEmpty()) {
+ return List.of("");
+ }
+ boolean covered = false;
+ for (String existing : compact) {
+ if (prefix.startsWith(existing)) {
+ covered = true;
+ break;
+ }
+ }
+ if (!covered) {
+ compact.add(prefix);
+ }
+ }
+ return compact;
+ }
+
+ /**
+ * Compares object keys by unsigned UTF-8 byte order, matching
object-store listing order.
+ */
+ public static int compareUtf8Binary(String left, String right) {
+ byte[] leftBytes = left.getBytes(StandardCharsets.UTF_8);
+ byte[] rightBytes = right.getBytes(StandardCharsets.UTF_8);
+ int commonLength = Math.min(leftBytes.length, rightBytes.length);
+ for (int i = 0; i < commonLength; i++) {
+ int result = Integer.compare(
+ Byte.toUnsignedInt(leftBytes[i]),
Byte.toUnsignedInt(rightBytes[i]));
+ if (result != 0) {
+ return result;
+ }
+ }
+ return Integer.compare(leftBytes.length, rightBytes.length);
+ }
+
+ /**
+ * Expands bounded {N..M} numeric ranges inside brace groups into
comma-separated alternatives.
+ */
+ public static String expandNumericRanges(String pattern) {
+ StringBuilder expanded = new StringBuilder();
+ for (int i = 0; i < pattern.length(); i++) {
+ char c = pattern.charAt(i);
+ if (c == '\\') {
+ expanded.append(c);
+ if (i + 1 < pattern.length()) {
+ expanded.append(pattern.charAt(++i));
+ }
+ } else if (c == '[') {
+ int closeIndex = findClosingBracket(pattern, i);
+ if (closeIndex >= 0) {
+ expanded.append(pattern, i, closeIndex + 1);
+ i = closeIndex;
+ } else {
+ expanded.append(c);
+ }
+ } else if (c == '{') {
+ int closeIndex = findClosingBrace(pattern, i);
+ if (closeIndex >= 0) {
+ expanded.append('{').append(expandNumericBraceGroup(
+ pattern.substring(i + 1, closeIndex))).append('}');
+ i = closeIndex;
+ } else {
+ expanded.append(c);
+ }
+ } else {
+ expanded.append(c);
+ }
+ }
+ return expanded.toString();
+ }
+
+ private static String expandNumericBraceGroup(String content) {
+ List<String> segments = splitBraceAlternatives(content);
+ boolean isMixed = segments.size() > 1;
+ LinkedHashSet<String> values = new LinkedHashSet<>();
+ for (String segment : segments) {
+ Matcher range = numericRangeMatcher(segment, isMixed);
+ if (range == null) {
+ values.add(expandNumericRanges(segment));
+ } else {
+ long from;
+ long to;
+ try {
+ from = Long.parseLong(range.group(1));
+ to = Long.parseLong(range.group(2));
+ } catch (NumberFormatException e) {
+ return content;
+ }
+ long cardinality = numericRangeCardinality(from, to);
+ if (cardinality < 0 || cardinality >
MAX_EXPANDED_GLOB_LIST_PREFIXES - values.size()) {
+ return content;
+ }
+ long step = from <= to ? 1L : -1L;
+ boolean zeroPad = hasLeadingZero(range.group(1)) ||
hasLeadingZero(range.group(2));
+ int width = Math.max(digitWidth(range.group(1)),
digitWidth(range.group(2)));
+ for (long offset = 0; offset < cardinality; offset++) {
+ long value = step > 0 ? from + offset : from - offset;
+ values.add(formatRangeValue(value, width, zeroPad));
+ }
+ }
+ if (values.size() > MAX_EXPANDED_GLOB_LIST_PREFIXES) {
+ return content;
+ }
+ }
+ return String.join(",", values);
+ }
+
+ private static Matcher numericRangeMatcher(String alternative, boolean
isMixed) {
+ Matcher matcher = NUMERIC_RANGE_ALTERNATIVE.matcher(isMixed ?
alternative.trim() : alternative);
+ if (!matcher.matches() || (!isMixed
+ && (matcher.group(1).startsWith("-") ||
matcher.group(2).startsWith("-")))) {
+ return null;
+ }
+ return matcher;
+ }
+
+ private static boolean containsNumericRange(String text) {
+ return NUMERIC_RANGE_PATTERN.matcher(text).find();
+ }
+
+ private static long numericRangeCardinality(long from, long to) {
+ try {
+ long distance = from <= to
+ ? Math.subtractExact(to, from)
+ : Math.subtractExact(from, to);
+ return Math.addExact(distance, 1L);
+ } catch (ArithmeticException e) {
+ return -1L;
+ }
+ }
+
+ private static boolean hasLeadingZero(String value) {
+ String digits = value.startsWith("-") ? value.substring(1) : value;
+ return digits.length() > 1 && digits.charAt(0) == '0';
+ }
+
+ private static int digitWidth(String value) {
+ return value.startsWith("-") ? value.length() - 1 : value.length();
+ }
+
+ private static String formatRangeValue(long value, int width, boolean
zeroPad) {
+ if (!zeroPad) {
+ return Long.toString(value);
+ }
+ if (value == Long.MIN_VALUE) {
Review Comment:
[P2] Preserve zero padding for the minimum long value
The early return skips the padding loop even when the requested width
exceeds 19 digits. For the valid mixed range
`data_{-09223372036854775808..-09223372036854775807,x}.csv`, both endpoints
parse as longs, the cardinality is 2 and the digit width is 20, but
normalization emits `-9223372036854775808` for the first arm and keeps the
leading zero for the second. Consequently Azure/S3 listing and
`compileGlobPattern` omit the correctly padded minimum key and accept its
unpadded counterpart. The BigInteger regex fallback preserves this width, so
behavior also differs between normalization paths. Derive magnitude digits
without negating `Long.MIN_VALUE`, then apply the common padding logic to this
value as well.
--
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]
