Skip to content

Fix ORCA permission checking crash for ROLLUP + window function queries#1643

Draft
lss602726449 wants to merge 8 commits intoapache:cbdb-postgres-mergefrom
lss602726449:fix_orca_perminfo
Draft

Fix ORCA permission checking crash for ROLLUP + window function queries#1643
lss602726449 wants to merge 8 commits intoapache:cbdb-postgres-mergefrom
lss602726449:fix_orca_perminfo

Conversation

@lss602726449
Copy link
Copy Markdown
Contributor

transformGroupedWindows() incorrectly created an RTEPermissionInfo with relid=0 for a subquery RTE. Per PG16 commit a61b1f7, only RTE_RELATION entries need RTEPermissionInfo. This caused Assert(OidIsValid(perminfo->relid)) failure in ExecCheckPermissions when ORCA translated the query.

Also fix subquery not inheriting rteperminfos from the original query, and correct AddPerfmInfo/Perfission typos.

Fixes #ISSUE_Number

What does this PR do?

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

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@lss602726449 lss602726449 marked this pull request as draft March 27, 2026 07:52
@lss602726449 lss602726449 force-pushed the fix_orca_perminfo branch 2 times, most recently from 4a21dfc to f2c27eb Compare March 30, 2026 09:00
liushengsong added 4 commits March 30, 2026 18:27
1. orca.c: transformGroupedWindows() incorrectly created an RTEPermissionInfo
   with relid=0 for a subquery RTE. Per PG16 commit a61b1f7, only
   RTE_RELATION entries need RTEPermissionInfo. This caused
   Assert(OidIsValid(perminfo->relid)) failure when ORCA translated
   ROLLUP + window function queries.

2. matview.c: replace_rte_with_delta() converted RTE_RELATION to
   RTE_SUBQUERY but forgot to clear perminfoindex, causing
   Assert(rte->rtekind == RTE_RELATION || ...) failure during IVM
   incremental maintenance with ORCA enabled.

Also fix subquery not inheriting rteperminfos, and correct
AddPerfmInfo/Perfission typos.
BuildForeignScan() calls build_simple_rel() which looks up
RTEPermissionInfo via root->parse->rteperminfos using the RTE's
perminfoindex. When ORCA translates DXL back to a PlannedStmt, it
creates new RTEs with its own perminfoindex numbering, but passes
m_orig_query (the rewritten query) to BuildForeignScan. After the
rewriter expands external table ON SELECT rules into subqueries, the
outer query's rteperminfos shrinks, causing a mismatch: e.g.
perminfoindex=2 but rteperminfos has only 1 entry.

Fix by temporarily swapping m_orig_query->rteperminfos with ORCA's
own perminfos list around CreateForeignScan calls, so the indices
are consistent. This covers both static and dynamic foreign scans.
@reshke
Copy link
Copy Markdown
Contributor

reshke commented Mar 30, 2026

I am +1 on 9cba113

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants