| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Haibo Yan <tristan(dot)yim(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Optimize UUID parse using SIMD |
| Date: | 2026-08-16 00:59:00 |
| Message-ID: | CALj2ACU_ZAwqox+02rC-MDS=VuouuGS4nFZ=eYdGPRBzkOVY7g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, Aug 14, 2026 at 6:08 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> > > >> +static void
> > > >> +string_to_uuid(const char *source, pg_uuid_t *uuid, Node *escontext)
> > > >> +{
> > > >> + const char *body = source;
> > > >> + size_t len = strlen(source);
> > > >> ```
> > > >>
> > > >> I think it would be better to avoid strlen(). The old code processes at most UUID_LEN (16) byte pairs, so it does not need to scan arbitrarily far on malformed input. So, maybe we could use something like strnlen(source, 39) instead.
> > > >
> > > > While strnlen(source, 39) works there, 39 is a magic number and it's
> > > > tied to the current format check logic. What is the benefit of using
> > > > strnlen(source, 39) instead? I'm not sure it warrants having the magic
> > > > number.
> > >
> > > It doesn't have to be exactly 39; 1024 (long enough) would also work, or perhaps something based on UUID_LEN, such as UUID_LEN * 3. I think the main point is to avoid unbounded scanning on malformed input.
> > >
> > > The old code did not have this issue because it only examined as much input as needed based on UUID_LEN. The new fast path starts to use strlen(), so this would be a new risk introduced by the optimization.
> >
> > I don't think the scan can be really unbounded. string_to_uuid()
> > receives a cstring, so by the time it is called the caller has already
> > walked or copied the whole string to produce it. So unless the
> > unbounded scan can be reached in some path I have overlooked, I'd
> > prefer to keep strlen() here. Happy to change it if you still think it
> > is worth it.
>
> After more thoughts, while I still don't think the scan can be
> unbounded, using strlen() would add an extra scan just to determine we
> use hex_decode_safe(). I'll change it to use strnlen() instead.
+1 to use strnlen() instead of strlen() just for the simple reason
that it's better not to scan malformed-yet-null-terminated input
strings (say a null-terminated input of 1GB).
DO $$ DECLARE s text := repeat('a', 1000000000); r uuid;
BEGIN r := s::uuid; EXCEPTION WHEN others THEN NULL; END $$;
(gdb) p strlen(uuid_str)
$2 = 1000000000
Instead of N * UUID_LEN in strnlen, NAMEDATALEN also works unless I'm
missing anything.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-08-16 01:18:11 | Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE |
| Previous Message | Sami Imseih | 2026-08-16 00:23:32 | Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE |