Repository navigation
Conversation
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary: PushProjectThroughUnion allowed cast projections to be pushed below UNION DISTINCT. For non-injective casts, distinct was then evaluated after the cast instead of before the cast, which could incorrectly collapse rows that were distinct in the original union output. Restrict cast project pushdown to UNION ALL and add tests for the qualifier guard and the DORIS-26135 datetime-to-date regression case.
### Release note
None
### Check List (For Author)
- Test: Regression test / Unit Test
- ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.PushProjectThroughUnionTest
- ./run-regression-test.sh --run -d nereids_syntax_p0 -s set_operation
- mvn checkstyle:check -pl fe-core
- Behavior changed: No
- Does this need documentation: No
There was a problem hiding this comment.
Review result: request changes.
Critical checkpoint conclusions:
- Goal and behavior: the PR fixes the non-injective cast case for UNION DISTINCT and the added datetime-to-date regression captures that path, but the same rule still allows a reachable non-injective bare-slot projection.
- Scope and clarity: the change is narrow, but the safety check is incomplete for DISTINCT set operations because it only guards cast aliases.
- Parallel paths:
PushProjectThroughUnionapplies throughLogicalSetOperation/ProjectProcessor, and the duplicate bare-slot issue is the same root cause for DISTINCT set operations; unsupported INTERSECT ALL / EXCEPT ALL is rejected during binding, so that risk was dismissed. - Tests: the new unit/regression tests cover cast aliases, but they do not cover duplicate slot projections such as
SELECT a, aover a distinct set operation. - Concurrency, lifecycle, config, persistence, FE/BE protocol, observability: not involved in this PR.
No builds or tests were run because this review environment is instructed to perform review-only operations.
| @@ -59,10 +60,11 @@ public static boolean canPushProject(List<NamedExpression> projects, LogicalSetO | |||
| return projects.size() == logicalSetOperation.getOutput().size() && projects.stream().allMatch(e -> { | |||
There was a problem hiding this comment.
This still allows a non-injective projection through UNION DISTINCT when the projection is made only of bare slots. For example, SELECT a, a FROM (SELECT 1 a, 2 b UNION SELECT 1, 3) t should run the distinct union on (a,b) first, preserving two rows, then return two identical (1,1) rows. If this rule pushes [a, a] into both union children, the union sees (1,1) from both sides and can collapse them to one row. For DISTINCT set operations, the slot-only path also needs to prove the projection is one-to-one over the set-operation outputs, not just reject cast aliases.
…ability (#65178) The Codex GitHub Actions review pipeline needed stronger guarantees that agents review the intended PR snapshot, use one authoritative diff, pursue high-risk cross-module mechanisms, and expose the work of spawned subagents in Litefuse. This PR: - verifies the checked-out PR head and rechecks the live base/head after context preparation; - prepares an authoritative aggregate PR patch and changed-file list, with atomic bounded retry (`2s`, `4s`, `8s`) for transient GitHub API failures; - directs the main agent to perform an initial risk scan, dispatch focused subagents for suspicious mechanisms, maintain a shared append-only ledger, validate/deduplicate every candidate, and converge for at most three rounds; - requires paths outside the authoritative diff/changed-file context to be confirmed with `rg --files` before reading; - records separate Litefuse traces for each Codex subagent session, including command/tool I/O and non-empty encrypted-reasoning status, instead of mixing subagent activity into the main trace; - makes Litefuse ingestion more resilient to transport timeouts while keeping telemetry upload non-blocking for the review outcome. The repository review skill is kept focused on review invariants; orchestration and completion policy live in the workflow prompt, where they are available to the GitHub Actions invocation. ### Validation and experiment material - Static checks: `python3 -m py_compile .github/scripts/emit_litefuse_otel_io.py` and `git diff --check`. - Litefuse uploader validation: real/synthetic subagent session conversion, payload-limit coverage, and readback of independent subagent traces. - End-to-end replayed review experiments against fresh fork PRs built from the original PR base/head commits: | Replay PR | GitHub Actions run | Result | | --- | --- | --- | | [#136](zclllyybb#136) | [29112823062](https://github.com/zclllyybb/doris/actions/runs/29112823062) | Success; 2 inline comments | | [#137](zclllyybb#137) | [29114260631](https://github.com/zclllyybb/doris/actions/runs/29114260631) | Success; 3 inline comments | | [#138](zclllyybb#138) | [29115946034](https://github.com/zclllyybb/doris/actions/runs/29115946034) | Success; 1 inline comment | | [#139](zclllyybb#139) | [29150372758](https://github.com/zclllyybb/doris/actions/runs/29150372758) | Success; 1 inline comment | | [#140](zclllyybb#140) | [29151057520](https://github.com/zclllyybb/doris/actions/runs/29151057520) | Success; 1 inline comment | The latest replay trace, `86be226391a40e5992bb8d3b4e125eca`, confirms one authoritative `pr.diff` read, two changed-file-list reads, no `gh pr diff` or `git diff` calls from the reviewing agent, and separate subagent sessions. It also exposed one remaining prompt-compliance gap: a guessed adjacent `.cpp` path was read without a preceding `rg --files` lookup; this is documented test evidence rather than hidden by the validation.
Fresh replay PR for validating the current aggregate-diff review workflow. The base and head reproduce the referenced Apache Doris PR snapshot.