On Sat, Aug 22, 2026 at 10:16:44PM +0200, Matthias van de Meent wrote: > I've moved them to readfuncs.h, which I think is a reasonable > compromise; avoiding the forward declaration whilst still exposing > these functions to extensions that might need them.
The forward declaration feeling unnecessary was my main grip, so I am fine with the suggestion of moving them to readfuncs.c. > There's a "shardman" repo that seems to directly use readOidCols: > https://github.com/ZhaoDiankui/shardman/blob/merge/src/exchange.c#L162 Noted. > Apache AGE seems to use (or at least, reference) our read*Cols > definitions in their backend/nodes/cypher_readfuncs.c; in copied > macros that don't currently seem to have any usage but might get used > in the future. Its READ_BITMAP_FIELD macro won't work as-is (it > references _readBitmap, which is the private/static version, whereas > readBitmap is the exposed version). I saw this one, but it just looked like a copy-paste of the readfuncs.c code to me, not a legit call. > * new 0001 patch, which moves the read* functions from nodes.h to > readfuncs.h. > As was discussed upthread. 0002 is adjusted accordingly. OK for this one here. > * nodes/extensible.h now includes nodes/readfuncs.h > I revised my opinion on this: I noticed my compiler complained > about the plain "struct ReadNodeContext *ctx" argument if I didn't put > a "struct ReadNodeContext;" declaration at the top level of the file, > which (when combined with relevant comments) would add more verbose > clutter than the alternative of including nodes/readfuncs.h. Avoiding the struct markers when we can is a better practice to me. Digression: We still have a few places in the code where we have these markers, I think, perhaps they could be cleaned up.. > * adjusted comment on parseNodeString, as reported upthread. Thanks. -- Michael
signature.asc
Description: PGP signature
