Gabriel39 commented on code in PR #66413:
URL: https://github.com/apache/doris/pull/66413#discussion_r3791092085
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -194,6 +205,86 @@ public PluginDrivenScanNode(PlanNodeId id, TupleDescriptor
desc,
this.currentHandle = tableHandle;
}
+ @Override
+ protected void doInitialize() throws UserException {
+ super.doInitialize();
+ // Pin before projection pruning so every later connector decision
uses this scan's snapshot.
+ // The Variant compatibility fence itself runs in finalize, after
Nereids prunes scan slots.
+ pinMvccSnapshot();
+ }
+
+ void checkVariantBackendCompatibilityForCurrentScan(Iterable<Backend>
backends)
+ throws UserException {
+ boolean projectsVariant = projectsComputeVariant(desc);
+ boolean metadataCountProven = false;
+ ConnectorScanPlanProvider scanProvider = resolveScanProvider();
+ if (isTableLevelCountStarPushdown() && conjuncts.isEmpty() &&
scanProvider != null) {
Review Comment:
Addressed in 472954f6908. Metadata-count capability no longer bypasses
compatibility before planning; TABLESAMPLE disables count pushdown, and the
finalized data ranges are then fenced before dispatch to an old backend. A
sampled one-Variant-column COUNT regression was added.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -1663,6 +1777,10 @@ private boolean computeBatchMode() {
if (tableSample != null && onPluginClassLoader(scanProvider,
scanProvider::supportsTableSample)) {
return false;
}
+ if (variantCompatibilityDeferred) {
Review Comment:
Addressed in 472954f6908. Projected Variant COUNT plans always set the
deferred range-proof flag, which also forces computeBatchMode to stay
synchronous. The combined metadata-count and partition-batch capability
regression verifies that batch mode cannot be entered before the range proof.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1576,15 +1601,103 @@ private long computeSplitWeight(DataSplit dataSplit) {
* must still be allowed native.
*
* <p>{@code forceJniScanner} is the user/session escape hatch ({@code SET
force_jni_scanner=true},
- * read via {@link #isForceJniScannerEnabled}): when set, every
native-eligible split is routed to
- * JNI to dodge native-reader bugs. Default false, so normal reads are
unaffected.
+ * read via {@link #isForceJniScannerEnabled}): when set, every
native-eligible non-Variant split is
+ * routed to JNI to dodge native-reader bugs. Variant projections on
ordinary tables stay native because
+ * JNI cannot carry Variant columns, but the semantic handle-level force
remains unconditional. Default
+ * false, so normal reads are unaffected.
*
* <p>Extracted as a pure static so the correctness-critical routing
decision is unit-testable
* with real {@link RawFile}s, without driving a full Paimon {@code
ReadBuilder}/{@code TableScan}.
*/
static boolean shouldUseNativeReader(boolean forceJni, boolean
forceJniScanner,
Optional<List<RawFile>> optRawFiles) {
- return !forceJni && !forceJniScanner &&
supportNativeReader(optRawFiles);
+ return shouldUseNativeReader(forceJni, forceJniScanner, false,
optRawFiles);
+ }
+
+ static boolean shouldUseNativeReader(boolean forceJni, boolean
forceJniScanner,
+ boolean hasVariantProjection, Optional<List<RawFile>> optRawFiles)
{
+ Set<Long> physicalVariantSchemaIds = hasVariantProjection &&
optRawFiles.isPresent()
+ ?
optRawFiles.get().stream().map(RawFile::schemaId).collect(Collectors.toSet())
+ : Collections.emptySet();
+ return shouldUseNativeReader(forceJni, forceJniScanner,
hasVariantProjection,
+ physicalVariantSchemaIds, optRawFiles);
+ }
+
+ static boolean shouldUseNativeReader(boolean forceJni, boolean
forceJniScanner,
+ boolean hasVariantProjection, Set<Long> physicalVariantSchemaIds,
+ Optional<List<RawFile>> optRawFiles) {
+ // Handle-level force marks system-table semantics, while only the
session debugging knob may be
+ // overridden for Variant. An ORC file is safe only when its
historical physical schema predates
+ // the projected Variant field; BE never needs to install a Variant
schema override for that file.
+ return !forceJni && (hasVariantProjection
+ ? supportNativeVariantReader(optRawFiles,
physicalVariantSchemaIds)
+ : !forceJniScanner && supportNativeReader(optRawFiles));
+ }
+
+ private static Set<Long> physicalVariantSchemaIds(Table table, RowType
currentRowType,
+ List<ConnectorColumnHandle> columns, List<DataSplit> dataSplits) {
+ Set<Integer> projectedVariantFieldIds = columns.stream()
+ .filter(PaimonColumnHandle.class::isInstance)
+ .map(PaimonColumnHandle.class::cast)
+ .map(column -> currentRowType.getFields().stream()
+ .filter(field ->
field.name().equalsIgnoreCase(column.getName()))
+ .findFirst().orElse(null))
+ .filter(Objects::nonNull)
+ .filter(field -> containsVariant(field.type()))
+ .map(DataField::id)
+ .collect(Collectors.toSet());
+ if (projectedVariantFieldIds.isEmpty()) {
+ return Collections.emptySet();
+ }
+
+ Set<Long> rawSchemaIds = new HashSet<>();
+ for (DataSplit split : dataSplits) {
+ split.convertToRawFiles()
+ .ifPresent(files -> files.forEach(file ->
rawSchemaIds.add(file.schemaId())));
+ }
+ if (!(table instanceof FileStoreTable)) {
Review Comment:
Addressed in 67d923c6e033. Read-optimized scans now resolve SchemaManager
state through the pinned base table carried by the handle. The regression
creates a historical ORC file, adds a Variant field to the pinned generation,
and verifies that the old file remains native-readable.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/ConnectorComputeVariantType.java:
##########
@@ -0,0 +1,42 @@
+// 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.nereids.types;
+
+import org.apache.doris.catalog.Type;
+
+/** Execution-only Variant marker retained while connector scan slots pass
through Nereids. */
+public final class ConnectorComputeVariantType extends VariantType {
+
+ public static final ConnectorComputeVariantType INSTANCE = new
ConnectorComputeVariantType();
+
+ private ConnectorComputeVariantType() {
+ super(0);
+ }
+
+ @Override
+ public DataType conversion() {
Review Comment:
Addressed in 67d923c6e033. Conversion at persisted query-schema boundaries
normalizes the execution-only marker to ordinary Variant while executable scan
slots retain the marker. CTAS, MTMV, view, and Gson replay coverage was added.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1600,6 +1658,12 @@ private static boolean
supportNativeReader(Optional<List<RawFile>> optRawFiles)
return true;
}
+ private static boolean supportNativeVariantReader(Optional<List<RawFile>>
optRawFiles) {
+ return optRawFiles.isPresent() && !optRawFiles.get().isEmpty()
Review Comment:
Addressed in 0f45d1a6cb95. Native routing now uses the physical Variant
field IDs resolved from each historical schema: ORC files without a physical
Variant field remain native-readable, while ORC files that contain one are
still rejected. The mixed-format schema-evolution case is covered in
PaimonScanPlanProviderTest.
--
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]
