github-actions[bot] commented on code in PR #66413:
URL: https://github.com/apache/doris/pull/66413#discussion_r3789645759
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -194,6 +201,79 @@ public PluginDrivenScanNode(PlanNodeId id, TupleDescriptor
desc,
this.currentHandle = tableHandle;
}
+ @Override
+ protected void doInitialize() throws UserException {
+ super.doInitialize();
+ // Compatibility must inspect the snapshot-specific handle: latest
metadata may answer
+ // COUNT(*) while an older time-travel snapshot still requires a
Variant data scan.
+ pinMvccSnapshot();
+
checkVariantBackendCompatibilityForCurrentScan(backendPolicy.getBackends());
+ }
+
+ void checkVariantBackendCompatibilityForCurrentScan(Iterable<Backend>
backends)
+ throws UserException {
+ boolean metadataCountProven = false;
+ ConnectorScanPlanProvider scanProvider = resolveScanProvider();
+ if (isTableLevelCountStarPushdown() && conjuncts.isEmpty() &&
scanProvider != null) {
+ metadataCountProven = onPluginClassLoader(scanProvider,
+ () -> canServeMetadataOnlyCount(scanProvider,
connectorSession, currentHandle));
Review Comment:
[P1] Do not fence Paimon counts that never decode Variant. This bypass asks
the provider for a metadata-only proof, but Paimon inherits the default `false`
even though its first routing arm siphons every `DataSplit` with
`mergedRowCountAvailable()`, collapses an all-countable table to one
`paimon.row_count` range, and BE serves that range with `CountReader`.
`COUNT(*)` pruning retains the smallest scan slot, so a one-column Variant
table still exposes `ConnectorComputeVariantType` here and is rejected at
execution version 11 before `planScan()`, although no V2 payload would be sent.
Please let Paimon prove the all-split condition on this pinned handle (reusing
the planned splits), or apply the fence after ranges are known, and cover an
old-BE all-countable Variant count plus a partial-count fallback.
##########
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:
[P1] Keep this execution marker out of persisted schemas. CTAS derives each
output with `s.getDataType().conversion()`, so this override preserves the
marker; `ColumnDefinition.translateToCatalogStyle()` then calls
`toCatalogDataType()` and installs the catalog-side subclass in the internal
table. CREATE VIEW and MTMV schema derivation reach the same durable boundary.
That metadata is journaled with `GsonUtils.GSON`, whose `Type` adapter
registers exact `VariantType` but not `ConnectorComputeVariantType`, so
serialization throws `did you forget to register a subtype?` (and persisting
the marker would violate its execution-only/rolling-upgrade contract anyway).
Please normalize or intentionally reject this type at persisted query-schema
boundaries while retaining it in executable scan slots, and add CTAS/view/MTMV
edit-log round-trip coverage.
##########
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:
[P1] Resolve `$ro` schemas through the pinned base table. A `$ro` scan is a
`ReadOptimizedTable`, not a `FileStoreTable`, although it reads the wrapped
base table's raw files and the handle retains that exact base in
`getSysBaseTable()`. This branch therefore marks every raw schema ID as
physically containing Variant. An old ORC file that predates the nullable
Variant addition is then rejected by `supportNativeVariantReader()`, and the
JNI fallback throws even though that file has no Variant payload and could
synthesize NULL. Please obtain the `SchemaManager` from the pinned base table
(as `resolveSchemaDictTable()` already does) and cover `$ro` with historical
ORC followed by a Variant schema addition.
--
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]
