Skip to content

Conversation

@jrgemignani
Copy link
Contributor

Merge in the following PRs to PG18 to align it with the master branch

1702ae0 Add RLS support and fix permission checks (#2309)
b29ca5e Replace libcsv with pg COPY for csv loading (#2310)
56a92d8 Fix Issue 1884: Ambiguous column reference (#2306)
8bdeec5 Upgrade Jest to v29 for node: protocol compatibility (#2307)
b9d0982 Optimize vertex/edge field access with direct array indexing (#2302)
c979380 feat: Add 32-bit platform support for graphid type (#2286)
a1f472d Fix and improve index.sql addendum (#2301)
7beb653 Fix and improve index.sql regression test coverage (#2300)
2e8f7ab Fix Issue 2289: handle empty list in IN expression (#2294)
4eeceab Revise README for Python driver updates (#2298)
dd6deb7 Makefile: fix race condition on cypher_gram_def.h (#2273)

tureba and others added 11 commits January 20, 2026 12:49
The file cypher_gram.c generates cypher_gram_def.h, which is directly
necessary for cypher_parser.o and cypher_keywords.o and their respective
.bc files.

But that direct dependency is not reflected in the Makefile, which only
had the indirect dependency of .o on .c. So on high parallel builds, the
.h may not have been generated by bison yet.

Additionally, the .bc files should have the same dependencies as the .o
files, but those are lacking.

Here is an example output where the .bc file fails to build, as it was
running concurrently with the bison instance that was about to finalize
cypher_gram_def.h:

  In file included from src/backend/parser/cypher_parser.c:24:
  clang-17 -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -O2  -I.//src/include -I.//src/include/parser -I. -I./ -I/usr/pgsql-17/include/server -I/usr/pgsql-17/include/internal -D_GNU_SOURCE  -I/usr/include -I/usr/include/libxml2  -flto=thin -emit-llvm -c -o src/backend/parser/cypher_parser.bc src/backend/parser/cypher_parser.c
  .//src/include/parser/cypher_gram.h:65:10: fatal error: 'parser/cypher_gram_def.h' file not found
     65 | #include "parser/cypher_gram_def.h"
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.
  make: *** [/usr/pgsql-17/lib/pgxs/src/makefiles/../../src/Makefile.global:1085: src/backend/parser/cypher_parser.bc] Error 1
  make: *** Waiting for unfinished jobs....
  gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -O2 -g -fmessage-length=0 -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -fPIC -fvisibility=hidden -I.//src/include -I.//src/include/parser -I. -I./ -I/usr/pgsql-17/include/server -I/usr/pgsql-17/include/internal -D_GNU_SOURCE  -I/usr/include -I/usr/include/libxml2   -c -o src/backend/catalog/ag_label.o src/backend/catalog/ag_label.c
  /usr/bin/bison -Wno-deprecated  --defines=src/include/parser/cypher_gram_def.h -o src/backend/parser/cypher_gram.c src/backend/parser/cypher_gram.y

Previously, cypher_parser.o was missing the dependency, so it could
start before cypher_gram_def.h was available:

 Considering target file 'src/backend/parser/cypher_parser.o'.
  File 'src/backend/parser/cypher_parser.o' does not exist.
  Considering target file 'src/backend/parser/cypher_parser.c'.
  File 'src/backend/parser/cypher_parser.c' was considered already.
  Considering target file 'src/backend/parser/cypher_gram.c'.
  File 'src/backend/parser/cypher_gram.c' was considered already.
 Finished prerequisites of target file 'src/backend/parser/cypher_parser.o'.
 Must remake target 'src/backend/parser/cypher_parser.o'.

As well as cypher_parser.bc, missing the dependency on
cypher_gram_def.h:

 Considering target file 'src/backend/parser/cypher_parser.bc'.
  File 'src/backend/parser/cypher_parser.bc' does not exist.
  Considering target file 'src/backend/parser/cypher_parser.c'.
  File 'src/backend/parser/cypher_parser.c' was considered already.
 Finished prerequisites of target file 'src/backend/parser/cypher_parser.bc'.
 Must remake target 'src/backend/parser/cypher_parser.bc'.

Now cypher_parser.o correctly depends on cypher_gram_def.h:

 Considering target file 'src/backend/parser/cypher_parser.o'.
  File 'src/backend/parser/cypher_parser.o' does not exist.
  Considering target file 'src/backend/parser/cypher_parser.c'.
  File 'src/backend/parser/cypher_parser.c' was considered already.
  Considering target file 'src/backend/parser/cypher_gram.c'.
  File 'src/backend/parser/cypher_gram.c' was considered already.
  Considering target file 'src/include/parser/cypher_gram_def.h'.
  File 'src/include/parser/cypher_gram_def.h' was considered already.
 Finished prerequisites of target file 'src/backend/parser/cypher_parser.o'.
 Must remake target 'src/backend/parser/cypher_parser.o'.

And cypher_parser.bc correctly depends on cypher_gram_def.h as well:

 Considering target file 'src/backend/parser/cypher_parser.bc'.
  File 'src/backend/parser/cypher_parser.bc' does not exist.
  Considering target file 'src/backend/parser/cypher_parser.c'.
  File 'src/backend/parser/cypher_parser.c' was considered already.
  Considering target file 'src/backend/parser/cypher_gram.c'.
  File 'src/backend/parser/cypher_gram.c' was considered already.
  Considering target file 'src/include/parser/cypher_gram_def.h'.
  File 'src/include/parser/cypher_gram_def.h' was considered already.
 Finished prerequisites of target file 'src/backend/parser/cypher_parser.bc'.
 Must remake target 'src/backend/parser/cypher_parser.bc'.
Updated README to from psycopg2 to psycopg3 (psycopg)
NOTE: This PR was created with AI tools and a human.

When evaluating 'x IN []' with an empty list, the transform_AEXPR_IN
function would return NULL because no expressions were processed.
This caused a 'cache lookup failed for type 0' error downstream.

This fix adds an early check for the empty list case:
- 'x IN []' returns false (nothing can be in an empty list)

Additional NOTE: Cypher does not have 'NOT IN' syntax. To check if
a value is NOT in a list, use 'NOT (x IN list)'. The NOT operator
will invert the false from an empty list to true as expected.

The fix returns a boolean constant directly, avoiding the NULL result
that caused the type lookup failure.

Added regression tests.

modified:   regress/expected/expr.out
modified:   regress/sql/expr.sql
modified:   src/backend/parser/cypher_expr.c
NOTE: This PR was created with AI tools and a human.

- Remove unused copy command (leftover from deleted agload_test_graph test)
- Replace broken Section 4 that referenced non-existent graph with
  comprehensive WHERE clause tests covering string, int, bool, and float
  properties with AND/OR/NOT operators
- Add EXPLAIN tests to verify index usage:
  - Section 3: Validate GIN indices (load_city_gin_idx, load_country_gin_idx)
    show Bitmap Index Scan for property matching
  - Section 4: Validate all expression indices (city_country_code_idx,
    city_id_idx, city_west_coast_idx, country_life_exp_idx) show Index Scan
    for WHERE clause filtering

All indices now have EXPLAIN verification confirming they are used as expected.

modified:   regress/expected/index.out
modified:   regress/sql/index.sql
NOTE: This PR was created with the help of AI tools and a human.

Added additional requested regression tests -

 *EXPLAIN for pattern with WHERE clause

 *EXPLAIN for pattern with filters on both country and city

modified:   regress/expected/index.out
modified:   regress/sql/index.sql
* feat: Add 32-bit platform support for graphid type

This enables AGE to work on 32-bit platforms including WebAssembly (WASM).

Problem:
- graphid is int64 (8 bytes) with PASSEDBYVALUE
- On 32-bit systems, Datum is only 4 bytes
- PostgreSQL rejects pass-by-value types larger than Datum

Solution:
- Makefile-only change (no C code modifications)
- When SIZEOF_DATUM=4 is passed to make, strip PASSEDBYVALUE from the generated SQL
- If not specified, normal 64-bit behavior is preserved (PASSEDBYVALUE kept)

This change is backward compatible:
- 64-bit systems continue using pass-by-value
- 32-bit systems now work with pass-by-reference

Motivation: PGlite (PostgreSQL compiled to WebAssembly) uses 32-bit
pointers and requires this patch to run AGE.

Tested on:
- 64-bit Linux (regression tests pass)
- 32-bit WebAssembly via PGlite (all operations work)

Co-authored-by: abbuehlj <jean-paul.abbuehl@roche.com>
…2302)

NOTE: This PR was created using AI tools and a human.

Leverage deterministic key ordering from uniqueify_agtype_object() to
access vertex/edge fields in O(1) instead of O(log n) binary search.

Fields are sorted by key length, giving fixed positions:
- Vertex: id(0), label(1), properties(2)
- Edge: id(0), label(1), end_id(2), start_id(3), properties(4)

Changes:
- Add field index constants and accessor macros to agtype.h
- Update age_id(), age_start_id(), age_end_id(), age_label(),
  age_properties() to use direct field access
- Add fill_agtype_value_no_copy() for read-only scalar extraction
  without memory allocation
- Add compare_agtype_scalar_containers() fast path for scalar comparison
- Update hash_agtype_value(), equals_agtype_scalar_value(), and
  compare_agtype_scalar_values() to use direct field access macros
- Add fast path in get_one_agtype_from_variadic_args() bypassing
  extract_variadic_args() for single argument case
- Add comprehensive regression test (30 tests)

Performance impact: Improves ORDER BY, hash joins, aggregations, and
Cypher functions (id, start_id, end_id, label, properties) on vertices
and edges.

All previous regression tests were not impacted.
Additional regression test added to enhance coverage.

modified:   Makefile
new file:   regress/expected/direct_field_access.out
new file:   regress/sql/direct_field_access.sql
modified:   src/backend/utils/adt/agtype.c
modified:   src/backend/utils/adt/agtype_util.c
modified:   src/include/utils/agtype.h
Note: This PR was created with AI tools and a human.

The pg-connection-string module (dependency of pg) now uses the node:
protocol prefix for built-in modules (e.g., require('node:process')).
Jest 26 does not support this syntax, causing test failures.

Changes:
- Upgrade jest from ^26.6.3 to ^29.7.0
- Upgrade ts-jest from ^26.5.1 to ^29.4.6
- Upgrade @types/jest from ^26.0.20 to ^29.5.14
- Update typescript to ^4.9.5

This also resolves 19 npm audit vulnerabilities (17 moderate, 2 high)
that existed in the older Jest 26 dependency tree.

modified:   drivers/nodejs/package.json
Fix Issue 1884: Ambiguous column reference and invalid AGT header
errors.

Note: This PR was created with AI tools and a human, or 2.

This commit addresses two related bugs that occur when using SET to store
graph elements (vertices, edges, paths) as property values:

Issue 1884 - "column reference is ambiguous" error:
When a Cypher query uses the same variable in both the SET expression RHS
and the RETURN clause (e.g., SET n.prop = n RETURN n), PostgreSQL would
report "column reference is ambiguous" because the variable appeared in
multiple subqueries without proper qualification.

Solution: The fix for this issue was already in place through the target
entry naming scheme that qualifies column references.

"Invalid AGT header value" offset error:
When deserializing nested VERTEX, EDGE, or PATH values stored in properties,
the system would fail with errors like "Invalid AGT header value: 0x00000041".
This occurred because ag_serialize_extended_type() did not include alignment
padding (padlen) in the agtentry length calculation for these types, while
fill_agtype_value() uses INTALIGN() when reading, causing offset mismatch.

Solution: Modified ag_serialize_extended_type() in agtype_ext.c to include
padlen in the agtentry length for VERTEX, EDGE, and PATH cases, matching
the existing pattern used for INTEGER, FLOAT, and NUMERIC types:

    *agtentry = AGTENTRY_IS_AGTYPE | (padlen + (AGTENTRY_OFFLENMASK & ...));

This ensures the serialized length accounts for alignment padding, allowing
correct deserialization of nested graph elements.

Appropriate regression tests were added to verify the fixes.

Co-authored by: Zainab Saad <105385638+Zainab-Saad@users.noreply.github.com>

modified:   regress/expected/cypher_set.out
modified:   regress/sql/cypher_set.sql
modified:   src/backend/parser/cypher_clause.c
modified:   src/backend/utils/adt/agtype_ext.c
- Commit also adds permission checks
- Resolves a critical memory spike issue on loading large file
- Use pg's COPY infrastructure (BeginCopyFrom, NextCopyFromRawFields)
  for 64KB buffered CSV parsing instead of libcsv
- Add byte based flush threshold (64KB) matching COPY behavior for memory safety
- Use heap_multi_insert with BulkInsertState for optimized batch inserts
- Add per batch memory context to prevent memory growth during large loads
- Remove libcsv dependency (libcsv.c, csv.h)
- Improves loading performance by 15-20%
- No previous regression tests were impacted
- Added regression tests for permissions/rls
Assisted-by AI

Resolved conflict with ExecInitRangeTable
- Previously, age only set ACL_SELECT and ACL_INSERT in RTEPermissionInfo,
  bypassing pg's privilege checking for DELETE and UPDATE operations.
- Additionally, RLS policies were not enforced because AGE uses CMD_SELECT
  for all Cypher queries, causing the rewriter to skip RLS policy application.

Permission fixes:
- Add ACL_DELETE permission flag for DELETE clause operations
- Add ACL_UPDATE permission flag for SET/REMOVE clause operations
- Recursively search RTEs including subqueries for permission info

RLS support:
- Implemented at executor level because age transforms all cypher
  queries to CMD_SELECT, so pg's rewriter never adds RLS
  policies for INSERT/UPDATE/DELETE operations. There isnt an
  appropriate rewriter hook to modify this behavior, so we do it
  in executor instead.
- Add setup_wcos() to apply WITH CHECK policies at execution time
  for CREATE, SET, and MERGE operations
- Add setup_security_quals() and check_security_quals() to apply
  USING policies for UPDATE and DELETE operations
- USING policies silently filter rows (matching pg behavior)
- WITH CHECK policies raise errors on violation
- DETACH DELETE raises error if edge RLS blocks deletion to prevent
  dangling edges
- Add permission checks and rls in startnode/endnode functions
- Add regression tests

Assisted-by AI

Resolved Conflicts:
	src/backend/executor/cypher_create.c
	src/backend/executor/cypher_delete.c
	src/backend/executor/cypher_merge.c
	src/backend/executor/cypher_set.c
	src/backend/executor/cypher_utils.c
@github-actions github-actions bot added the override-stale To keep issues/PRs untouched from stale action label Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

override-stale To keep issues/PRs untouched from stale action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants