Copilot commented on code in PR #2286:
URL: https://github.com/apache/age/pull/2286#discussion_r2665967921
##########
sql/age_main.sql:
##########
@@ -189,7 +189,8 @@ CREATE TYPE graphid (
SEND = ag_catalog.graphid_send,
RECEIVE = ag_catalog.graphid_recv,
INTERNALLENGTH = 8,
- PASSEDBYVALUE,
+ -- Note: PASSEDBYVALUE removed for 32-bit WASM compatibility (PGlite)
+ -- On 32-bit systems, Datum is 4 bytes and cannot hold 8-byte graphid by
value
Review Comment:
Removing PASSEDBYVALUE unconditionally from the SQL type definition creates
a critical mismatch with the C code on 64-bit platforms.
On 64-bit systems:
- The SQL type definition (without PASSEDBYVALUE) indicates pass-by-reference
- But the C macros in graphid.h use pass-by-value
(DatumGetInt64/Int64GetDatum)
This inconsistency will cause PostgreSQL to incorrectly interpret graphid
values on 64-bit platforms, leading to crashes or data corruption. The type's
pass-by-value/reference behavior in SQL must match the C implementation.
The SQL type definition would need to be conditional (if PostgreSQL supports
that), or the approach needs to be reconsidered. One solution is to always use
pass-by-reference for consistency, though this would hurt 64-bit performance.
```suggestion
PASSEDBYVALUE,
-- Note: graphid is passed by value to match the C implementation on
64-bit PostgreSQL.
-- 32-bit environments where Datum is 4 bytes cannot safely use this
definition.
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
