From: | Steve Chavez <steve(at)supabase(dot)io> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Assert name/short_desc to prevent SHOW ALL segfault |
Date: | 2022-05-25 05:05:55 |
Message-ID: | CAGRrpzZ-3xhdR0q-S9Tk58fDh5=Y=j98duYNa2_-SLnfKQDe5w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for the reviews Nathan, Michael.
I agree with handling NULL in ShowAllGUCConfig() instead.
I've attached the updated patch.
--
Steve Chavez
Engineering at https://supabase.com/
On Tue, 24 May 2022 at 20:21, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Tue, May 24, 2022 at 11:41:49AM -0700, Nathan Bossart wrote:
> > I would actually ERROR on this so that we aren't relying on
> > --enable-cassert builds to catch it. That being said, if there's no
> strong
> > reason to enforce that a short description be provided, then why not
> adjust
> > ShowAllGUCConfig() to set that column to NULL when short_desc is missing?
>
> Well, issuing an ERROR on the stable branches would be troublesome for
> extension developers when reloading after a minor update if they did
> not set their short_desc in a custom GUC. So, while I'd like to
> encourage the use of short_desc, using your suggestion to make the
> code more flexible with NULL is fine by me. GetConfigOptionByNum()
> does that for long_desc by the way, meaning that we also have a
> problem there on a build with --enable-nls for short_desc, no?
> --
> Michael
>
Attachment | Content-Type | Size |
---|---|---|
0001-Handle-a-NULL-short_desc-in-ShowAllGUCConfig.patch | text/x-patch | 1.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2022-05-25 05:31:01 | Re: Assert name/short_desc to prevent SHOW ALL segfault |
Previous Message | Michael Paquier | 2022-05-25 05:00:23 | Re: fix stats_fetch_consistency value in postgresql.conf.sample |