zhaorongsheng commented on code in PR #67725:
URL: https://github.com/apache/doris/pull/67725#discussion_r4255899703
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -2495,53 +2591,269 @@ 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;
+ }
+ 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);
Review Comment:
Addressed in `[fix](fe) Decline mixed-case HMS keys and degrade MTMV warmup`.
`buildHmsPartitionFilter` now declines the direct HMS path for any key that
is not already in its stored lowercase form (`HiveConnectorMetadata.java`):
```java
if (!partKeyName.equals(partKeyName.toLowerCase(Locale.ROOT))) {
return null;
}
```
A `null` return makes `applyFilter` fall back to the positional local
prefilter (`prunePartitionNames` over `listPartitionNames`), so a mixed-case
key no longer emits a `P = 'x'` fragment that JDO's `ExpressionTree` compares
against the stored lowercase `p=x` name.
Covering test:
`HiveConnectorMetadataPartitionPruningTest.testHmsFilterDeclinesUppercasePartitionKeyForJdoCaseMismatch`.
It models the JDO case-mismatch response with a client whose filtered list is
empty (any filter path yields nothing), asserts the pruned view is still the
correct `[p=1]`, and asserts `client.filter == null` plus that local name
listing was used — i.e. the direct path is never taken for `P`.
Run: `./run-fe-ut.sh --run
"org.apache.doris.connector.hive.HiveConnectorMetadataPartitionPruningTest,org.apache.doris.datasource.mvcc.PluginDrivenMvccExternalTableTest,org.apache.doris.nereids.StatementContextTest"`
→ Hive 30/30, PluginDrivenMvcc 94/94, StatementContext 25/25, BUILD SUCCESS.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -714,11 +692,142 @@ static boolean schemaCacheDisabled(Connector connector) {
@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() &&
sharedView.get().hasScanPartitionView()
+ && sharedView.get().getScanPartitionView().isPresent()) {
+ return sharedView.get().getScanPartitionView().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());
+ }
+ }
+ return super.getNameToPartitionItemsForScan(snapshot);
+ }
+
+ @Override
+ public Optional<ConnectorFilteredPartitionView>
applyPartitionFilterForScan(Optional<MvccSnapshot> snapshot,
+ ConnectorExpression partitionFilter) {
+ if (snapshot.isPresent() && snapshot.get() instanceof
PluginDrivenMvccSnapshot
+ && ((PluginDrivenMvccSnapshot)
snapshot.get()).isPartitionViewUnavailable()) {
+ return Optional.empty();
+ }
+ return super.applyPartitionFilterForScan(snapshot, partitionFilter);
+ }
+
+ /**
+ * Threads this statement's MVCC pin onto the handle the partition view is
enumerated from, so a
+ * time-travel / {@code @options} query never prunes against the latest
generation: the data scan reads the
+ * pinned snapshot, and a latest view can be missing (or contain)
partitions it will never read.
+ */
+ @Override
+ protected ConnectorTableHandle pinPartitionViewHandle(ConnectorTableHandle
handle,
+ ConnectorMetadata metadata, ConnectorSession session,
Optional<MvccSnapshot> snapshot) {
+ if (snapshot.isPresent() && snapshot.get() instanceof
PluginDrivenMvccSnapshot) {
+ return metadata.applySnapshot(session, handle,
+ ((PluginDrivenMvccSnapshot)
snapshot.get()).getConnectorSnapshot());
+ }
+ return handle;
+ }
+
+ /**
+ * Materializes the complete partition view for an MTMV refresh before it
acquires base-table locks.
+ * The lightweight Hive query pin keeps the connector snapshot/freshness
kind but deliberately omits the
+ * partition map; MTMV alignment needs that map and must not load it while
holding internal table locks.
+ */
+ public MvccSnapshot materializePartitionViewForMtmv(MvccSnapshot snapshot)
{
+ if (!supportsConnectorPartitionPruning() || !(snapshot instanceof
PluginDrivenMvccSnapshot)) {
+ return snapshot;
+ }
+ PluginDrivenMvccSnapshot pin = (PluginDrivenMvccSnapshot) snapshot;
+ if (!pin.isPartitionViewDeferred()) {
+ return pin;
+ }
+ Map<String, PartitionItem> partitionItems =
super.getNameToPartitionItems(Optional.of(pin));
Review Comment:
Addressed in `[fix](fe) Decline mixed-case HMS keys and degrade MTMV warmup`.
`materializePartitionViewForMtmv` no longer calls the fail-loud eager build.
It now enumerates with the LIST path's per-partition contract —
`listPartitions(...)` logs the unrepresentable partition and leaves the item
map short of the listed names — so `isPartitionInvalid()` is true, the base
table degrades to `UNPARTITIONED`, and the whole-MV refresh continues as a
scan-all (`PluginDrivenMvccExternalTable.java`). Enumeration is taken from the
snapshot-pinned handle, so the warmup reads the same generation as the data
scan.
Covering test:
`PluginDrivenMvccExternalTableTest.testMtmvWarmupDegradesOnUnrepresentableTypedPartition`.
With `dt=2024-01-01` and an unrepresentable `dt=not-a-date` DATEV2 partition,
it asserts the materialized view holds 1 item (the bad one is skipped, not
thrown), `isPartitionInvalid()` is true, and `getPartitionType(...)` is
`UNPARTITIONED` with no partition columns; it also verifies `applySnapshot` +
`listPartitions` were invoked on the pinned handle.
Run: `./run-fe-ut.sh --run
"org.apache.doris.connector.hive.HiveConnectorMetadataPartitionPruningTest,org.apache.doris.datasource.mvcc.PluginDrivenMvccExternalTableTest,org.apache.doris.nereids.StatementContextTest"`
→ Hive 30/30, PluginDrivenMvcc 94/94, StatementContext 25/25, BUILD SUCCESS.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -166,6 +172,16 @@ private PluginDrivenMvccSnapshot materializeLatest(
ConnectorMvccSnapshot connectorSnapshot = existingFence.orElseGet(
() -> metadata.beginQuerySnapshot(session,
handle).orElseGet(this::emptySnapshot));
+ // A connector that can materialize a partition predicate remotely
must not populate the latest
+ // snapshot with every partition before Nereids has supplied that
predicate. Plain Hive reaches this
+ // MVCC table class because the catalog also serves snapshot-capable
sibling formats, but its latest
+ // pin is deliberately not a data snapshot and applySnapshot is a
no-op. Keep only that lightweight
+ // query-begin pin here; PruneFileScanPartition will request the
selected partition view later.
+ if (supportsConnectorPartitionPruning()) {
Review Comment:
Addressed. Both deferred-materialization paths now pin the MVCC snapshot
before touching metadata: `pinPartitionViewHandle(...)` applies
`metadata.applySnapshot` to the resolved handle and the filtered path then runs
`applyFilter` on that pinned handle, while the full path runs `listPartitions`
on it. Neither path prunes against the freshly resolved latest metadata any
more, so a snapshot query cannot short-circuit on a stale empty selection.
Covering tests:
`PluginDrivenMvccExternalTableTest.testConnectorPartitionPruningPinsSnapshotBeforeFilteredMaterialization`
(verifies `applyFilter` is called on the pinned handle, never the base handle)
and `testMtmvWarmupDegradesOnUnrepresentableTypedPartition` /
`testMaterializedEmptyMtmvSnapshotIsNotRehydrated` (verify `listPartitions`
runs on the pinned handle).
--
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]
