PG18 to Apache AGE 1.8.0#2466
Merged
Merged
Conversation
…raph (apache#2248) Fixed issue 2245 - Creating more than 41 vlabels causes drop_grapth to fail with "label (relation) cache corrupted" and crashing out on the following command. This was due to corruption of the label_relation_cache during the HASH_DELETE process. As the issue was with a cache flush routine, it was necessary to fix them all. Here is the list of the flush functions that were fixed - static void flush_graph_name_cache(void) static void flush_graph_namespace_cache(void) static void flush_label_name_graph_cache(void) static void flush_label_graph_oid_cache(void) static void flush_label_relation_cache(void) static void flush_label_seq_name_graph_cache(void) Added regression tests. modified: regress/expected/catalog.out modified: regress/sql/catalog.sql modified: src/backend/utils/cache/ag_cache.c
- Whenever a label will be created, indices on id columns will be created by default. In case of vertex, a unique index on id column will be created, which will also serve as a unique constraint. In case of edge, a non-unique index on start_id and end_id columns will be created. - This change is expected to improve the performance of queries that involve joins. From some performance tests, it was observed that the performance of queries improved alot. - Loader was updated to insert tuples in indices as well. This has caused to slow the loader down a bit, but it was necessary. - A bug related to command ids in cypher_delete executor was also fixed.
…ache#2259) Fixed issue 2256: A segmentation fault occurs when calling the coalesce function in PostgreSQL version 17. This likely predates 17 and includes other similar types of "functions". See issues 1124 (PR 1125) and 1303 (PR 1317) for more details. This issue is due to coalesce() being processed differently from other functions. Additionally, greatest() was found to exhibit the same behavior. They were added to the list of types to ignore during the cypher analyze phase. A few others were added: CaseExpr, XmlExpr, ArrayExpr, & RowExpr. Although, I wasn't able to find cases where these caused crashes. Added regression tests. modified: regress/expected/cypher.out modified: regress/sql/cypher.sql modified: src/backend/parser/cypher_analyze.c
Adjusted the following type of error message. It was mentioned in
issue 2263 as being incorrect, which it isn't. However, it did need
some clarification added -
ERROR: could not find rte for <column name>
Added a HINT for additional clarity -
HINT: variable <column name> does not exist within scope of usage
For example:
CREATE p0=(n0), (n1{k:EXISTS{WITH p0}}) RETURN 1
ERROR: could not find rte for p0
LINE 3: CREATE p0=(n0), (n1{k:EXISTS{WITH p0}})
^
HINT: variable p0 does not exist within scope of usage
Additionally, added pstate->p_expr_kind == EXPR_KIND_INSERT_TARGET to
transform_cypher_clause_as_subquery.
Updated existing regression tests.
Added regression tests from issue.
modified: regress/expected/cypher_call.out
modified: regress/expected/cypher_subquery.out
modified: regress/expected/cypher_union.out
modified: regress/expected/cypher_with.out
modified: regress/expected/expr.out
modified: regress/expected/list_comprehension.out
modified: regress/expected/scan.out
modified: src/backend/parser/cypher_clause.c
modified: src/backend/parser/cypher_expr.c
- Used postgres memory allocation functions instead of standard ones. - Wrapped main loop of csv loader in PG_TRY block for better error handling.
NOTE: This PR was partially created with AI tools and reviewed by a human.
ORDER BY clauses failed when referencing column aliases from RETURN:
MATCH (p:Person) RETURN p.age AS age ORDER BY age DESC
ERROR: could not find rte for age
Added SQL-99 compliant alias matching to find_target_list_entry() that
checks if ORDER BY identifier matches a target list alias before
attempting expression transformation. This enables standard SQL behavior
for sorting by aliased columns with DESC/DESCENDING/ASC/ASCENDING.
Updated regression tests.
Added regression tests.
modified: regress/expected/cypher_match.out
modified: regress/expected/expr.out
modified: regress/sql/expr.sql
modified: src/backend/parser/cypher_clause.c
Consolidated duplicate code, added helper functions, and reviewed
the grammar file for issues.
NOTE: I used an AI tool to review and cleanup the grammar file. I
have reviewed all of the work it did.
Improvements:
1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths
2. Consolidated EXPLAIN statement handling into make_explain_stmt helper
3. Extracted WITH clause validation into validate_return_item_aliases helper
4. Created make_default_return_node helper for subquery return-less logic
Benefits:
- Reduced code duplication by ~150 lines
- Improved maintainability with helper functions
- Eliminated manual string length calculations (error-prone)
All 29 existing regression tests pass
modified: src/backend/parser/cypher_gram.y
apache#2267) - Changed '\s' to r'\s'
- Add pyproject.toml with package configuration - Simplify setup.py to minimal backward-compatible wrapper. - Updated CI workflow and .gitignore. - Resolves warning about using setup.py directly.
This PR applies restrictions to the following age_load commands -
load_labels_from_file()
load_edges_from_file()
They are now tied to a specific root directory and are required to have a
specific file extension to eliminate any attempts to force them to access
any other files.
Nothing else has changed with the actual command formats or parameters,
only that they work out of the /tmp/age directory and only access files
with an extension of .csv.
Added regression tests and updated the location of the csv files for
those regression tests.
modified: regress/expected/age_load.out
modified: regress/sql/age_load.sql
modified: src/backend/utils/load/age_load.c
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
- 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
* Updated CI, Labeler, Docker, and branch security files for PG18 (apache#2246) Updated the CI and Docker files for the PG18 Updated the labeler and branch security files for PG18. Some of these only apply to the master branch but are updated for consistency. modified: .asf.yaml modified: .github/labeler.yml modified: .github/workflows/go-driver.yml modified: .github/workflows/installcheck.yaml modified: .github/workflows/jdbc-driver.yaml modified: .github/workflows/nodejs-driver.yaml modified: .github/workflows/python-driver.yaml modified: docker/Dockerfile modified: docker/Dockerfile.dev modified: drivers/docker-compose.yml * PG18 port for AGE (apache#2251) * [PG18 port][Set1] Fix header dependencies and use TupleDescAttr macro - Include executor/executor.h for PG18 header reorganization and use TupleDescAttr() accessor macro instead of direct attrs[] access. * [PG18 port][Set2] Adapt to expandRTE signature change and pg_noreturn - Add VarReturningType parameter to expandRTE() calls using VAR_RETURNING_DEFAULT. - Replace pg_attribute_noreturn() with pg_noreturn prefix specifier. * [PG18 port][Set3] Fix double ExecOpenIndices call for PG18 compatibility - PG18 enforces stricter assertions in ExecOpenIndices, requiring ri_IndexRelationDescs to be NULL when called. - In update_entity_tuple(), indices may already be opened by the caller (create_entity_result_rel_info), causing assertion failures. - Add a check to only open indices if not already open, and track ownership with a boolean flag to ensure we only close what we opened. - Found when regression tests failed with assertions, which this change resolves. * [PG18 port][Set4] Update regression test expected output for ordering PG18's implementation changes result in different row ordering for queries without explicit ORDER BY clauses. Update expected output files to reflect the new ordering while maintaining identical result content. * [PG18 port][Set5] Address review comments - coding standard fix Note: Assisted by GitHub Copilot Agent mode. * Fix DockerHub build warning messages (apache#2252) PR fixes build warning messages on DockerHub and on my local build. No regression tests needed. modified: src/include/nodes/ag_nodes.h modified: src/include/optimizer/cypher_createplan.h modified: src/include/optimizer/cypher_pathnode.h modified: tools/gen_keywordlist.pl --------- Co-authored-by: Krishnakumar R (KK) <65895020+kk-src@users.noreply.github.com>
- Added index creation for existing labels Assisted-by AI
Updated the following files to advance the Apache AGE version to 1.7.0 modified: Makefile modified: README.md modified: RELEASE renamed: age--1.6.0--y.y.y.sql -> age--1.6.0--1.7.0.sql new file: age--1.7.0--y.y.y.sql modified: age.control modified: docker/Dockerfile deleted: age--1.5.0--1.6.0.sql
Add pg_upgrade support functions for PostgreSQL for major version upgrades NOTE: This PR was created with AI tools and a human. The ag_graph.namespace column uses the regnamespace type, which pg_upgrade cannot handle in user tables. This commit adds four SQL functions to enable seamless PostgreSQL major version upgrades while preserving all graph data. New functions in ag_catalog: - age_prepare_pg_upgrade(): Converts namespace from regnamespace to oid, creates backup table with graph-to-namespace mappings (stores nspname directly to avoid quoting issues) - age_finish_pg_upgrade(): Remaps stale OIDs after upgrade, restores regnamespace type, invalidates AGE caches while preserving schema ownership - age_revert_pg_upgrade_changes(): Cancels preparation if upgrade is aborted - age_pg_upgrade_status(): Returns current upgrade readiness status Usage: 1. Before pg_upgrade: SELECT age_prepare_pg_upgrade(); 2. Run pg_upgrade as normal 3. After pg_upgrade: SELECT age_finish_pg_upgrade(); Key implementation details: - Uses transaction-level advisory locks (pg_advisory_xact_lock) for safety - Preserves original schema ownership during cache invalidation - Validates all backup rows are mapped before proceeding - Handles zero-graph edge case gracefully - Handles insufficient privileges gracefully with informative notices - Backup table deleted only after all steps succeed Files changed: - sql/age_pg_upgrade.sql: New file with function implementations - sql/sql_files: Added age_pg_upgrade entry - age--1.7.0--y.y.y.sql: Added functions for extension upgrades All regression tests pass.
Fix JDBC driver CI test failures: encoding, Testcontainers, and Docker compatibility. Note: This PR was created with the help of AI tools and a human. - Set JavaCompile encoding to UTF-8 to fix unicode test failures in CI environments that default to US-ASCII. - Add Testcontainers wait strategy (Wait.forLogMessage) to wait for PostgreSQL to be fully ready before connecting. - Use agensGraphContainer.getHost() instead of hardcoded localhost for Docker-in-Docker compatibility. - Add sslmode=disable to JDBC URL since PostgreSQL driver 42.6.0+ attempts SSL by default. - Remove silent exception swallowing around connection setup to fail fast with meaningful errors instead of NullPointerException. - Upgrade Testcontainers from 1.18.0 to 1.21.4 to fix Docker 29.x detection failure on ubuntu-24.04 runners (docker-java 3.3.x in 1.18.0 cannot negotiate with Docker Engine 29.1.5 API). modified: drivers/jdbc/lib/build.gradle.kts modified: drivers/jdbc/lib/src/test/java/org/apache/age/jdbc/BaseDockerizedTest.java
Note: This PR was created with AI tools and a human.
- Add parameterized query construction using psycopg.sql to prevent
SQL injection in all Cypher execution paths (age.py, networkx/lib.py)
- Replace all %-format and f-string SQL in networkx/lib.py with
sql.Identifier() for schema/table names and sql.Literal() for values
- Add validate_graph_name() with AGE-aligned VALID_GRAPH_NAME regex:
start with letter/underscore, allow dots and hyphens in middle positions,
end with letter/digit/underscore, min 3 chars, max 63 chars
- Add validate_identifier() with strict VALID_IDENTIFIER regex for labels,
column names, and SQL types (no dots or hyphens)
- Add validation calls to all networkx/lib.py entry points:
graph names validated on entry, labels validated before SQL construction
- Add _validate_column() to sanitize column specifications in buildCypher()
- Fix exception constructors (AgeNotSet, GraphNotFound, GraphAlreadyExists)
to always call super().__init__() with a meaningful default message so
that str(exception) never returns an empty string
- Add InvalidGraphName and InvalidIdentifier exception classes with
structured name/reason/context fields
- Fix builder.py: change erroneous 'return Exception(...)' to
'raise ValueError(...)' for unknown float expressions
- Fix copy-paste docstring in create_elabel() ('create_vlabels' -> 'create_elabels')
- Remove unused 'from psycopg.adapt import Loader' import in age.py
- Add design documentation in source explaining:
- VALID_GRAPH_NAME regex uses '*' (not '+') intentionally so that the
min-length check fires first with a clear error message
- buildCypher uses string concatenation (not sql.Identifier) because
column specs are pre-validated 'name type' pairs that don't map to
sql.Identifier(); graphName and cypherStmt are NOT embedded
- Update test_networkx.py GraphNotFound assertion to use assertIn()
instead of assertEqual() to match the improved exception messages
- Strip Windows carriage returns (^M) from 7 source files
- Fix requirements.txt: convert from UTF-16LE+BOM+CRLF to clean UTF-8+LF,
move --no-binary flag from requirements.txt to CI workflow pip command
- Upgrade actions/setup-python from v4 (deprecated) to v5 in CI workflow
- Add 46 security unit tests in test_security.py covering:
- Graph name validation (AGE naming rules, injection, edge cases)
- SQL identifier validation (labels, columns, types)
- Column spec sanitization
- buildCypher injection prevention
- Exception constructor correctness (str() never empty)
- Add test_security.py to CI pipeline (python-driver.yaml)
- pip-audit: 0 known vulnerabilities in all dependencies
modified: .github/workflows/python-driver.yaml
modified: drivers/python/age/VERSION.py
modified: drivers/python/age/__init__.py
modified: drivers/python/age/age.py
modified: drivers/python/age/builder.py
modified: drivers/python/age/exceptions.py
modified: drivers/python/age/models.py
modified: drivers/python/age/networkx/lib.py
modified: drivers/python/requirements.txt
modified: drivers/python/setup.py
modified: drivers/python/test_agtypes.py
modified: drivers/python/test_networkx.py
new file: drivers/python/test_security.py
Fix security vulnerabilities in Node.js driver and harden input
validation and query construction.
Note: This PR was created with AI tools and a human.
- Add input validation for graph names, label names, and column names
to prevent SQL injection via string interpolation. Graph name rules
are based on Apache AGE's naming conventions and Neo4j/openCypher
compatibility (hyphens and dots permitted, min 3 chars, max 63 chars).
Label names follow AGE's stricter rules (no hyphens or dots).
- Add design documentation noting intentional ASCII-only restriction
in driver-side regex validation as a security hardening measure
(homoglyph/encoding attack surface reduction). AGE's server-side
validation (name_validation.h) uses full Unicode ID_Start/ID_Continue
and remains the authoritative check for Unicode names.
- Add safe query helpers: queryCypher(), createGraph(), dropGraph()
with graph name validation and dollar-quoting for Cypher strings
- Add runtime typeof check on dropGraph cascade parameter to prevent
injection from plain JavaScript consumers (TypeScript types are
erased at runtime)
- Use BigInt for integer values exceeding Number.MAX_SAFE_INTEGER to
prevent silent precision loss with 64-bit AGE graph IDs
- Make CREATE EXTENSION opt-in via SetAGETypesOptions.createExtension
instead of running DDL automatically without user consent
- Wrap LOAD/search_path setup in try/catch with actionable error
message mentioning CREATE EXTENSION and { createExtension: true }
- Improve agtype-not-found error message with installation guidance
- Tighten pg dependency from >=6.0.0 to >=8.0.0
- Add comprehensive test suites covering validation, SQL injection
prevention, cascade type safety, hyphenated graph name integration,
BigInt parsing, and setAGETypes error handling
- Add design note in tests documenting why createExtension: false is
the correct default (CI image has AGE pre-installed, auto-creating
extensions requires SUPERUSER and conflates concerns)
modified: drivers/nodejs/package.json
modified: drivers/nodejs/src/antlr4/CustomAgTypeListener.ts
modified: drivers/nodejs/src/index.ts
modified: drivers/nodejs/test/Agtype.test.ts
modified: drivers/nodejs/test/index.test.ts
Previously, when iterating through an agtype container, the code would access `elem->val` even when `elem` was null. This adds a null check to set the result type to AGTV_NULL when the element is null, preventing a potential segmentation fault. Fixes: 4274f10 ("Added the toStringList() function (apache#1084)") Found by PostgresPro. Signed-off-by: Maksim Korotkov <m.korotkov@postgrespro.ru>
Fixed the following ISO C90 warning -
src/backend/utils/adt/agtype.c:7503:9: warning: ISO C90 forbids mixed declarations
and code [-Wdeclaration-after-statement]
7503 | enum agtype_value_type elem_type = elem ? elem->type : AGTV_NULL;
| ^~~~
No regression tests impacted.
modified: src/backend/utils/adt/agtype.c
Implement the openCypher reduce(acc = init, var IN list | body) expression,
which folds an arbitrary expression over a list, threading an accumulator
across the elements in list order. This closes a long-standing gap (reduce()
was previously unsupported) and works both at the SQL top level and inside a
cypher() RETURN/WHERE.
Implementation
--------------
reduce() is desugared, at transform time, into a correlated scalar subquery
over a new ordered aggregate rather than a new executor node, so no PostgreSQL
core changes are required:
CASE WHEN list IS NULL THEN NULL
ELSE COALESCE((SELECT ag_catalog.age_reduce(
<init>, '<serialized-body>'::text,
r.elem ORDER BY r.ord)
FROM unnest(<list>) WITH ORDINALITY AS r(elem, ord)),
<init>)
END
- A new cypher_reduce extensible node carries the accumulator/element names
and the init/list/body expressions (grammar production, keyword, and the
copy/out/read serialization plumbing).
- The fold body is transformed against a throwaway two-column agtype
namespace, its accumulator and element Var references are rewritten to
PARAM_EXEC params 0 and 1, and it is serialized with nodeToString() into a
text argument.
- age_reduce_transfn (a custom agtype aggregate transition function)
deserializes and compiles the body once per group with ExecInitExpr, then
evaluates it per element with ExecEvalExpr, rebinding the two params. The
body is normalized to agtype at transform time so a boolean or other
non-agtype result cannot be misread as a by-reference Datum.
Semantics
---------
- List order is preserved (unnest WITH ORDINALITY + aggregate ORDER BY).
- An empty list yields the initial value; a NULL list yields NULL.
- The list and initial value may reference outer-query variables (e.g.
reduce(total = 0, n IN nodes(p) | total + n.age)); the body may reference
only the accumulator and element.
- Arithmetic, string, list-building, boolean/comparison (AND/OR/=/>), CASE,
and element property-access bodies are all supported.
- Outer-variable, query-parameter, nested-reduce, and aggregate references
inside the body raise a clean ERRCODE_FEATURE_NOT_SUPPORTED error.
- reduce is registered as a safe keyword so it remains usable as a property
or map key, preserving backward compatibility.
Tests
-----
Adds the age_reduce regression test (registered in the install SQL and the
upgrade template so age_upgrade passes), covering: arithmetic/product/string
folds; order sensitivity; empty/NULL list; NULL element and NULL init;
list-building and CASE bodies; boolean and comparison bodies; element property
access; multiple and nested (in list/init) reduce(); reduce() in boolean
expressions, WHERE, and list comprehensions; folds over collected nodes and
node list properties; the not-supported rejections; and reduce as a map key.
Following reviewer feedback, three further semantics-coverage gaps are
pinned directly so the mechanisms that make the aggregate desugaring
correct are exercised by tests rather than only correct by inspection:
- A fold body that produces null mid-fold and then recovers: the
agtype 'null' running state is a readable value, so a later element
folds back out of it (distinct from "null propagates to a null
result", which was already covered).
- An empty list with a NULL initial value: COALESCE(<no rows>, init)
yields NULL, kept distinct from a body that legitimately folds to
agtype 'null', which must not be resurrected to the initial value.
- A type error and a runtime division-by-zero error in the body: both
abort cleanly out of the standalone per-element evaluator rather
than corrupting the running aggregate state.
All multi-row results are ordered. 42/42 installcheck pass.
Future work
-----------
The body restriction (accumulator and element only) is a property of the
standalone expression evaluation and can be relaxed without core changes:
- Allow loop-invariant outer-variable and cypher $parameter references in
the body by capturing them as additional eager aggregate arguments bound
to extra param slots.
- Support a nested reduce() inside the body via an SPI-based evaluation
fallback for subquery-bearing bodies.
Aggregates inside the body remain intentionally unsupported, matching the
openCypher specification.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
modified: Makefile
modified: age--1.7.0--y.y.y.sql
new file: regress/expected/age_reduce.out
new file: regress/sql/age_reduce.sql
modified: sql/age_aggregate.sql
modified: src/backend/nodes/ag_nodes.c
modified: src/backend/nodes/cypher_copyfuncs.c
modified: src/backend/nodes/cypher_outfuncs.c
modified: src/backend/nodes/cypher_readfuncs.c
modified: src/backend/parser/cypher_analyze.c
modified: src/backend/parser/cypher_clause.c
modified: src/backend/parser/cypher_gram.y
modified: src/backend/utils/adt/agtype.c
modified: src/include/nodes/ag_nodes.h
modified: src/include/nodes/cypher_copyfuncs.h
modified: src/include/nodes/cypher_nodes.h
modified: src/include/nodes/cypher_outfuncs.h
modified: src/include/nodes/cypher_readfuncs.h
modified: src/include/parser/cypher_kwlist.h
The vertex/edge staging copies in create_subgraph() generated new
graphids with nextval(%L), which binds the sequence as a string literal
and invokes the nextval(text) overload. That re-resolves the
schema-qualified sequence name on each call.
Cast the literal to regclass (nextval(%L::regclass)) so the sequence is
resolved once to its OID, matching how AGE defines its label id defaults
(nextval('schema.seq'::regclass)). Applied to both the vertex and edge
staging queries, in sql/age_subgraph.sql and the identical body in the
age--1.7.0--y.y.y.sql upgrade template so the upgrade-path catalog
comparison still matches.
Behavior is unchanged; all 38 regression tests pass against PostgreSQL 18.
Addresses Copilot review feedback on apache#2441.
Co-authored-by: GitHub Copilot (Claude Opus 4.8) <[email protected]>
modified: age--1.7.0--y.y.y.sql
modified: sql/age_subgraph.sql
Support relationship-type filters and a minimum hop count in shortest_path SRFs age_shortest_path / age_all_shortest_paths gain two related capabilities, both following openCypher / Neo4j semantics. Relationship-type filtering: the edge_types argument now accepts an array of types; an edge matches when its label is any one of the requested types. A bare string or a one-element array keeps the single-type behaviour, an empty string/array or NULL means no filter, and an unknown type matches nothing. sp_run_bfs takes an Oid set rather than a single oid, and sp_compute_paths resolves the argument into that set. Minimum hop count: the new min_hops argument is a lower bound on the path length. When it does not exceed the true shortest distance it imposes no constraint, so the normal BFS shortest-path result is returned. When it exceeds the shortest distance, BFS cannot produce a qualifying path, so the search falls back to the variable-length-edge depth-first engine (sp_minhops_fallback), which enumerates edge-distinct paths (relationship-uniqueness / trail semantics) and returns the shortest path(s) whose length is at least min_hops. This regime permits revisiting a vertex and closed walks back to the start, but never reusing an edge. A private memory context bounds the search and a cost guard caps the number of examined paths, raising PROGRAM_LIMIT_EXCEEDED (with a hint to bound the search with a maximum hop count) when the cap is exceeded. The hard regime combined with multiple relationship types is unsupported, because the VLE engine matches a single label; that case raises FEATURE_NOT_SUPPORTED. Regression coverage spans single- and multi-type filters, directed and undirected reachability, multiplicity of equal-length paths, max_hops bounds, NULL and non-existent endpoints, and both min_hops regimes, including a vertex-revisiting longer path (sp_revisit) and a closed-walk cycle back to the start (sp_tri). The in-cypher() Tier 1 call forms are exercised as well. Review feedback addressed: 1. Error messages now report the function actually called. age_shortest_path and age_all_shortest_paths share their argument-resolution helpers, which hard-coded an "age_shortest_path" prefix regardless of the caller; the caller's name is now threaded through so each function reports its own (this also corrects a mislabeled multi-type min_hops error). A new regression case (sp_errname) pins the behaviour for both functions. 2. age_all_shortest_paths now bounds the number of materialized result paths. The shortest-path DAG can contain exponentially many equal-length paths, all built up front before the first row streams; enumeration is capped at SP_MAX_RESULT_PATHS (1,000,000), raising PROGRAM_LIMIT_EXCEEDED with a hint to narrow the search, mirroring the existing min-hops candidate cap. 3. The BFS search state (visited table, frontier queue, predecessor multiset, and intermediate path arrays) now lives in a private scratch memory context that is deleted once the surviving result Datums are built in the SRF context, rather than persisting in multi_call_memory_ctx for the life of the SRF. This bounds peak memory to the result set plus one search and matches the pattern sp_minhops_fallback already used. 4. A second review round tightened memory hygiene and reporting: the pnstrdup'd relationship-type name is freed once resolved to an oid (it was retained for the life of the SRF) in both the array and scalar cases; the invalid-direction error now carries the called function's name like the other argument errors; the min-hops fallback's private context is renamed to a caller-neutral "age shortest path minhops" (it is shared by both SRFs); and the multi-type label-filter comment is corrected to note that an unknown type merely contributes no matches -- known types in the same set still match, and only an all-unknown set leaves just the zero-length path. 41/41 installcheck. Co-authored-by: Copilot <copilot@github.com> modified: regress/expected/age_shortest_path.out modified: regress/sql/age_shortest_path.sql modified: src/backend/utils/adt/age_vle.c
…pache#2443) (apache#2444) A single-node labeled pattern used as a boolean expression -- e.g. `WHERE (a:Person)`, `WHERE EXISTS((a:Person))` -- was accepted but did not test the bound vertex's label. It desugars to an EXISTS sub-pattern, and make_path_join_quals() returned early for vertex-only patterns (list_length(entities) < 3), emitting no quals. With no edge to carry a correlation, the sub-pattern referenced nothing from the enclosing query, so the planner produced an uncorrelated one-time InitPlan that was trivially true whenever any vertex of that label existed -- the predicate matched every outer row. Emit an explicit label-id filter for a vertex-only pattern whose vertex carries a non-default label and whose variable is declared in an ancestor parse state (i.e. a correlated reference). make_qual() builds a name-based id reference that resolves to the outer variable, so the filter both correlates the sub-pattern to that variable and enforces the label. Freshly scanned, non-correlated vertices (no ancestor binding) are untouched, so plain MATCH (a:Person) and "does any X exist" EXISTS checks are unaffected. Add regression coverage in pattern_expression: WHERE (a:Person), WHERE NOT (a:Person), and EXISTS((a:Company)) against a graph with a non-Person vertex. All 41 regression tests pass.
* Preserve null-valued keys in map literals (apache#2391) Map literals such as RETURN {a: null} previously dropped keys whose values were null, producing {} instead of {"a": null}. This diverged from the openCypher / Neo4j semantics where map literals preserve every key the user wrote, including those bound to null. Root cause: cypher_map.keep_null defaulted to false (zero-initialised), so the grammar-produced node fed agtype_build_map_nonull, which strips null entries. Call sites that legitimately need strip-null semantics (CREATE node/edge property maps and SET = assignments) already set keep_null=false explicitly, and the MATCH pattern path sets it to true explicitly. Flipping the grammar default to true therefore only affects the cases that were buggy (bare map expressions and nested map values), and leaves CREATE/SET behaviour unchanged. Two preexisting tests encoded the old buggy output and are updated: expr.out (bare RETURN maps now keep the null value) and agtype.out (a nested map inside an orderability test no longer drops its null entry, shifting one row in the ORDER BY result). Dedicated regression coverage for apache#2391 is added to regress/sql/expr.sql. * Address review: move 'End of tests' marker and drop order claim - Move 'End of tests' to after the Issue 2391 test block so it marks the actual end of the file. - Remove 'in order' from the mixed-values comment since map key ordering is not guaranteed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update accessor EXPLAIN expected output for null-preserving map literals Map literals now build with agtype_build_map (keep_null) instead of agtype_build_map_nonull, so the accessor-optimization EXPLAIN plan in expr.out must reflect the new function name. Also strip a stray trailing CRLF on the last line of expr.sql/expr.out that leaked a carriage return into the regression output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add nested-map write coverage and clarify top-level strip wording A reviewer noted the keep_null=true default reaches further than the PR summary stated: CREATE / SET = only override keep_null=false on the top-level property map, not recursively. A nested map value is its own cypher_map node, so it now inherits the new default and preserves its null-valued keys (e.g. CREATE (n {a: {b: null}}) stores a -> {"b": null}). - regress/sql/expr.sql, regress/expected/expr.out: pin the nested case under a write with CREATE (n:Nested {a: {b: null}}), MATCH ... SET n = {a: {b: null}}, and a MATCH verify. - src/backend/parser/cypher_gram.y: clarify the map: rule comment to state the CREATE / SET override is top-level only and nested map values keep the null-preserving default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Node.js driver CI (npm install -> npm run build -> tsc) failed with parser errors in node_modules/@types/node/ffi.d.ts (TS1139/TS1005/TS1109/ TS1128). package-lock.json is gitignored, so CI resolves dependencies purely from package.json. @types/node was only pulled transitively via a wildcard range (@types/pg and jest depend on @types/node@*), so a fresh install grabbed the latest (26.x). That version uses `const` type parameters (a TypeScript 5.0 feature) in ffi.d.ts, which typescript@4.9 cannot parse. skipLibCheck does not suppress these parser-level errors. The runtime Node version is unrelated: @types/node is resolved from the npm dependency graph, not the Node.js runtime. Fix: - Add a bounded direct devDependency "@types/node": "^20.19.0" so a fresh install constrains the typings to the Node 20 LTS line, which is compatible with typescript@4.9 and keeps the toolchain consistent (eslint 7 / typescript-eslint 4 / TS 4.9 / Node 20 typings). - Pin CI to Node 20 (setup-node@v4, node-version: 20) for reproducibility and to match the pinned typings; replaces the deprecated setup-node@v3 and floating node-version: latest. Verified: a clean, no-lockfile install (matching CI) now resolves @types/node@20.19.43 and tsc builds successfully. Co-authored-by: Copilot <copilot@github.com> modified: .github/workflows/nodejs-driver.yaml modified: drivers/nodejs/package.json
…ache#2451) toFloatList()'s AGTV_FLOAT branch formatted each element with sprintf(buffer, "%f", ...) into a fixed 64-byte stack buffer and then re-parsed the string back into a float. This had two defects: 1. Stack overflow. "%f" prints the full integer part with no width limit, so a large magnitude overflows the 64-byte buffer. The value is query-reachable: RETURN toFloatList([1.0e308]) needs ~317 bytes (309 integer digits + ".000000") and smashes the stack. This is the issue reported in apache#2410. 2. Precision loss. "%f" emits only 6 fractional digits, so the format-and-reparse round trip was lossy -- toFloatList([0.123456789]) returned 0.123457. The element is already a float8, so the whole format/reparse step is unnecessary. Assign elem->val.float_value directly. This removes the stack buffer entirely (no magic buffer size to justify) and fixes both the overflow and the precision loss at once. Also harden toStringList(): its "%.*g"/"%ld" conversions use bounded formats and were never overflow-prone, but switch them from sprintf to snprintf as defensive depth. Add regression coverage to regress/sql/expr.sql for both the large magnitude case (no overflow) and precision preservation. This reimplements the fix originally proposed by David Christensen in apache#2410, whose report identified the sprintf overflow. Co-authored-by: David Christensen <david.christensen@snowflake.com>
Allow a reduce(acc = init, var IN list | body) fold body to reference
loop-invariant values from the enclosing query -- outer-query variables and
cypher() parameters -- in addition to the accumulator and element. These were
previously rejected with ERRCODE_FEATURE_NOT_SUPPORTED.
How it works
------------
The fold body is still compiled to a standalone expression evaluated by
age_reduce_transfn, so an outer reference (which cannot be evaluated there)
is captured at transform time and supplied as a value:
- After the accumulator and element are rewritten to PARAM_EXEC params 0 and
1, transform_cypher_reduce() walks the body and replaces each loop-invariant
outer reference -- one that references an outer Var or a cypher() $parameter
but not the accumulator/element -- with a new PARAM_EXEC param 2, 3, ... in
body order. Capture is at leaf granularity: only the bare outer value is
hoisted out of the body, while the operators, function calls and
CASE/AND/OR/coalesce branches around it stay in the serialized body.
Because a captured value becomes an aggregate argument that the executor
evaluates eagerly and unconditionally, hoisting a whole computed subtree
(for example "1/z" under a never-taken CASE branch) would defeat the fold's
short-circuiting; capturing only the leaf keeps evaluation under the body's
own control flow. The one exception is an outer reference that is not itself
agtype-typed -- most commonly the graphid inside a graph vertex/edge
variable -- whose smallest enclosing agtype-typed subtree is captured whole,
since it cannot stand alone as an agtype[] extra.
- The captured expressions are passed to the aggregate as a trailing
agtype[] argument; age_reduce(agtype, text, agtype, agtype[]) and its
transition function gain this argument.
- age_reduce_transfn sizes its param array to 2 + the number of captures and
binds the captured values to params 2.. on every row. Because the captures
are evaluated in the outer query context as ordinary aggregate arguments, a
correlated capture is re-evaluated per group, so an outer value that varies
per row (for example under UNWIND) is folded with the correct value.
Each capture slot is rebound on every row, and the trailing extras
argument is read only when the aggregate actually passes it (PG_NARGS),
keeping the transition safe under direct age_reduce() SQL calls and an
older 4-argument signature.
This keeps the no-core-patch design: the body is still a serialized standalone
expression, and the only new machinery is the captured-value plumbing.
Still rejected
--------------
Subqueries in the body (including a nested reduce()) and aggregate functions
remain unsupported and raise a clean ERRCODE_FEATURE_NOT_SUPPORTED error: a
subquery cannot be planned as a plain aggregate argument, and an aggregate in a
per-element fold is undefined per the openCypher specification.
Tests
-----
age_reduce gains an "Outer references in the fold body" section covering a
plain outer variable, an outer variable used as a multiplier, two distinct
outer variables, a property of an outer graph variable, the same outer variable
referenced more than once, a property of an outer map, a subexpression that
mixes an outer reference with the element (only the loop-invariant part is
captured), an outer reference inside a CASE branch of the body, a NULL outer
value propagating through the fold, multiple captures mixing a NULL and a
non-NULL outer value, an outer variable that changes per row (captured per
group), and a cypher() parameter supplied via a prepared statement.
A "Short-circuit evaluation is preserved for outer references in the body"
section verifies that a guarded outer sub-expression is not evaluated on a
branch that is not taken: a never-taken CASE THEN branch, a never-taken CASE
ELSE branch, an OR and an AND that short-circuit, and a coalesce -- each of
which would divide by zero if the outer "1/w" were hoisted into an eagerly
evaluated aggregate argument -- plus a guarded branch that is taken and
evaluates its outer division normally.
The previously-rejected outer-variable case is moved out of the not-supported
section, which now covers a nested reduce() (any subquery in the body is
unsupported) and an aggregate in the body.
The same change also broadens the base reduce() coverage with value-type folds
(a float accumulator, negative numbers, a map accumulator passed through
unchanged, and list elements indexed in the body), function calls in the fold
body (a scalar function over the element and the list itself produced by a
function), reduce() composed with surrounding expressions (consumed by another
function and used in a comparison), and syntax-error checks for each required
piece of the form -- the "= init", ", var IN list", and "| body" clauses, plus
a rejected qualified iterator variable. 42/42 installcheck pass.
Co-authored-by: Copilot <copilot@github.com>
modified: age--1.7.0--y.y.y.sql
modified: regress/expected/age_reduce.out
modified: regress/sql/age_reduce.sql
modified: sql/age_aggregate.sql
modified: src/backend/parser/cypher_clause.c
modified: src/backend/utils/adt/agtype.c
apache#2453) * Fix segfault and out-of-bounds reads in file loaders on malformed rows load_edges_from_file() and load_labels_from_file() build their COPY parser with only format=csv and header=false, so COPY uses its default comma delimiter. A file delimited by anything else (or a malformed row) then parses with an unexpected column count, and the loaders indexed the parsed fields without validating that count: - process_edge_row() reads the four fixed fields fields[0..3] unconditionally. A non-comma-delimited edge file parses as a single column, so fields[1..3] are out of bounds -> segfault (issue apache#2449). - create_agtype_from_list()/_i() pair header[i] with fields[i] for all i < nfields, so a row with more fields than the header reads header[i] out of bounds. Add bounds validation that turns these into clear errors: - Edge header must have >= 4 columns; a smaller count almost always means the wrong delimiter, so the error carries a hint. - Each edge row must have >= 4 columns and no more than the header's. - Each label row must have no more than the header's column count. Rows with fewer trailing columns than the header remain allowed, matching existing behavior (exercised by the existing conversion tests). This closes the segfault and out-of-bounds reads. The silent mis-parsing of a non-comma file whose header and rows share the same (wrong) column count is not detectable here; adding a delimiter option to the load functions is a separate follow-up. Adds a regression test in age_load using a pipe-delimited edge file. Addresses apache#2449. * loader guards: clarify error wording and add per-row regression coverage Address review feedback on the nfields guards: - Error messages now say "the header's %d columns" (was "the header's %d"), making the count's unit explicit. - Add regression cases exercising the per-row guards, which previously only had coverage for the mis-delimited-header path: * an edge row with fewer than 4 columns * an edge row with more columns than the header * a label row with more columns than the header Each asserts a clean ERROR (these were the out-of-bounds reads the guards now catch).
…on for GLR grammar (apache#2445) * ci: pin runner to ubuntu-24.04 + guard Bison version for GLR grammar The Cypher GLR grammar pins exact shift/reduce and reduce/reduce conflict counts via %expect / %expect-rr in cypher_gram.y, and Bison treats %expect as exact-match: a different Bison version can report different counts and break the build. ubuntu-latest floats to new Ubuntu releases (and new Bison versions), which would silently shift those counts. Pin runs-on to ubuntu-24.04 to freeze Bison at 3.8.x, and add a guard step that fails loudly with a pointer to cypher_gram.y if Bison ever drifts off 3.8.x. Reproducibility comes from pinning the variable rather than widening the conflict-count tolerance, keeping the exact-match alarm for genuinely new grammar conflicts intact. * ci: make Bison version parse robust (awk + explicit empty guard) Address Copilot review on apache#2445: the previous grep-based parse could silently yield an empty version and fail with a confusing 'Bison != 3.8.x' message. Parse the version field with awk and error explicitly when it can't be determined.
AGE lists OBJS explicitly and relies on PGXS, whose built-in dependency
tracking only runs when the server was built with --enable-depend (often
disabled). Consequently, editing a header did not recompile the .c files
that include it, leaving stale .o files. This is especially dangerous for
node/struct headers: a stale ag_nodes.o keeps an outdated node_size, so
_readExtensibleNode under-allocates and readNode corrupts the heap
("unrecognized node type").
Emit a .d file beside each object via -MMD -MP and -include them, deriving
DEPFILES from OBJS. The mechanism is self-contained (independent of
--enable-depend): -MMD skips system headers and -MP tolerates deleted
headers. On servers built with --enable-depend, PGXS appends its own -MF
after CFLAGS (last -MF wins), so this degrades cleanly to PGXS's tracking.
Add DEPFILES to EXTRA_CLEAN and *.d to .gitignore.
Co-authored-by: Copilot <copilot@github.com>
modified: .gitignore
modified: Makefile
…#2406) Fix age_unnest to emit SQL NULL for AGTV_NULL elements and rewrite single() to proper three-valued logic, fixing UNWIND [null] count, list-comprehension null filters, and single() semantics per openCypher. Closes apache#2383 Closes apache#2393
apache#2458) 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 and id/start_id/end_id/properties for edges. A user-added column on the label table -- most notably a GENERATED ALWAYS ... STORED column, but also any plain ADD COLUMN -- therefore left an uninitialized slot entry. ExecStoreVirtualTuple marks all attributes valid, so heap_form_tuple() -> heap_compute_data_size() read the uninitialized tts_isnull[]/tts_values[] for that column and segfaulted dereferencing garbage as a varlena. This crashed the backend on CREATE, MERGE and SET. Fix in two parts: - clear_entity_slot() clears the slot and marks every attribute NULL before AGE fills in its own columns, so columns AGE does not manage default to NULL instead of stale slot memory. It is used at every slot build site: create_vertex, create_edge, merge_vertex, merge_edge, and the populate_vertex_tts/populate_edge_tts helpers used by SET. - insert_entity_tuple_cid (CREATE/MERGE) and update_entity_tuple (SET) call ExecComputeStoredGenerated() before materializing the heap tuple when the relation has stored generated columns, so those columns are computed on insert and recomputed on update per PostgreSQL semantics. Adds a generated_columns regression test covering stored generated columns on vertex and edge labels across CREATE, MERGE and SET, plus a plain user column that must not crash.
Fix the installcheck.yaml fetch-depth parameter to 100. This release, being rather large, exceeded the original value of 75 (commits) and caused the upgrade test to fail. The new value will resolve this and make it unlikely to become a problem in the future. NOTE: We need to keep releases under this number of commits. modified: .github/workflows/installcheck.yaml
Updated the following files to advance the Apache AGE version to 1.8.0 modified: META.json modified: README.md modified: RELEASE renamed: age--1.7.0--y.y.y.sql -> age--1.7.0--1.8.0.sql modified: age.control modified: docker/Dockerfile new file: age--1.8.0--y.y.y.sql
Bring the PG18 branch current with master (both target PostgreSQL 18) by merging all of master's commits, preserving PG18-specific CI/driver config. Content is taken from master (all code, tests, docs, SQL, 1.8.0 bump). Preserved on top: CI workflow triggers kept at branches: [ "PG18" ] (master's improved workflows otherwise adopted); driver test image kept at apache/age:dev_snapshot_PG18; .github/labeler.yml dropped to match master (apache#2335). Co-authored-by: Copilot <copilot@github.com>
Contributor
Author
|
@MuhammadTahaNaveed @gregfelice Opus 4.8 Max 1M review as Copilot can't handle it. PR #2466 sanity check — PG18-specific CI/docker/driver items preserved ✅Verdict: PASS. This is a true merge of master into PG18 that brings in all of the Merge integrity
PG18-specific items — all preserved
The green Build/Regression check is itself confirmation the CI builds and tests against Net: all of master's 1.8.0 code is brought in, and nothing PG18-specific (CI triggers, |
gregfelice
approved these changes
Jul 7, 2026
gregfelice
left a comment
gregfelice
left a comment
Contributor
There was a problem hiding this comment.
Approve — independently verified locally (tree audit + -Werror build + regression on PG18.4)
Reviewed on the PR head (f4798d33), not just by summary. Everything checks out.
Merge integrity
- True merge, 2 parents:
806fa2eb(PG18) +9fb7df8c(master).upstream/masteris a direct ancestor of the head — master is fully contained, and master is not ahead by a single commit. - The entire diff between the PR head tree and
masteris 8 files / 13 lines, all amaster→PG18substitution: CI branch triggers in 5 workflows (branches: [ "PG18" ]) plus the driver test image in 3 places (dev_snapshot_PG18). No straymasterleaks, no code/test/SQL drift.
PG18 identity intact: age.control default_version = '1.8.0', Dockerfile FROM postgres:18 + age--1.8.0.sql, META.json 1.8.0, .github/labeler.yml absent (matches master #2335).
Built and tested locally against system PostgreSQL 18.4:
make clean && make -j4 COPT=-Werror→ exit 0, zero warnings, finalage.solink carries-Werror.pg_regresson a fresh temp instance → 42/42 tests passed (skipped onlyage_upgrade, which is the upgrade-staging test, not merge code — its logic already landed and was validated on #2455).- Matches the green CI here.
Net: textbook-clean catch-up merge, safe to land. 👍
One merge-mechanics question before I press the button, @jrgemignani: this repo has merge-commits disabled (only squash + rebase are allowed), so I can't land your merge commit byte-for-byte via the button. Squash is out (it'd collapse all 101 master commits into one on PG18). My plan is a rebase-merge, which preserves every individual commit but replays them with new SHAs and drops the merge-commit wrapper. That fine with you, or would you rather land it another way (e.g. a fast-forward push to keep the merge commit exactly)? I'll merge as soon as you confirm.
Contributor
|
Going ahead and landing this now via rebase-merge (merge-commits are disabled on the repo, and squash would collapse the 101 commits). This preserves every individual commit; only the merge-commit wrapper is dropped and SHAs are replayed onto PG18. Shout if you'd have preferred a fast-forward push to keep the merge commit exactly and I'll help sort it out. |
Contributor
|
Correction to my note above: rebase-merge turned out to be unavailable (GitHub can't rebase a PR whose history contains merge commits), and squash would've collapsed the 101 commits. So I landed it via a fast-forward push instead — PG18's tip was exactly the first parent of your merge commit, so it fast-forwarded cleanly and your merge commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: There will still be another PR to update the RELEASE and other files needed prior to the release. This is just catching PG18 up to speed.
Merge: 806fa2e 9fb7df8
Author: John Gemignani jgemignani@users.noreply.github.com
Date: Mon Jul 6 23:50:37 2026 +0000