terrymanu commented on PR #40229:
URL: https://github.com/apache/shardingsphere/pull/40229#issuecomment-6258859203
### Result
Review Result: Mergeable
No blocking issues found. During projection-context creation, null text is
skipped without throwing an exception or creating a derived projection with a
null expression.
### Evidence
- `ProjectionsContextEngine.java:93–109`: a null normalization result stops
matching; null original text skips derived-projection creation while preserving
existing projections.
- Three tests cover null ORDER BY text, null GROUP BY text, and initially
empty projections. Their assertions detect incorrectly appended derived
projections.
- Verified test results for identical source content: all 18 SQLUtilsTest
tests and 16 ProjectionsContextEngineTest tests passed. The utility method’s
existing `@Nullable` contract remains unchanged.
### Coverage
- Public PR #40229, HEAD: `09a0659a81d2ac037535941cd9ed119f6600b503`;
comparison baseline: `e81c7c9aa5b49f6c1939eb62ec5c9f6c70e2491d`.
- The complete GitHub and local scopes match: two files, nine changed
blocks, and no additional local changes:
-
`infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionsContextEngine.java`
-
`infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/context/segment/select/projection/engine/ProjectionsContextEngineTest.java`
- Reviewed matching, derived output, ordinal ordering, alias allocation, and
tests against the PR objective, SQLUtils’ nullable contract, Projection’s
default nonnull contract, and repository standards. This does not establish
complete SELECT support for arbitrary null-text ASTs.
- Completed all three discovery lenses and convergence checks. Four
independent assessments across two rounds agreed after evidence reconciliation,
with no unresolved issues or evidence gaps.
- Local command: `./mvnw -B -pl infra/binder/core spotless:check
checkstyle:check -Pcheck -T1C`, with exact-file filters for both files and
resource scanning disabled, exited **0**. A same-scope `checkstyle:check`
recheck also exited **0**. Both tools covered the production and test files;
cached source and configuration identities were verified.
- This conclusion covers code correctness only; CI was not reviewed.
--
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]
