yjhjstz opened a new pull request, #1672:
URL: https://github.com/apache/cloudberry/pull/1672

   
   ## Changes
   
   Two enhancements to `CostHashAgg` in GPORCA's `CCostModelGPDB`:
   
   1. **Single-column NDV optimization**: When `GROUP BY` has exactly one 
column, use `GetNDVs()` (global NDV from column statistics) instead of 
`pci->Rows()` to estimate the local partial aggregation output. This allows the 
optimizer to distinguish high-NDV cases (partial agg streams nearly as many 
rows as input → 1-phase preferred) from low-NDV cases (partial agg 
significantly reduces data → 2-phase preferred). Multi-column `GROUP BY` falls 
back to the original `pci->Rows() * UlHosts()` logic.
   
   2. **Spill-aware cost model**: When `num_output_rows × width` exceeds the 50 
MB spilling threshold, apply higher cost unit values to reflect disk I/O 
overhead.
   
   ---
   
   ## TPC-H: -13.0% overall (274,201ms → 238,438ms)
   
   Only 2 queries changed plans:
   
   | Query | Baseline | Optimized | Change | Cause |
   |---|---:|---:|---|---|
   | **Q17** | 60,282ms | 24,005ms | **-60.2%** | 2-phase → 1-phase; eliminated 
1.3 GB disk spill |
   | Q03 | 19,977ms | 19,513ms | -2.3% | HashAgg → GroupAgg; avoided hash spill 
|
   
   Q17 is the dominant win. `l_partkey` is a high-NDV single-column group key 
(~6.6M distinct values); partial aggregation produced 19.6M output rows (nearly 
identical to input), causing 1.3 GB of disk spill in the Finalize stage. The 
optimizer now correctly chooses 1-phase, reducing spill to 490 MB and cutting 
latency by 36 seconds. No plan change caused a regression.
   
   ---
   
   ## TPC-DS: +0.1% overall (605,646ms → 606,421ms)
   
   Only 2 queries changed plans:
   
   | Query | Baseline | Optimized | Change | Cause |
   |---|---:|---:|---|---|
   | **Q59** | 11,141ms | 7,686ms | **-31.0%** | 2-phase → 1-phase; eliminated 
Planned Partitions: 4 spill |
   | Q11 | 9,830ms | 11,327ms | +15.2% | Statistics drift — not a code issue |
   
   Q59 benefits for the same reason as Q17. The Q11 regression is caused 
entirely by statistics drift (`date_dim` filter estimate shifted from 271 to 
~237 rows due to re-sampling), not by the code change — the code takes the 
identical fallback path as the original binary for Q11's 8-column `GROUP BY`. 
After re-running `ANALYZE` with `default_statistics_target = 1000`, Q11 
recovers to +3.7% (within noise).
   
   ---
   
   ## Combined Assessment
   
   | Benchmark | Overall | Plan-change win | Plan-change regression |
   |---|---|---|---|
   | TPC-H | **-13.0%** | Q17 -60% | None |
   | TPC-DS | **+0.1%** | Q59 -31% | None (Q11 is statistics drift) |
   
   
   Fixes #ISSUE_Number
   
   ### What does this PR do?
   <!-- Brief overview of the changes, including any major features or fixes -->
   
   ### Type of Change
   - [ ] Bug fix (non-breaking change)
   - [ ] New feature (non-breaking change)
   - [ ] Breaking change (fix or feature with breaking changes)
   - [ ] Documentation update
   
   ### Breaking Changes
   <!-- Remove if not applicable. If yes, explain impact and migration path -->
   
   ### Test Plan
   <!-- How did you test these changes? -->
   - [ ] Unit tests added/updated
   - [ ] Integration tests added/updated
   - [ ] Passed `make installcheck`
   - [ ] Passed `make -C src/test installcheck-cbdb-parallel`
   
   ### Impact
   <!-- Remove sections that don't apply -->
   **Performance:**
   <!-- Any performance implications? -->
   
   **User-facing changes:**
   <!-- Any changes visible to users? -->
   
   **Dependencies:**
   <!-- New dependencies or version changes? -->
   
   ### Checklist
   - [ ] Followed [contribution 
guide](https://cloudberry.apache.org/contribute/code)
   - [ ] Added/updated documentation
   - [ ] Reviewed code for security implications
   - [ ] Requested review from [cloudberry 
committers](https://github.com/orgs/apache/teams/cloudberry-committers)
   
   ### Additional Context
   <!-- Any other information that would help reviewers? Remove if none -->
   
   ### CI Skip Instructions
   <!--
   To skip CI builds, add the appropriate CI skip identifier to your PR title.
   The identifier must:
   - Be in square brackets []
   - Include the word "ci" and either "skip" or "no"
   - Only use for documentation-only changes or when absolutely necessary
   -->
   
   ---
   <!-- Join our community:
   - Mailing list: 
[[email protected]](https://lists.apache.org/[email protected])
 (subscribe: [email protected])
   - Discussions: https://github.com/apache/cloudberry/discussions -->
   


-- 
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]

Reply via email to