Date: Mon, 31 May 2021 16:51:31 +0200
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: "Poot, Bas (B.J.)" <bas.poot@politie.nl>,
	"pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: XX000: unknown type of jsonb container.
Message-ID: <20210531145131.b34sjhhgj7xkq36w@localhost>
References: <534fca83789c4a378c7de379e9067d4f@politie.nl>
 <20210407181105.lmi7hgifwzs3eeee@localhost>
 <20210413161500.kufk4iyrmcmhhbrc@localhost>
 <20210423151505.lxx5ti7ivcza3xqw@localhost>
 <2433209.1622416353@sss.pgh.pa.us>
 <2491424.1622422768@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2491424.1622422768@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/20210531145131.b34sjhhgj7xkq36w%40localhost>
Precedence: bulk

> On Sun, May 30, 2021 at 08:59:28PM -0400, Tom Lane wrote:
> 
> Of course, it's not like the planner has never heard of that restriction.
> The problem occurs because what arrives at createplan.c looks like
> 
> 	ProjectionPath
> 		ProjectionPath
> 			SortPath
> 				SeqScanPath
> 
> that is, we've got *two* redundant ProjectionPaths, and the logic in
> createplan.c that is supposed to decide whether we can skip generating
> a projecting Result node for a ProjectionPath gets confused and
> decides we don't need any Result, rather than deciding we need one
> Result.
> 
> Maybe we could work around that, but having two stacked ProjectionPaths
> is just silly, so the attached patch deals with this problem by making
> sure we don't do that.
> 
> The place where that happens in the example reported in this thread is
> apply_projection_to_path, which somebody kluged up to the point where
> it'd create this situation just below a Gather/GatherMerge node.
> So I first tried to fix it there, and for safety added an Assert to
> create_projection_path saying that its input wasn't a ProjectionPath.
> 
> The Assert blew up the regression tests.  The same thing is happening
> in other places, and only by luck have we not noticed any bad effects.
> 
> So the attached fixes it by stripping any input ProjectionPath in
> create_projection_path, instead.
 
Thanks for looking into this. I don't see any problems with the
suggested patch, so +1.

> This test case only fails back to v13, as does the original example.
> I suspect we should back-patch the code change further though,
> since create_projection_plan will be just as confused by such cases
> in earlier branches.

That would be my assumption as well.