Em ter., 12 de abr. de 2022 às 10:47, Alvaro Herrera < alvhe...@alvh.no-ip.org> escreveu:
> On 2022-Apr-02, Ranier Vilela wrote: > > > Em sáb., 2 de abr. de 2022 às 12:01, Alvaro Herrera < > alvhe...@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. > Thanks for the explanation. > > 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. > Thanks for hardening this. > > > 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. > Sure, it is always better to avoid doing work, unless strictly necessary. regards, Ranier Vilela
