Date: Tue, 12 Apr 2022 15:47:08 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Ranier Vilela <ranier.vf@gmail.com>
Cc: Daniel Gustafsson <daniel@yesql.se>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Amit Langote <amitlangote09@gmail.com>,
	Japin Li <japinli@hotmail.com>, Zhihong Yu <zyu@yugabyte.com>,
	simon.riggs@enterprisedb.co,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Daniel Westermann <dwe@dbi-services.com>,
	Erik Rijkers <er@xs4all.nl>,
	Jaime Casanova <jcasanov@systemguards.com.ec>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: support for MERGE
Message-ID: <202204121347.2s2p6eed5mv2@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAEudQApp3aHZ+Vfn0Drx1hc+=pOnvZ7dMxJqcbf5t_riS0ZXqA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202204121347.2s2p6eed5mv2%40alvherre.pgsql>
Precedence: bulk

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)