Date: Sat, 2 Apr 2022 16:11:51 -0700
From: Andres Freund <andres@anarazel.de>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	John Naylor <john.naylor@enterprisedb.com>,
	Peter Geoghegan <pg@bowt.ie>,
	pgsql-hackers <pgsql-hackers@postgresql.org>,
	Robert Haas <rhaas@postgresql.org>
Subject: Re: A qsort template
Message-ID: <20220402231151.635ohpc4q3gppdzb@alap3.anarazel.de>
References: <CAFBsxsHr-C1xqjUMjeUN3-FvNzKiAt3gcfBKt8PFN2mov7z2gQ@mail.gmail.com>
 <CAFBsxsHzECMg460Y08KqZqqMEks-c9YQGz7Se2CLJVGj+t3Nfg@mail.gmail.com>
 <CAFBsxsEyWo3+eGMYy2w+q4WjGt5OxywNx-kDBHTT2zgQmQz1KA@mail.gmail.com>
 <CA+hUKG+6Cj_yFgOiwSAqUCbysJuN9_HS3=ukUSV4ziY-irc26Q@mail.gmail.com>
 <CAFBsxsGEixFbRyeO6wmbvOnR-Z70qZFdNkQwDQNeWfMpGYeFMw@mail.gmail.com>
 <CA+hUKG+67T0nr1mcB2cFkKYktWc2TFh52QWfvGdvf259Ah+PhA@mail.gmail.com>
 <CAFBsxsEsA2BjUcfWftLbaVMNs3Z_n2fZYHcErva9MdZcNYiYLA@mail.gmail.com>
 <20220402202027.GG28503@telsasoft.com>
 <20220402210333.bwf4h375p4fxt5j6@alap3.anarazel.de>
 <CA+hUKGLNpDXytCwUnOQYvGkvjR4h+0ev=ivqLGuUO-w0WAJ_7A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+hUKGLNpDXytCwUnOQYvGkvjR4h+0ev=ivqLGuUO-w0WAJ_7A@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20220402231151.635ohpc4q3gppdzb%40alap3.anarazel.de>
Precedence: bulk

Hi,

On 2022-04-03 09:45:13 +1200, Thomas Munro wrote:
> On Sun, Apr 3, 2022 at 9:03 AM Andres Freund <andres@anarazel.de> wrote:
> > It's certainly not pretty that copytup_cluster() can use SortTuples without
> > actually using SortTuples. Afaics it basically only computes isnull1/datum1 if
> > state->indexInfo->ii_IndexAttrNumbers[0] == 0.
> 
> I think we just need to decide up front if we're in a situation that
> can't provide datum1/isnull1 (in this case because it's an expression
> index), and skip the optimised paths.  Here's an experimental patch...
> still looking into whether there are more cases like this...

That's a lot of redundant checks. How about putting all the checks for
optimized paths into one if (state->sortKeys && !state->disable_datum1)?

I'm a bit worried that none of the !ubsan tests failed on this...

Greetings,

Andres Freund