github-actions[bot] commented on code in PR #66413:
URL: https://github.com/apache/doris/pull/66413#discussion_r3710864780
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -190,6 +196,64 @@ public PluginDrivenScanNode(PlanNodeId id, TupleDescriptor
desc,
this.currentHandle = tableHandle;
}
+ @Override
+ protected void doInitialize() throws UserException {
+ super.doInitialize();
+
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,
+ () -> scanProvider.canServeMetadataOnlyCount(
Review Comment:
[P1] Prove metadata COUNT on the same pinned handle that is planned
This capability check runs before `pinMvccSnapshot()` updates
`currentHandle`, while `planScan()` later uses the pinned handle and recomputes
whether the summary can serve the count. For a time-travel/reference query (or
a snapshot change between phases), the early latest snapshot can return true,
disabling the old-BE fence, but the selected snapshot can contain deletes or
missing counters and fall back to real Variant file ranges. Please apply the
statement pin before using this escape hatch, or base the bypass on the actual
planned ranges; add a test where latest is countable but the pinned snapshot is
not.
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/handle/ConnectorWriteHandle.java:
##########
@@ -100,6 +100,14 @@ default boolean isRequireMergeCardinalityCheck() {
return false;
}
+ /**
+ * Whether this write can emit data files. A delete-only MERGE returns
false so a connector may
+ * allow position-delete output even when the table has read-only column
types.
+ */
+ default boolean isWritesDataFiles() {
Review Comment:
[P1] Bump the connector plugin API for these new SPI methods
This method and `ConnectorScanPlanProvider.canServeMetadataOnlyCount()`
extend the public connector SPI, but the PR leaves the kernel/plugin API at
3.0. A new Iceberg plugin is therefore admitted by an old 3.0 FE; because
connector SPI classes are parent-first, `planWrite()` then invokes
`isWritesDataFiles()` on the old kernel interface and fails with
`NoSuchMethodError`. Please apply the repository's required major bump
(including the test pin/baseline), and include these reachable provider/handle
types in the frozen surface so this cannot evade the guard.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -190,6 +196,64 @@ public PluginDrivenScanNode(PlanNodeId id, TupleDescriptor
desc,
this.currentHandle = tableHandle;
}
+ @Override
+ protected void doInitialize() throws UserException {
+ super.doInitialize();
+
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,
+ () -> scanProvider.canServeMetadataOnlyCount(
+ connectorSession, currentHandle,
Optional.empty()));
+ }
+ checkVariantBackendCompatibility(
+ !metadataCountProven && projectsComputeVariant(desc),
backends);
+ }
+
+ static boolean projectsComputeVariant(TupleDescriptor tuple) {
+ // Nested-column pruning updates the effective slot type but
deliberately keeps the original
+ // Column metadata; compatibility must follow the payload this scan
actually projects.
+ return tuple.getSlots().stream().anyMatch(slot ->
containsComputeVariant(slot.getType()));
+ }
+
+ private static boolean containsComputeVariant(Type type) {
+ if (type instanceof VariantType) {
+ return ((VariantType) type).isComputeV2();
+ }
+ if (type instanceof ArrayType) {
+ return containsComputeVariant(((ArrayType) type).getItemType());
+ }
+ if (type instanceof MapType) {
+ MapType map = (MapType) type;
+ return containsComputeVariant(map.getKeyType()) ||
containsComputeVariant(map.getValueType());
+ }
+ if (type instanceof StructType) {
+ return ((StructType) type).getFields().stream()
+ .anyMatch(field ->
containsComputeVariant(field.getType()));
+ }
+ return false;
+ }
+
+ static void checkVariantBackendCompatibility(boolean projectsVariant,
Iterable<Backend> backends)
+ throws UserException {
+ if (!projectsVariant) {
+ return;
+ }
+ for (Backend backend : backends) {
+ if (backend.isSmoothUpgradeSrc()) {
Review Comment:
[P1] Fence ordinary rolling-upgrade backends too
`isSmoothUpgradeSrc()` identifies only the cloud colocated smooth-upgrade
source: its production setter is in `CloudSystemInfoService`, while ordinary
backends default this bit to false. During a normal mixed-version cluster
upgrade, an old BE therefore passes this loop and can be assigned an Iceberg
Variant scan even though it lacks the new carrier/native reader. Please gate on
an explicit BE capability or execution version that covers every eligible
backend, and test an old non-cloud backend rather than manually setting the
cloud-only flag.
--
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]
