MIME-Version: 1.0
References: <CAGECzQS=L1-Lszs_VfQxpYSfHwyv3eXEjxu6_7cajmnmbYjkCQ@mail.gmail.com>
 <202402291035.op26n4voqmba@alvherre.pgsql>
In-Reply-To: <202402291035.op26n4voqmba@alvherre.pgsql>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 6 Mar 2024 00:18:14 +0530
Message-ID: <CALj2ACWCKfZvcw0tebpzxpPK7D71BMkVinZNSuWZRk7DENj0ig@mail.gmail.com>
Subject: Re: Support a wildcard in backtrace_functions
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Jelte Fennema-Nio <me@jeltef.nl>, Daniel Gustafsson <daniel@yesql.se>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Peter Eisentraut <peter@eisentraut.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CALj2ACWCKfZvcw0tebpzxpPK7D71BMkVinZNSuWZRk7DENj0ig%40mail.gmail.com>
Precedence: bulk

On Thu, Feb 29, 2024 at 4:05=E2=80=AFPM Alvaro Herrera <alvherre@alvh.no-ip=
.org> wrote:
>
> Hmm, so if I write "foo,*" this will work but check all function names
> first and on the second entry.  But if I write "foo*" the GUC value will
> be accepted but match nothing (as will "*foo" or "foo*bar").  I don't
> like either of these behaviors.  I think we should tighten this up: an
> asterisk should be allowed only if it appears alone in the string
> (short-circuiting check_backtrace_functions before strspn); and let's
> leave the strspn() call alone.

+1 for disallowing *foo or foo* or foo*bar etc. combinations. I think
we need to go a bit further and convert backtrace_functions of type
GUC_LIST_INPUT so that check_backtrace_functions can just use
SplitIdentifierString to parse the list of identifiers. Then, the
strspn can just be something like below for each token:

        validlen =3D strspn(*tok,
                        "0123456789_"
                        "abcdefghijklmnopqrstuvwxyz"
                        "ABCDEFGHIJKLMNOPQRSTUVWXYZ");

Does anyone see a problem with it?

FWIW, I've recently noticed for my work on
https://commitfest.postgresql.org/47/2863/ that there isn't any test
covering all the backtrace related code - backtrace_functions GUC,
backtrace_on_internal_error GUC, set_backtrace(), backtrace(),
backtrace_symbols(). I came up with a test module covering these areas
https://commitfest.postgresql.org/47/4823/. I could make the TAP tests
pass on all the CF bot animals. Interestingly, the new code that gets
added for this thread can also be covered with it. Any thoughts are
welcome.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com