924060929 commented on code in PR #63641:
URL: https://github.com/apache/doris/pull/63641#discussion_r3542809225
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -731,7 +731,16 @@ public PlanFragment visitPhysicalFileScan(PhysicalFileScan
fileScan, PlanTransla
SessionVariable sv = ConnectContext.get().getSessionVariable();
// TODO(cmy): determine the needCheckColumnPriv param
ScanNode scanNode;
- if (table instanceof HMSExternalTable) {
+ // Plugin-driven (SPI) tables are matched first; the connector-specific
+ // instanceof branches below are migration-period fallbacks that get
removed
+ // as each connector lands on the SPI in P3-P7.
+ if (table instanceof PluginDrivenExternalTable) {
+ PluginDrivenExternalCatalog pluginCatalog =
+ (PluginDrivenExternalCatalog) table.getCatalog();
+ scanNode = PluginDrivenScanNode.create(context.nextPlanNodeId(),
tupleDescriptor,
Review Comment:
The plugin branch never forwards `fileScan.getTableSample()` — only the
legacy Hive branch calls `setTableSample(...)`, and PluginDrivenScanNode has no
table-sample plumbing at all (no SPI representation exists). Latent until hms
flips to the SPI (P7), then `SELECT ... TABLESAMPLE(10 PERCENT)` (and
ANALYZE-with-sample paths that rely on scan-node sampling) silently reads the
full table: wrong (unsampled) semantics plus a full-scan cost, with no error.
Suggest either adding sample plumbing to the SPI before the hive cutover, or
fail-loud here (like the hudi incremental/time-travel guards added later at
:901-908) when `getTableSample().isPresent()` on the plugin path. Verified
still absent at tip (plugin branch :806-815; no TableSample occurrence in
PluginDrivenScanNode).
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -819,19 +822,35 @@ public PlanFragment
visitPhysicalEmptyRelation(PhysicalEmptyRelation emptyRelati
@Override
public PlanFragment visitPhysicalHudiScan(PhysicalHudiScan hudiScan,
PlanTranslatorContext context) {
- if (directoryLister == null) {
- this.directoryLister = new
TransactionScopeCachingDirectoryListerFactory(
-
Config.max_external_table_split_file_meta_cache_num).get(new
FileSystemDirectoryLister());
- }
List<Slot> slots = hudiScan.getOutput();
ExternalTable table = hudiScan.getTable();
TupleDescriptor tupleDescriptor = generateTupleDesc(slots, table,
context);
+ SessionVariable sv = ConnectContext.get().getSessionVariable();
+
+ // Plugin-driven (SPI) Hudi: route through PluginDrivenScanNode.
Incremental scan
+ // (hudiScan.getIncrementalRelation) is not yet representable in the
SPI; that
+ // gap is tracked for P3 when Hudi migrates to the connector framework.
+ if (table instanceof PluginDrivenExternalTable) {
+ PluginDrivenExternalCatalog pluginCatalog =
+ (PluginDrivenExternalCatalog) table.getCatalog();
+ ScanNode scanNode =
PluginDrivenScanNode.create(context.nextPlanNodeId(), tupleDescriptor,
Review Comment:
This plugin branch diverges from its `visitPhysicalFileScan` sibling and
from the legacy hudi path below: it never calls
`setSelectedPartitions(hudiScan.getSelectedPartitions())` (legacy does at the
end of this method) nor `setDistributeExprLists(getDistributeExpr(hudiScan))`.
The FileScan sibling was later patched to forward pruned partitions (tip
PhysicalPlanTranslator.java:814), but this branch was not — so the hudi cutover
(P7) will inherit a prune-loss bug that was already found and fixed once on the
other branch: partition-pruned queries read all partitions, and bucket/colocate
planning info is lost.
Root cause is structural: the dispatch block (catalog cast +
PluginDrivenScanNode.create + params wiring) is copy-pasted between the two
visitors and has already drifted. Extracting one shared helper would make the
next divergence impossible. Still present at tip
(PhysicalPlanTranslator.java:911-916 vs :932).
--
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]
