Skip to content

backport of 8105 - #8171

Merged
ibrahim halatci (ihalatci) merged 2 commits into
release-13.1from
release-13.1-ihalatci
Sep 3, 2025
Merged

backport of 8105 #8171
ibrahim halatci (ihalatci) merged 2 commits into
release-13.1from
release-13.1-ihalatci

Conversation

@ihalatci

Copy link
Copy Markdown
Contributor

see PR#8106 for details

… assert; correct parentQueryList handling) (#8106)

fixes #8105 

This change lets `FindReferencedTableColumn()` correctly resolve columns
through a CTE even when the expression comes from an outer query level
(`varlevelsup > 0`, `skipOuterVars = false`). Before, we hit an
`Assert(skipOuterVars)` in this path.

**Problem**

* Hitting a CTE after walking outer Vars triggered
`Assert(skipOuterVars)`.
* Cause: we modified `parentQueryList` in place and didn’t rebuild the
correct parent chain before recursing into the CTE, so the path was
considered unsafe.

**Fix**

* Remove the `Assert(skipOuterVars)` in the `RTE_CTE` branch.
* Find the CTE’s owning level via `ctelevelsup` and compute
`cteParentListIndex`.
* Rebuild a private parent list for recursion: `list_copy` →
`list_truncate` → `lappend(current query)`.
* Add a bounds check before indexing the CTE’s `targetList`.

**Why it works**


```diff
-parentQueryList = lappend(parentQueryList, query);
-FindReferencedTableColumn(targetEntry->expr, parentQueryList,
-                          cteQuery, column, rteContainingReferencedColumn,
-                          skipOuterVars);
+    /* hand a private, bounded parent list to the recursion */
+    List *newParent = list_copy(parentQueryList);
+    newParent = list_truncate(newParent, cteParentListIndex + 1);
+    newParent = lappend(newParent, query);
+
+    FindReferencedTableColumn(targetEntry->expr,
+                              newParent,
+                              cteQuery,
+                              column,
+                              rteContainingReferencedColumn,
+                              skipOuterVars);
+}


```
**Before:** We changed `parentQueryList` in place (`parentQueryList =
lappend(...)`) and didn’t trim it to the CTE’s owner level.

**After:** We copy the list, trim it to the CTE’s owner level, then
append the current query. This keeps the parent list accurate for the
current recursion and safe when following outer Vars.


**Example: Nested subquery referencing the CTE (two levels down)**

```
WITH c AS MATERIALIZED (SELECT user_id FROM raw_events_first)
SELECT 1
FROM raw_events_first t
WHERE EXISTS (
  SELECT 1
  FROM (SELECT user_id FROM c) c2
  WHERE c2.user_id = t.user_id
);
```

Levels:
Q0 = top SELECT
Q1 = EXISTS subquery
Q2 = inner (SELECT user_id FROM c)

When resolving c2.user_id inside Q2:

- parentQueryList is [Q0, Q1, Q2].
- `ctelevelsup`: 2


`cteParentListIndex = length(parentQueryList) - ctelevelsup - 1`

- Recurse into the CTE’s query with [Q0, Q2].


**Tests (added in `multi_insert_select`)**

* **T1:** Correlated subquery that references a CTE (one level down) 
Verifies that resolving through `RTE_CTE` after following an outer `Var`
succeeds, row count matches source table.
* **T2:** Nested subquery that references a CTE (two levels down) 
Exercises deeper recursion and confirms identical to T1.
* **T3:** Scalar subquery in a target list that reads from the outer CTE
Checks expected row count and that no NULLs are inserted.

These tests cover the cases that previously hit `Assert(skipOuterVars)`
and confirm CTE references while following outer Vars.
@ihalatci
ibrahim halatci (ihalatci) merged commit d94174d into release-13.1 Sep 3, 2025
274 checks passed
@ihalatci
ibrahim halatci (ihalatci) deleted the release-13.1-ihalatci branch September 3, 2025 11:04
@codecov

codecov Bot commented Sep 4, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.18%. Comparing base (0ce8fb6) to head (7eeee03).
⚠️ Report is 2 commits behind head on release-13.1.

Additional details and impacted files
@@               Coverage Diff                @@
##           release-13.1    #8171      +/-   ##
================================================
- Coverage         89.20%   89.18%   -0.03%     
================================================
  Files               284      284              
  Lines             61519    61521       +2     
  Branches           7698     7698              
================================================
- Hits              54880    54866      -14     
- Misses             4435     4450      +15     
- Partials           2204     2205       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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