Re: More CppAsString2() in psql's describe.c

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: More CppAsString2() in psql's describe.c
Date: 2024-12-02 07:37:46
Message-ID: a93de185-187c-443e-b621-fd7ceb028501@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 28.11.24 07:34, Michael Paquier wrote:
> - "WHERE p.prokind = 'a'\n",
> + "WHERE p.prokind = " CppAsString2(PROKIND_AGGREGATE) "\n",

I noticed something here. If the symbol that is the argument of
CppAsString2() is not defined, maybe because there is a typo or because
the required header file is not included, then there is no compiler
diagnostic. Instead, the symbol is just used as is, which might then
result in an SQL error or go unnoticed if there is no test coverage.

Now, the same is technically true for the previous code with the
hardcoded character values, but I think at least something like

"WHERE p.prokind = 'a'\n",

is easier to eyeball for correctness, whereas, you know,
CppAsString2(PROPARALLEL_RESTRICTED), is quite something.

I think this would be more robust if it were written something like

"WHERE p.prokind = '%c'\n", PROKIND_AGGREGATE

(Moreover, the current structure assumes that the C character literal
syntax used by the PROKIND_* and other symbols happens to be the same as
the SQL string literal syntax required in those queries, which is just
an accident.)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-12-02 07:41:37 Re: altering a column's collation leaves an invalid foreign key
Previous Message Thomas Munro 2024-12-02 07:32:33 Re: Interrupts vs signals