github-actions[bot] commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r4280182659
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -728,11 +706,183 @@ public boolean
supportInternalPartitionPruned(Optional<MvccSnapshot> snapshot) {
@Override
public Map<String, PartitionItem>
getNameToPartitionItems(Optional<MvccSnapshot> snapshot) {
+ if (supportsConnectorPartitionPruning()) {
+ PluginDrivenMvccSnapshot pin = getOrMaterialize(snapshot);
+ if (pin.isPartitionViewMaterialized()) {
+ return pin.getNameToPartitionItem();
+ }
+ if (pin.isPartitionViewUnavailable()) {
+ return Collections.emptyMap();
+ }
+ Optional<ExternalTablePreloadInfo> sharedView =
sharedLatestPreloadInfo(snapshot);
+ if (sharedView.isPresent()) {
+ if (sharedView.get().hasScanPartitionView()
+ &&
sharedView.get().getScanPartitionView().isPresent()) {
+ return sharedView.get().getScanPartitionView().get();
+ }
+ // A FILTERED latest scan deliberately leaves its scan view
unwarmed so connector predicate
+ // pruning stays effective, but this per-name reader is also
the async-MV compensation path
+ // (PartitionCompensator resolves each compensated base
partition by name while planner locks
+ // are held). The pre-lock pass materializes the complete
mapping view separately; reuse it
+ // rather than re-listing every partition under those locks.
+ if (sharedView.get().hasMtmvPartitionView()
+ &&
sharedView.get().getMtmvPartitionView().isPresent()) {
+ return sharedView.get().getMtmvPartitionView().get();
+ }
+ }
+ // The latest Hive query pin intentionally carries no partition
map so selective scans can send a
+ // predicate to HMS first. Consumers that explicitly ask for a
partition map (MTMV alignment,
+ // no-filter scan finalization, and a connector-declined pruning
fallback) require the real full
+ // view instead of treating that query-only pin as an empty table.
An unavailable shared scan view
+ // is not an authoritative empty map either: checked MTMV
consumers reject it in
+ // getAndCopyPartitionItems, while other explicit map consumers
retain this fail-loud full-view path.
+ return super.getNameToPartitionItems(snapshot);
+ }
return getOrMaterialize(snapshot).getNameToPartitionItem();
}
@Override
- public Map<String, PartitionItem>
getAndCopyPartitionItems(Optional<MvccSnapshot> snapshot) {
+ public Optional<Map<String, PartitionItem>>
getNameToPartitionItemsForScan(Optional<MvccSnapshot> snapshot) {
+ if (snapshot.isPresent() && snapshot.get() instanceof
PluginDrivenMvccSnapshot
+ && ((PluginDrivenMvccSnapshot)
snapshot.get()).isPartitionViewUnavailable()) {
+ return Optional.empty();
+ }
+ if (supportsConnectorPartitionPruning()) {
+ PluginDrivenMvccSnapshot pin = getOrMaterialize(snapshot);
+ if (pin.isPartitionViewMaterialized()) {
+ // This statement's pin already carries a materialized view:
reuse it instead of paying another
+ // connector round-trip that could observe a different remote
generation.
+ return Optional.of(pin.getNameToPartitionItem());
Review Comment:
[P1] Preserve scan-all for invalid materialized partition pins. With a
nontransactional Hive base containing at least `num_partitions_in_batch_mode`
valid DATE partitions plus `dt=not-a-date`, a `SELF_MANAGE` MTMV such as
`SELECT id FROM hive_table` gets an invalid MATERIALIZED pin from the new
warmup. This branch exposes only its valid items. `initSelectedPartitions`
still returns DEFERRED, finalization wraps that partial map as the full
selection, and the batch gate accepts it even with `isPruned=false`;
`startSplit` then schedules only those keys. The refresh can therefore succeed
while omitting the malformed partition's `id` rows. Make an invalid
materialized pin yield an unavailable scan view so finalization preserves
scan-all, and cover the refresh-to-batch path. This occurs after successful
warmup degradation, unlike the earlier warmup-exception finding.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -2495,53 +2602,406 @@ static String hiveDateTimeString(LocalDateTime ldt) {
* Prunes partition names based on extracted equality predicates.
* Partition names follow the Hive convention: key1=val1/key2=val2
*/
- private List<String> prunePartitionNames(List<String> allPartNames,
- List<String> partKeyNames, Map<String, List<String>> predicates) {
+ private List<String> prunePartitionNames(List<String> allPartNames,
List<String> partKeyNames,
+ Map<String, String> partKeyTypes, Map<String, List<String>>
predicates) {
List<String> matched = new ArrayList<>();
for (String partName : allPartNames) {
Map<String, String> partValues = parsePartitionName(partName,
partKeyNames);
- if (matchesPredicates(partValues, predicates)) {
+ // A name this prefilter cannot decode is KEPT, never dropped: its
result becomes the logical
+ // selected view, so dropping a name only because the raw text was
not interpretable loses rows the
+ // query must read. The typed PartitionPruner re-prunes the
survivors, so a superset only costs the
+ // lost optimization.
+ if (partValues == null || matchesPredicates(partValues,
partKeyTypes, predicates)) {
matched.add(partName);
}
}
return matched;
}
+ private static String buildHmsPartitionFilter(List<String> partKeyNames,
Map<String, String> partKeyTypes,
+ Map<String, String> partKeyHiveTypes, Map<String, List<String>>
partitionPredicates) {
+ List<String> filters = new ArrayList<>();
+ for (String partKeyName : partKeyNames) {
+ List<String> values = partitionPredicates.get(partKeyName);
+ if (values == null || values.isEmpty()) {
+ continue;
+ }
+ if (!isHmsFilterIdentifier(partKeyName)) {
+ return null;
+ }
+ // HMS stores a partition key's directory name LOWERCASED (`P` ->
`p=x`). The JDO filter path
+ // (ExpressionTree) accepts the key operand case-insensitively but
then compares the emitted
+ // `P = 'x'` fragment against that stored lowercase name with a
plain string compare, so a
+ // mixed-case key yields a SUCCESSFUL, EMPTY partition set. Doris
adopts that response as the
+ // logical AND physical selected view and silently loses every
row, so only a key already in its
+ // stored (lowercase) form may be named directly; anything else
falls back to the positional local
+ // prefilter. (Not the reserved-word/all-digit decline above:
those names are unnameable grammar.)
+ if (!partKeyName.equals(partKeyName.toLowerCase(Locale.ROOT))) {
+ return null;
+ }
+ if (isHmsStringType(partKeyTypes.get(partKeyName))
+ && !isHmsStringType(partKeyHiveTypes.get(partKeyName))) {
+ return null;
+ }
+ List<String> valueFilters = new ArrayList<>();
+ for (String value : values) {
+ String literal = toHmsFilterLiteral(value,
partKeyTypes.get(partKeyName));
+ if (literal == null) {
+ return null;
+ }
+ valueFilters.add(partKeyName + " = " + literal);
+ }
+ filters.add(valueFilters.size() == 1 ? valueFilters.get(0)
+ : "(" + String.join(" OR ", valueFilters) + ")");
+ }
+ return filters.isEmpty() ? null : "(" + String.join(" AND ", filters)
+ ")";
+ }
+
+ private static boolean isHmsFilterIdentifier(String value) {
+ if (value.isEmpty() || !isHmsFilterLetterOrDigit(value.charAt(0))) {
+ return false;
+ }
+ // The metastore filter grammar (Filter.g) reserves a set of
case-insensitive keyword tokens, and lexes
+ // an all-digit token as an IntegralLiteral (declared before
Identifier); a key operand must be an
+ // Identifier, so such a partition key can never be named in a filter.
Decline the direct path for it
+ // rather than sending a predictably invalid RPC that fails, taints
the pooled client and repeats the
+ // whole enumeration.
+ String lowerCaseValue = value.toLowerCase(Locale.ROOT);
+ if (lowerCaseValue.equals("not") || lowerCaseValue.equals("and") ||
lowerCaseValue.equals("or")
+ || lowerCaseValue.equals("like") ||
lowerCaseValue.equals("date")
+ || lowerCaseValue.equals("const") ||
lowerCaseValue.equals("struct")
+ || isAllDigits(value)) {
+ return false;
+ }
+ for (int index = 1; index < value.length(); index++) {
+ char character = value.charAt(index);
+ if (!isHmsFilterLetterOrDigit(character) && character != '_') {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isAllDigits(String value) {
+ for (int index = 0; index < value.length(); index++) {
+ char character = value.charAt(index);
+ if (character < '0' || character > '9') {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isHmsFilterLetterOrDigit(char value) {
+ return value >= 'a' && value <= 'z'
+ || value >= 'A' && value <= 'Z'
+ || value >= '0' && value <= '9';
+ }
+
+ private static String toHmsFilterLiteral(String value, String typeName) {
+ if (isHmsIntegralType(typeName)) {
+ // A successful HMS response is not necessarily complete for
integral JDO pushdown: Hive compares
+ // the rendered partition-name fragment as text, so `p = 1` can
omit the valid partition `p=01`.
+ // The client cannot know whether direct SQL or JDO served the
request, therefore integral predicates
+ // must use the full partition-name listing and the typed local
comparison below.
+ return null;
+ }
+ if (typeName != null && !isHmsStringType(typeName)) {
+ return null;
+ }
+ if (value.indexOf('\\') >= 0 || value.indexOf('\'') >= 0) {
+ return null;
+ }
+ return "'" + value + "'";
+ }
+
+ private static boolean isHmsDecimalType(String typeName) {
+ return typeName != null &&
typeName.toUpperCase(Locale.ROOT).startsWith("DECIMAL");
+ }
+
+ private static boolean isHmsIntegralType(String typeName) {
+ if (typeName == null) {
+ return false;
+ }
+ switch (typeName.toUpperCase(Locale.ROOT)) {
+ case "TINYINT":
+ case "SMALLINT":
+ case "INT":
+ case "INTEGER":
+ case "BIGINT":
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ private static boolean isHmsStringType(String typeName) {
+ return typeName != null &&
"STRING".equals(typeName.toUpperCase(Locale.ROOT));
+ }
+
+ private static boolean isHmsFloatType(String typeName) {
+ return typeName != null &&
"FLOAT".equals(typeName.toUpperCase(Locale.ROOT));
+ }
+
+ private static boolean isHmsDoubleType(String typeName) {
+ return typeName != null &&
"DOUBLE".equals(typeName.toUpperCase(Locale.ROOT));
+ }
+
+ private static boolean isHmsBooleanType(String typeName) {
+ return typeName != null &&
"BOOLEAN".equals(typeName.toUpperCase(Locale.ROOT));
+ }
+
+ /** {@code DATEV2} is the Doris type a hive {@code DATE} partition key
maps to; legacy {@code DATE} is kept
+ * for a handle built by an older module. */
+ private static boolean isHmsDateType(String typeName) {
+ return isHmsTypeName(typeName, "DATE", "DATEV2");
+ }
+
+ /** The Doris types a hive {@code TIMESTAMP} partition key maps to (see
{@code HmsTypeMapping}). */
+ private static boolean isHmsTimestampType(String typeName) {
+ return isHmsTypeName(typeName, "DATETIME", "DATETIMEV2",
"TIMESTAMPTZ");
+ }
+
+ private static boolean isHmsTypeName(String typeName, String... names) {
+ if (typeName == null) {
+ return false;
+ }
+ String upper = typeName.toUpperCase(Locale.ROOT);
+ for (String name : names) {
+ if (name.equals(upper)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Decodes one rendered Hive partition name ({@code key1=val1/key2=val2})
into the DECLARED partition
+ * keys, or {@code null} when the name cannot be bound to them.
+ *
+ * <p>Values are bound POSITIONALLY to {@code partKeyNames}: Hive's {@code
FileUtils.makePartName} renders
+ * the segments in the table's partition-key order, but it LOWERCASES the
rendered key, so a declared key
+ * such as {@code P-X} comes back as {@code p-x}. Matching the rendered
spelling against the declared one
+ * would find no value for that key, reject every real partition, and —
because this prefilter's result
+ * becomes the logical selected view — make the query return no rows. The
key spelling is therefore not
+ * trusted at all; only the value order is.</p>
+ *
+ * <p>{@code null} means "not interpretable" (a segment count that does
not match the declared key count),
+ * NOT "matches nothing": the caller must keep such a name rather than
drop it.</p>
+ */
static Map<String, String> parsePartitionName(String partName,
List<String> partKeyNames) {
- Map<String, String> values = new HashMap<>();
String[] parts = partName.split("/");
+ List<String> decodedValues = new ArrayList<>(parts.length);
for (String part : parts) {
int eq = part.indexOf('=');
if (eq > 0) {
// Unescape the VALUE: HMS get_partition_names returns
Hive-escaped names (e.g. "%3A" for ':').
// The predicate literal side (extractLiteralValue) is
unescaped, so matchesPredicates' string
// compare needs the value unescaped too — otherwise an
escaped partition value silently drops
// rows. Mirrors the sibling partition-value parse
(HiveWriteUtils.toPartitionValues) and legacy
- // FileUtils.unescapePathName. The KEY must be unescaped too:
Hive's makePartName escapes the
- // column name as well (a special-char partition column such
as `pt2=x!!!! **1+1/&^%3` comes
- // back as an escaped key), and matchesPredicates looks it up
by the real unescaped column name,
- // so an escaped key would silently miss and drop every row.
Unescaping a plain name is a no-op.
- values.put(HiveWriteUtils.unescapePathName(part.substring(0,
eq)),
- HiveWriteUtils.unescapePathName(part.substring(eq +
1)));
+ // FileUtils.unescapePathName. Unescaping a plain name is a
no-op.
+
decodedValues.add(HiveWriteUtils.unescapePathName(part.substring(eq + 1)));
}
}
+ if (partKeyNames.size() != decodedValues.size()) {
+ return null;
+ }
+ Map<String, String> values = new HashMap<>();
+ for (int index = 0; index < partKeyNames.size(); index++) {
+ values.put(partKeyNames.get(index), decodedValues.get(index));
+ }
return values;
}
- private boolean matchesPredicates(Map<String, String> partValues,
+ private boolean matchesPredicates(Map<String, String> partValues,
Map<String, String> partKeyTypes,
Map<String, List<String>> predicates) {
+ // partValues is keyed by exactly the declared partition keys and
predicates only ever names one of them
+ // (see extractPartitionPredicates), so every lookup below is present
by construction.
for (Map.Entry<String, List<String>> entry : predicates.entrySet()) {
String colName = entry.getKey();
List<String> allowedValues = entry.getValue();
- String actualValue = partValues.get(colName);
- if (actualValue == null || !allowedValues.contains(actualValue)) {
+ if (!matchesAnyValue(partValues.get(colName), allowedValues,
partKeyTypes.get(colName))) {
return false;
}
}
return true;
}
+ /**
+ * Whether a rendered partition value matches one of the predicate
literals for its key. Every typed key
+ * (numeric, boolean, date, timestamp) is compared in the same VALUE
domain as the typed Nereids pruner,
+ * because this local prefilter's result is reused as the logical selected
view: a raw text compare would
+ * silently drop a type-equal partition (and, in batch mode, omit it from
the scan altogether). STRING and
+ * the remaining key types keep exact text comparison.
+ */
+ private static boolean matchesAnyValue(String actualValue, List<String>
allowedValues, String typeName) {
+ if (isHmsFloatType(typeName)) {
+ return matchesParsedValue(actualValue, allowedValues,
HiveConnectorMetadata::parseFloatValue);
+ }
+ if (isHmsDoubleType(typeName)) {
+ return matchesParsedValue(actualValue, allowedValues,
HiveConnectorMetadata::parseDoubleValue);
+ }
+ if (isHmsIntegralType(typeName) || isHmsDecimalType(typeName)) {
+ BigDecimal actual = parseDecimalValue(actualValue);
+ if (actual != null) {
+ for (String allowed : allowedValues) {
+ BigDecimal candidate = parseDecimalValue(allowed);
+ if (candidate != null ? candidate.compareTo(actual) == 0 :
allowed.equals(actualValue)) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+ // The remaining typed keys are compared by their VALUE, not by their
rendered text: Hive keeps the
+ // partition value exactly as the DDL spelled it and the predicate
literal as the type's canonical text,
+ // so `b=1` vs `b=true`, `d=2024-1-1` vs `d=2024-01-01`, and `ts` with
a zero fractional suffix all
+ // denote the same value while differing textually. The typed Nereids
pruner keeps them, so this
+ // prefilter - whose result IS the logical selected view - must too.
+ if (isHmsBooleanType(typeName)) {
+ return matchesParsedValue(actualValue, allowedValues,
HiveConnectorMetadata::parseBooleanValue);
+ }
+ if (isHmsDateType(typeName)) {
+ return matchesParsedValue(actualValue, allowedValues,
HiveConnectorMetadata::parseDateValue);
+ }
+ if (isHmsTimestampType(typeName)) {
+ return matchesParsedValue(actualValue, allowedValues,
HiveConnectorMetadata::parseTimestampValue);
+ }
+ return allowedValues.contains(actualValue);
+ }
+
+ /**
+ * Whether {@code actualValue} parses to the same value as one of {@code
allowedValues}, with the typed
+ * comparison the Nereids pruner uses. A text {@code parse} cannot
interpret - on either side - drops back to
+ * a plain string comparison for that value, exactly as the untyped branch
does: this prefilter's result
+ * becomes the logical selected view, so a partition it cannot read may
never be dropped (that loses rows),
+ * while keeping it only loses pruning.
+ */
+ private static <T> boolean matchesParsedValue(String actualValue,
List<String> allowedValues,
+ Function<String, T> parse) {
+ T actual = parse.apply(actualValue);
+ if (actual == null) {
+ return allowedValues.contains(actualValue);
+ }
+ for (String allowed : allowedValues) {
+ T candidate = parse.apply(allowed);
+ if (candidate != null ? candidate.equals(actual) :
allowed.equals(actualValue)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Parses the rendered text of a BOOLEAN partition value. Hive accepts
{@code true}/{@code false} and
+ * {@code 1}/{@code 0} interchangeably for a boolean partition, and the
predicate literal carries whichever
+ * spelling the query used, so both are the same value here.
+ */
+ private static Boolean parseBooleanValue(String value) {
+ if (value == null) {
+ return null;
+ }
+ switch (value.trim().toLowerCase(Locale.ROOT)) {
+ case "true":
+ case "1":
+ return Boolean.TRUE;
+ case "false":
+ case "0":
+ return Boolean.FALSE;
+ default:
+ return null;
+ }
+ }
+
+ /**
+ * Parses the rendered text of a DATE partition value. The components, not
the spelling, carry the value:
+ * {@code 2024-1-1} and the literal's canonical {@code 2024-01-01} are the
same day.
+ */
+ private static LocalDate parseDateValue(String value) {
+ if (value == null) {
+ return null;
+ }
+ Matcher matcher = HMS_DATE_TEXT.matcher(value.trim());
+ if (!matcher.matches()) {
+ return null;
+ }
+ try {
+ return LocalDate.of(Integer.parseInt(matcher.group(1)),
Review Comment:
[P1] Preserve Doris date semantics in the local prefilter. For `Filter(d =
DATE '2024-01-01') -> FileScan(Hive)`, HMS/API-created DATE partitions
`d=20240101` and `d=24-01-01` both decode as 2024-01-01 in Doris's existing
partition-item and BE date parsers. This parser rejects the compact spelling
and interprets the second as year 24; `matchesParsedValue` consequently removes
both before typed pruning. The new connector-filtered logical view carries that
omission into physical planning, including batch scans that previously used the
typed selected-name set, and matching rows disappear. The integral fallback fix
does not cover this new temporal branch. Preserve unknown typed forms
conservatively and use Doris's two-digit-year rules; add compact/two-digit-date
cases through logical pruning and batch planning.
--
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]
