morningman opened a new pull request, #68848:
URL: https://github.com/apache/doris/pull/68848
### What problem does this PR solve?
Issue Number: None
Related PR: #66473, #66530
Original author: @924060929 (the cases follow the tests of branch-4.1
#66473, rewritten for master's connector providers; the commit carries
Co-authored-by)
Problem Summary:
**In short.** master #66530 reuses the planned ranges of equivalent external
scans within one statement, and branch-4.1 carries the same feature as #66473.
Comparing the two found no case where master shares a plan it must not. Still,
master's tests miss most of what keeps that true. If one of the inputs that
keep two plans apart is dropped from a reuse key, nothing fails. Nothing fails
either if a scan that must never reuse starts reusing. This PR adds those tests
for Iceberg, Paimon, Hive and Hudi, plus one for the session variable that
turns reuse on. It changes no production code.
**Background.** In master, each connector's `planScan` keeps its result in
the statement scope under `<connector>.scan-reuse:<catalog id>:<query id>`. The
entry is keyed by a typed key built from the table handle and the scan request,
so equivalent relations plan once: self-joins, repeated CTE references, UNION
branches. The cached value is the final `List<ConnectorScanRange>`: already
split, routed to a reader and, for COUNT pushdown, collapsed into one range. So
every fact those ranges depend on must be part of the key. System tables, Hive
partition batches, Iceberg streaming and transactional Hive scans bypass the
memo. The switch is the session variable `enable_external_scan_task_reuse`
(default true). It reaches a connector as a session property, and a missing
property counts as off. The branch-4.1 version was ported while #66530 was
still under review. It lives in fe-core scan nodes, so its tests
(`IcebergScanNodeTest`, `PaimonScanNodeTest`, `HiveScanNodeTest`, `HudiScanNode
Test`, `StatementContextTest`, ...) do not apply to master as written.
**The problem, and what it cost.**
- Master's tests check that an identical scan reuses its plan. They also
check that a few inputs keep plans apart: Hudi instants and partition sets,
null versus empty partition lists, structurally equal filters. For most of the
key, nothing fails if a fact is dropped:
- Iceberg: COUNT pushdown, ref, rewrite file scope, schema id, catalog id.
- Paimon: LIMIT and COUNT pushdown, pinned snapshot, scan options,
projection, branch. The existing LIMIT test runs with reuse off.
- Hive: serde, input format, first-column flag, an unpartitioned table's
location, catalog id.
- Hudi: serde, input format, partition key order, generation, base path.
Several of these would return wrong results, because a scan would get
another scan's ranges: the single collapsed COUNT range, LIMIT 1's single
split, a tag's or a rewrite group's subset of files, or ranges planned for
another file format.
- Nothing checks the bypasses. A system table, a Hive partition batch or a
transactional Hive scan entering the memo would go unnoticed. A transactional
scan would then skip opening its read transaction.
- Nothing checks that the session variable reaches the connector session.
Renaming it or making it invisible would silently turn reuse off for every
connector, because the connector tests set the property by hand.
**How this PR fixes it.** Only tests, plus small test-fixture hooks: a
catalog id on the fake sessions, and a probe on the test statement scopes for
whether the reuse memo was installed.
- `IcebergScanPlanProviderTest`
- Scans that differ in snapshot, ref, schema, filter, table, catalog,
COUNT pushdown or rewrite scope never share a plan.
- `$snapshots` and `$position_deletes` never enter the memo.
- A plan that fails is not kept, and the retry is reused.
- `PaimonScanPlanProviderTest`
- Scans that differ in pinned snapshot, bounded incremental range, filter,
projection, LIMIT, COUNT pushdown, branch or catalog never share a plan.
- The same incremental range given in two option orders still shares.
- System tables never enter the memo.
- `HiveScanBatchModeTest`
- Equal handles built separately share a plan.
- Partition, table, serde, first-column flag, file format, catalog and an
unpartitioned table's location keep plans apart.
- Partition batches and transactional scans never enter the memo.
- `HudiScanReuseKeyTest`: input format, serde, partition key order,
generation, base path, database and table each separate the key.
- `ConnectorSessionImplTest`: `enable_external_scan_task_reuse` reaches the
connector session; it is on by default and off when set to false.
**Results.** Each change below was made to the production code locally, and
the module's test class was run against it:
| Change to production code | Existing cases in the class | New cases |
|---|---|---|
| Iceberg key without COUNT pushdown, and system tables reused | all 181
pass | `statementReuseKeepsScansWithDifferentPlanningInputsApart` (the COUNT
scan gets the plain scan's 2 ranges) and
`statementReuseNeverMemoizesSystemTables` fail |
| Paimon key without LIMIT, and system tables reused | all 103 pass |
`statementReuseKeepsScansWithDifferentPlanningInputsApart` (LIMIT 1 gets the
plain scan's 3 ranges) and `statementReuseNeverMemoizesSystemTables` fail |
| Hive key without the serde, and transactional scans reused | all 25 pass |
`statementReuseKeepsScansWithDifferentInputsApart` (the CSV scan gets the TEXT
plan) and `transactionalScanNeverUsesStatementReuse` fail |
| Hudi key without the serde | all 14 pass |
`everyRangeInputSeparatesTheKey` fails |
| `enable_external_scan_task_reuse` renamed | all 19 pass |
`testExternalScanTaskReuseFollowsTheSessionVariable` fails |
The comparison also found two memory differences, handled in separate PRs.
Master keeps every synchronous scan's ranges until the statement ends, even
when nothing reuses them; branch-4.x caps what it keeps. A prepared EXECUTE
that fails after planning keeps its plans until the next EXECUTE.
**Classes, and how they call each other.**
```
PluginDrivenScanNode.getSplits
└─ {Iceberg,Paimon,Hive,Hudi}ScanPlanProvider.planScan(session, request)
├─ reuse off / system table / transactional Hive ──> plan directly
└─
session.getStatementScope().computeIfAbsent("<connector>.scan-reuse:<catalog>:<query>")
└─ memo.computeIfAbsent(<Connector>ScanReuseKey(handle,
request), plan)
HiveScanPlanProvider.planScanForPartitionBatch ──> plan directly (never the
memo)
ConnectorSessionBuilder.from(ctx) ──> session property
enable_external_scan_task_reuse
└─ ConnectorSession.isExternalScanTaskReuseEnabled()
```
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
`run-fe-ut.sh` locally: `IcebergScanPlanProviderTest` 184,
`PaimonScanPlanProviderTest` 105, `HiveScanBatchModeTest` 28,
`HudiScanReuseKeyTest` 15 and `ConnectorSessionImplTest` 20, all passed. The
mutations in the table above were each run against the module's whole test
class.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
