public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
To: Justin Pryzby <[email protected]>
Cc: John Naylor <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Peter Geoghegan <[email protected]>
Cc: [email protected]
Cc: Robert Haas <[email protected]>
Subject: Re: A qsort template
Date: Sat, 2 Apr 2022 14:03:33 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAFBsxsEV0nzyCmiEhqEHN9jUf8vT073TpW-cwU9mwMr1tk3=xw@mail.gmail.com>
	<CAFBsxsHxy4LuJZVaFD7n-197qoCWZAvXgCxVkWV=_b_9MDnmPA@mail.gmail.com>
	<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>
	<[email protected]>

Hi,

On 2022-04-02 15:20:27 -0500, Justin Pryzby wrote:
> On Sat, Apr 02, 2022 at 06:41:30PM +0700, John Naylor wrote:
> > On Sat, Apr 2, 2022 at 5:27 PM Thomas Munro <[email protected]> wrote:
> > > Reproduced locally, using the same few lines from the cluster.sql
> > > test.  I'll try to dig more tomorrow...
> > 
> > Thanks! Unfortunately I can't reproduce locally with clang 13/gcc 11,
> > with -Og or -O2 with CFLAGS="-fsanitize=undefined,alignment" ...
> 
> Like Thomas just said, I had to use:
> CFLAGS="-Og -fsanitize=undefined,alignment -fno-sanitize-recover=all
> 
> I'm a couple few steps out of my league here, but it may be an issue with:
> 
> commit 4ea51cdfe85ceef8afabceb03c446574daa0ac23
> Author: Robert Haas <[email protected]>
> Date:   Mon Jan 19 15:20:31 2015 -0500
> 
>     Use abbreviated keys for faster sorting of text datums.
> 
> This is enough to avoid the crash, which might be a useful hint..
>
> @@ -4126,22 +4126,23 @@ copytup_cluster(Tuplesortstate *state, SortTuple *stup, void *tup)
>         /*
>          * set up first-column key value, and potentially abbreviate, if it's a
>          * simple column
>          */
> +       stup->isnull1 = false;
>         if (state->indexInfo->ii_IndexAttrNumbers[0] == 0)
>                 return;
>  
>         original = heap_getattr(tuple,
>                                                         state->indexInfo->ii_IndexAttrNumbers[0],
>                                                         state->tupDesc,
>                                                         &stup->isnull1);

I don't think that can be correct - the column can be NULL afaics. And I don't
think in that patch it's needed, because it always goes through ->comparetup()
when state->onlyKey isn't explicitly set. Which tuplesort_begin_cluster() as
well as several others don't.  And you'd just sort an uninitialized datum
immediately after.

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.

Greetings,

Andres Freund






view thread (31+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: A qsort template
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox