Hi,
On 2025-02-19 01:48:53 -0500, Tom Lane wrote:
> Pavel Stehule <pavel.steh...@gmail.com> writes:
> > I investigated the crashes in "xmlnamespaces to xmlelement" patch and it
> > looks like there was a badly used makeString function. The argument should
> > not be null, elsewhere serialization to string fails - and deserialization
> > doesn't support this case.
> > I propose to add an assert there like (make check-world passed)
>
> Hmmm ... while I don't necessarily object to this patch, we have a lot
> of makeFoo() functions that build nodes, and hardly any of them have
> asserts like this one.
I also suspect that adding is-not-NULL asserts isn't that helpful on its own,
because you still need to reach that function with it set to NULL. We probably
should use pg_attribute_nonnull() much more widely, so that compilers and
static analyzers can help.
> Why makeString() in particular? Is the fault on the serialization side,
> instead? If there's a general expectation that a String node's value isn't
> null, how come the original patch worked at all?
It's worth noting that the CI task just failed on freebsd, which builds with:
CPPFLAGS: -DRELCACHE_FORCE_RELEASE
-DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
PG_TEST_INITDB_EXTRA_OPTS: -c debug_copy_parse_plan_trees=on -c
debug_write_read_parse_plan_trees=on -c debug_raw_expression_coverage_test=on
Greetings,
Andres Freund
