You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by "gregfelice (via GitHub)" <gi...@apache.org> on 2026/07/03 11:08:39 UTC
[PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
gregfelice opened a new pull request, #2458:
URL: https://github.com/apache/age/pull/2458
## Problem
Fixes #2450. A `GENERATED ALWAYS ... STORED` column on a label table caused a backend segfault (signal 11) on Cypher `CREATE` / `MERGE` / `SET`:
```sql
SELECT create_graph('age_bug');
SELECT create_vlabel('age_bug', 'Product');
ALTER TABLE age_bug."Product"
ADD COLUMN category varchar(25)
GENERATED ALWAYS AS (agtype_access_operator(properties, '"category"')) STORED;
-- crashes the backend:
SELECT * FROM cypher('age_bug', $$ CREATE (p:Product {category:'disk'}) RETURN p $$) AS (p agtype);
```
## Root cause
AGE builds the insert/update tuple slot from the **full** label-table descriptor (`table_slot_create`), but only populates the columns it manages — `id`/`properties` for vertices, `id`/`start_id`/`end_id`/`properties` for edges. Any user-added column is left as an uninitialized slot entry. `ExecStoreVirtualTuple` then marks all attributes valid, so `heap_form_tuple()` → `heap_compute_data_size()` reads the uninitialized `tts_isnull[]`/`tts_values[]` and segfaults dereferencing garbage as a varlena.
```
#0 heap_compute_data_size
#1 heap_form_tuple
#3 ExecFetchSlotHeapTuple
#4 insert_entity_tuple_cid cypher_utils.c
#6 create_vertex cypher_create.c
```
The SET path crashes identically via `update_entity_tuple` (`cypher_set.c`). It affects both stored generated columns and plain `ADD COLUMN`s.
## Fix
1. **`clear_entity_slot()`** clears the slot and marks every attribute NULL before AGE fills in its own columns, so columns AGE doesn't manage default to NULL instead of stale slot memory. Used at all six slot-build sites: `create_vertex`, `create_edge`, `merge_vertex`, `merge_edge`, and the `populate_vertex_tts` / `populate_edge_tts` helpers (SET).
2. **`ExecComputeStoredGenerated()`** is called in `insert_entity_tuple_cid` (CREATE/MERGE, `CMD_INSERT`) and `update_entity_tuple` (SET, `CMD_UPDATE`) before the heap tuple is materialized when the relation has stored generated columns, so those columns are computed on insert and recomputed on update per PostgreSQL semantics.
## Testing
New `generated_columns` regression test covering stored generated columns on vertex **and** edge labels across CREATE / MERGE / SET (values verified computed and recomputed), plus a plain user column that must not crash. Passes under `pg_regress`.
Verified against the reporter's exact repro on PG18: no crash, and `category` correctly computes (`disk` on CREATE) and recomputes (`ssd` after SET).
## Note
On `SET`, AGE rebuilds the row from `id`/`properties`, so a **generated** column recomputes correctly but a **plain** (non-generated) column is reset to NULL. Preserving plain-column values across updates would be a larger change and is out of scope here; the supported pattern is a `GENERATED ... STORED` column derived from `properties`.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4876466560
> Thanks for the careful review and the approval, @MuhammadTahaNaveed — the tableoid catch in particular was a real gap. All feedback is addressed, CI is green, and the follow-up perf item is tracked in #2459.
>
> Since this is my PR I won't self-merge — feel free to land it whenever you have a moment, or I can leave it for another committer. Thanks again!
>
@gregfelice Its already merged :)
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "gregfelice (via GitHub)" <gi...@apache.org>.
gregfelice commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4876444843
Thanks for the careful review and the approval, @MuhammadTahaNaveed — the tableoid catch in particular was a real gap. All feedback is addressed, CI is green, and the follow-up perf item is tracked in #2459.
Since this is my PR I won't self-merge — feel free to land it whenever you have a moment, or I can leave it for another committer. Thanks again!
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "gregfelice (via GitHub)" <gi...@apache.org>.
gregfelice commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4876148133
Thanks for the thorough review, @MuhammadTahaNaveed — genuinely useful. Addressed in the latest push:
**1. [P1] tableoid generated columns** — Fixed. Good catch. The slot now carries `tts_tableOid = RelationGetRelid(...)` before `ExecComputeStoredGenerated()` in both the insert (`cypher_utils.c`) and update (`cypher_set.c`) paths, matching PostgreSQL's own ExecInsert/ExecUpdate. Added a regression case with `tbl oid GENERATED ALWAYS AS (tableoid) STORED` that asserts the stored value equals the label table's real OID on both CREATE and SET.
**2. [P1] PROJECT.yaml** — Removed. That was a stray commit that rode in from a local branch base; the branch has been rebased onto current master so the PR now contains only the fix.
**3. [P3] Rebase** — Done, the branch is rebased onto current master and CI is green against the current tree.
**B. [P3] MERGE clears slot before the should_insert check** — Fixed. `clear_entity_slot()` is now gated on `should_insert` in both `merge_vertex` and `merge_edge`, so the matched path skips the null-init. Added a matched-MERGE regression case to cover that path.
**A. [perf] SET builds a ResultRelInfo per row** — Agreed this is worth doing, but I'd like to keep it out of this bugfix. The per-row `ResultRelInfo` is pre-existing structure, and the generated-column recompute is guarded by `has_generated_stored`, so only labels that actually have a stored generated column pay the cost — no regression for the common case. Caching `ResultRelInfo`/`ri_GeneratedExprsU` per label across `apply_update_list` is a cleaner standalone change (alongside the existing relation/index/RLS caching); filed as #2459 so it doesn't get lost.
**4. [P3] Trailing whitespace in the .out** — Left as-is intentionally. pg_regress does an exact byte comparison against psql's output, which emits trailing spaces on table rows; every expected file in `regress/expected/` has them (e.g. `cypher_set.out`). Stripping them would make the expected file no longer match psql's actual output and break the test, and there's no `diff --check`/whitespace gate in CI. So this one I have to decline.
Thanks again — the tableoid case in particular was a real gap.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed merged PR #2458:
URL: https://github.com/apache/age/pull/2458
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "gregfelice (via GitHub)" <gi...@apache.org>.
gregfelice commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4875544656
@jrgemignani ready for review when you have a cycle. This fixes the #2450 segfault where a `GENERATED ALWAYS ... STORED` (or any extra) column on a label table crashed the backend on CREATE/MERGE/SET — root cause and the two-part fix (null-init the entity slot + `ExecComputeStoredGenerated`) are in the description. New `generated_columns` regression test covers vertex + edge across all three write paths, and CI is green.
One design note called out in the PR: on SET, AGE rebuilds the row from `id`/`properties`, so a *generated* column recomputes correctly but a *plain* column resets to NULL — I scoped plain-column preservation out. Happy to adjust if you'd rather handle that here.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4875865687
@gregfelice
**GPT 5.5 xhigh review**:
Found one performance issue worth commenting on:
Performance note: `SET` now calls `ExecComputeStoredGenerated(...)` for stored generated columns, but this path creates a fresh `ResultRelInfo` for each updated entity at `src/backend/executor/cypher_set.c:607`.
`ExecComputeStoredGenerated(...)` lazily initializes `ri_GeneratedExprsU`; PostgreSQL stores those expression states in the query context. Because AGE’s `SET` path uses a new `ResultRelInfo` per row/update item, multi-row `SET` on a label with stored generated columns will repeatedly rebuild generated-column expression state and retain that memory until query end.
Relevant spots:
- `src/backend/executor/cypher_set.c:607` creates a new `ResultRelInfo`
- `src/backend/executor/cypher_set.c:143` / `:146` calls `ExecComputeStoredGenerated(...)`
- `src/backend/executor/cypher_set.c:840` / `:841` closes indexes/relation, but the generated expression state has already been allocated in the executor query context
This should probably cache/reuse `ResultRelInfo` per label/relation for the duration of `apply_update_list`, similar to the existing relation/index/RLS caching pattern. That would let
`ri_GeneratedExprsU` initialize once per relation instead of once per updated row.
**Secondary, lower-priority nit:**
Minor: `MERGE` now calls `clear_entity_slot(...)` before checking whether the matched path will actually insert.
- `src/backend/executor/cypher_merge.c:1202`
- `src/backend/executor/cypher_merge.c:1529`
On a matched `MERGE` where `should_insert == false`, the code later skips `insert_entity_tuple(...)`, so the new full-null-bitmap `memset` is avoidable work. Usually tiny for AGE’s base
label tables, but it scales with extra user-added columns. Consider moving the slot clear closer to the insert-only path if practical.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2458:
URL: https://github.com/apache/age/pull/2458#discussion_r3519520661
##########
PROJECT.yaml:
##########
@@ -0,0 +1,25 @@
+project:
+ id: apache-age
+ name: Apache Age
+ shortdesc: <br>
+ description: <p align="center"> <img src="https://age.apache.org/age-manual/master/_static/logo.png"
+ width="30%" height="30%"> </p>
+ category: ip.foss
+ lifecycle: garage
+ layer: L3
+ technical:
+ languages: []
+ implements:
+ - Standard SDLC (Makefile)
+ os:
+ - Linux/Debian
+ license: Proprietary
Review Comment:
`PROJECT.yaml` declares the project license as "Proprietary", which contradicts the repository's Apache 2.0 `LICENSE` file and could cause downstream tooling to misclassify the project.
##########
PROJECT.yaml:
##########
@@ -0,0 +1,25 @@
+project:
+ id: apache-age
+ name: Apache Age
+ shortdesc: <br>
+ description: <p align="center"> <img src="https://age.apache.org/age-manual/master/_static/logo.png"
+ width="30%" height="30%"> </p>
+ category: ip.foss
+ lifecycle: garage
+ layer: L3
+ technical:
+ languages: []
+ implements:
+ - Standard SDLC (Makefile)
+ os:
+ - Linux/Debian
+ license: Proprietary
+ release:
+ revision: 0.1.0
+ date: '2026-05-31'
+ governance:
+ maintainer: operator
+ owner: Rizlabs
+ links:
+ docs: ''
+ repo: https://git.rizlabs.com/gregf/apache-age
Review Comment:
`PROJECT.yaml` points the repository link to a non-Apache host (git.rizlabs.com), which is inconsistent with the rest of this repo (multiple files reference https://github.com/apache/age). This looks unrelated to the segfault fix and should be corrected or removed.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix segfault on CREATE/MERGE/SET with generated or extra label columns (#2450) [age]
Posted by "MuhammadTahaNaveed (via GitHub)" <gi...@apache.org>.
MuhammadTahaNaveed commented on PR #2458:
URL: https://github.com/apache/age/pull/2458#issuecomment-4875747849
@gregfelice
**GPT 5.5 xhigh review:**
**Findings**
1. **[P1] Stored generated columns using `tableoid` are computed incorrectly**
- `src/backend/executor/cypher_utils.c:354`
- `src/backend/executor/cypher_set.c:143`
The new `ExecComputeStoredGenerated(...)` calls compute stored generated columns, but the tuple slot does not appear to have `tts_tableOid` set first. PostgreSQL’s own executor sets
`slot->tts_tableOid = RelationGetRelid(...)` before computing stored generated columns.
Repro: adding this to a vertex label produces `InvalidOid` instead of the label table OID:
```sql
ALTER TABLE tableoid_case."T"
ADD COLUMN tbl oid GENERATED ALWAYS AS (tableoid) STORED;
Then CREATE and SET through Cypher both leave tbl as - / invalid. The fix should set elemTupleSlot->tts_tableOid before ExecComputeStoredGenerated(...) in both insert/create and update/set
paths, and add a regression case for tableoid.
2. **[P1] PROJECT.yaml adds incorrect Apache project metadata**
- PROJECT.yaml:16 says license: Proprietary
- PROJECT.yaml:22 says owner: Rizlabs
- PROJECT.yaml:25 points at a Rizlabs repo
**This file is completely unrelated to the fix and should be removed immediately.**
3. **[P2] Branch needs a rebase onto current master**
The PR is based on merge base 12e2a31c, while current apache/master is d84b4578. It is missing several upstream commits, so the branch-local regression suite is stale. Please rebase so
CI runs against the current tree.
4. **[P3] git diff --check fails on trailing whitespace**
The new regress/expected/generated_columns.out has multiple trailing-whitespace lines and a blank line at EOF. Please clean that up so whitespace checks pass.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org