Andrey Lepikhov <a.lepik...@postgrespro.ru> writes:
> In the AQO project (Adaptive Query Optimization) [1] the nodeToString()
> function is used by the planner to convert an query parse tree into a
> string to generate a hash value [2].
Hmm. Not sure that is a bright idea; in fact, I'm pretty sure it's
a *bad* idea. This choice will result in the hash randomly varying
depending on whitespace, for instance. However ...
> In PostgreSQL v.11 call nodeToString(parse) segfaulted.
... that seems like a bad thing, because we commonly use nodeToString
(particularly pprint) to dump nodetrees for debugging purposes.
> The reason is: parse tree node for XMLNAMESPACES clause has null pointer
> in the case of DEFAULT namespace (the pointer will be initialized at
> executor on the first call).
Arguably, *that* is a bug. Or at least it requires some suspicion.
> Function _outValue() uses value->val.str[0]
> [3] without checking of value->val.str.
Indeed, because Value nodes aren't supposed to contain null strings.
I doubt this is the only place that would have a problem with that.
> I want to know, which of next options is correct:
> 1. Converting a parse tree into string with nodeToString() is illegal
> operation. We need to add a comment to the description of nodeToString().
Don't like this one too much because of the debug angle.
> 2. We can use nodeToString() for parse tree convertation. In this case
> we need to check node variable 'value->val.str' to NULL pointer (Now I
> use this approach, see attachment).
This patch is unacceptable because it turns outfuncs/readfuncs into
a non-idempotent transformation, ie a Value node would not read in
the same as it wrote out.
My immediate reaction is that somebody made a bad decision about how
to represent XMLNAMESPACES clauses. It's not quite too late to fix
that; but could you offer a concrete example that triggers this,
so it's clear what case we're talking about?
regards, tom lane
