Peter Eisentraut <peter@eisentraut.org> writes:
> 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.
Yeah, if memory serves we've actually been bit by that at least once.
But isn't there a way to improve the macro so this'd lead to an error?
> I think this would be more robust if it were written something like
> "WHERE p.prokind = '%c'\n", PROKIND_AGGREGATE
The problem is that you frequently have several of these in a
statement, which'd lead to a lot of %c items that readers have
to mentally match up with not-very-close-by printf arguments.
We already have that with translatable column headers, for instance,
and it's a pain in the rear for readability and maintainability.
I do not buy that this is a preferable answer.
> (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.)
So? There isn't much about C syntax that isn't an accident.
Neither literal syntax is going to change, so I don't see why
it's problematic to rely on them being the same.
regards, tom lane
