aviralgarg05 opened a new pull request, #1715: URL: https://github.com/apache/cloudberry/pull/1715
Fixes #1660 ### What does this PR do? This PR fixes an ORCA planning issue where unsafe target list functions, such as the PL/pgSQL function used in the reported reproduction, could be planned in a way that made them execute on the coordinator instead of the expected worker path. To address that, the change teaches ORCA to fall back to the Postgres planner when the target list contains an unsafe SIRV function implemented in a non-`internal` and non-`c` language. This keeps the fallback narrowly scoped so built-in volatile functions that ORCA already handles correctly are unaffected, while PL/pgSQL and similar procedural functions no longer get placed above Motion nodes in unsafe ways. ### Type of Change - [x] Bug fix (non-breaking change) - [ ] New feature (non-breaking change) - [ ] Breaking change (fix or feature with breaking changes) - [ ] Documentation update ### Breaking Changes None. ### Test Plan - [ ] Unit tests added/updated - [x] Integration tests added/updated - [ ] Passed `make installcheck` - [ ] Passed `make -C src/test installcheck-cbdb-parallel` How this was tested: - Synced to the latest `upstream/main` before making changes. - Built a fresh local Cloudberry sandbox image from this branch using the repo’s Docker sandbox workflow. - Started a patched cluster from that image and reproduced the reported `test_conv(...)` / UTF-8 verification scenario with `optimizer=on`. - Confirmed the query no longer fails under ORCA mode and instead falls back to the Postgres planner for this unsafe target list function shape. - Re-ran the same reproduction with `optimizer=off` to confirm the expected control behavior matched. - Re-ran the `optimizer=on` reproduction a second time to verify the result was stable. - Verified that ordinary queries still use GPORCA, so the fallback remains narrowly targeted. - Ran `git diff --check` to confirm there were no whitespace or formatting issues. ### Impact **Performance:** The impact should be minimal and limited to queries whose target lists contain unsafe SIRV functions in non-`internal` and non-`c` languages. Those queries will now fall back to the Postgres planner instead of being handled by ORCA. Normal ORCA-planned queries are unaffected. **User-facing changes:** Users will no longer hit the incorrect coordinator execution path for the function shape described in #1660. Queries of that form now execute successfully through a safe planner fallback. **Dependencies:** No new dependencies were added to the codebase. ### Checklist - [x] Followed [contribution guide](https://cloudberry.apache.org/contribute/code) - [ ] Added/updated documentation - [x] Reviewed code for security implications - [x] Requested review from [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers) ### Additional Context This fix intentionally keeps the fallback as narrow as possible. An earlier broad approach would have caused ORCA to fall back for built-in volatile functions such as `random()`, which existing optimizer coverage already handles correctly. The final change avoids that by only treating non-`internal` and non-`c` target list SIRV functions as unsafe for ORCA in this context. Files changed: - `src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp` - `src/include/gpopt/translate/CTranslatorQueryToDXL.h` -- 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]
