On 2022-Apr-02, Ranier Vilela wrote:
> Em sáb., 2 de abr. de 2022 às 12:01, Alvaro Herrera <alvherre@alvh.no-ip.org>
> escreveu:
> IMHO, actually there are bug here.
> ExecGetChildToRootMap is clear, is possible returning NULL.
> To discover if the map is NULL, ExecGetChildToRootMap needs to process
> "ResultRelInfo *leaf_part_rri".
> So, the argument "if the map is NULL, this function should not be called",
> is contradictory.
I was not explicit enough. I meant "if no map is needed to adjust
columns, then this function should not be called". The caller already
knows if it's needed or not; it doesn't depend on literally testing
'map'. If somebody mis-calls this function, it would have crashed, yes;
but that's a caller bug, not this function's.
A few days ago, the community Coverity also complained about this, so I
added an Assert that the map is not null, which should silence it.
> If the right fix is to return the original list, here is the patch attached.
... for a buggy caller (one that calls it when unnecessary), then yes
this would be the correct code -- except that now the caller doesn't
know if the returned list needs to be freed or not. So it seems better
to avoid accumulating pointless calls to this function by just not
coping with them.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"I suspect most samba developers are already technically insane...
Of course, since many of them are Australians, you can't tell." (L. Torvalds)
