From: | Peter Geoghegan <pg(at)bowt(dot)ie> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | James Inform <james(dot)inform(at)pharmapp(dot)de>, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com> |
Subject: | Re: BUG #17584: SQL crashes PostgreSQL when using ICU collation |
Date: | 2022-08-14 02:43:30 |
Message-ID: | CAH2-Wzk11wvwjhrbje4DDyXydy5pGACb4wNCHCXiuij0Y10h+Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
On Sat, Aug 13, 2022 at 5:30 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Attached is a quick draft fix. Some notes:
>
> * I'm not really proposing the added Asserts for commit,
> though they helped provide confidence that I was on the
> right track.
It seems very unlikely that anybody will notice the added cycles in
debug builds. And while the risk of introducing unwanted side-effects
in assert-enabled builds is greater than zero, it might be worth it.
> * We need to look around and see if the same mistake appears
> in any other sortsupport code.
I just did -- I see no problems. The only other SortSupport code that
allocates a buffer that is associated its SortSupport state is used by
numeric. That is a fixed-sized buffer (it's dynamically allocated
because we only need it when abbreviated keys are used).
> * The bug could never have existed at all if these buffer
> resizings had been done with repalloc(). I get that the
> idea is to avoid copying data we don't care about, but
> this coding is still feeling like an antipattern. I wonder
> if it'd be worth inventing a variant of repalloc that makes
> the chunk bigger without preserving its contents.
Or we could just use repalloc(). The vast majority of text sorts won't
need a buffer larger than the initial buffer size (TEXTBUFLEN/1024
bytes), so it seems like a fairly useless optimization.
I think that repalloc might even be mandatory. These buffers are used
as caches that allow us to avoid repeat strxfrm()/strcoll() calls,
which is quite a useful optimization. But we're not accounting for
that here -- we're not invalidating the cache. We wouldn't have to
bother with that if we just used repalloc().
--
Peter Geoghegan
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-08-14 03:21:04 | Re: BUG #17584: SQL crashes PostgreSQL when using ICU collation |
Previous Message | Tom Lane | 2022-08-14 00:30:17 | Re: BUG #17584: SQL crashes PostgreSQL when using ICU collation |