Re: Use more CppAsString2() in pg_amcheck.c - Mailing list pgsql-hackers
From | Karina Litskevich |
---|---|
Subject | Re: Use more CppAsString2() in pg_amcheck.c |
Date | |
Msg-id | CACiT8iYzjeiziq+pWcRH=fT4g5zSzr5A88QQsRCuM659NQCwrQ@mail.gmail.com Whole thread Raw |
In response to | Re: Use more CppAsString2() in pg_amcheck.c (Nathan Bossart <nathandbossart@gmail.com>) |
List | pgsql-hackers |
On Sat, Oct 19, 2024 at 4:17 AM Michael Paquier <michael@paquier.xyz> wrote: > Removed that in the v2 attached. Hi Michael, The patch looks good to me. I'd like to suggest discussing a little cosmetic change in the affected lines. Compare the following. Lines 2095-2098: appendPQExpBuffer(&sql, " AND c.relam = %u" " AND c.relkind = " CppAsString2(RELKIND_INDEX) ")", BTREE_AM_OID); Lines 2058-2061: appendPQExpBuffer(&sql, " AND c.relam = %u " "AND c.relkind = " CppAsString2(RELKIND_INDEX), BTREE_AM_OID); They are not identical: space before AND vs space at the end of the previous line. I'd say that it would be better if they were identical. Personally, I prefer the one with a space before AND. Though we have two more similar cases in lines 1974-2001: if (opts.allrel) appendPQExpBuffer(&sql, " AND c.relam = %u " "AND c.relkind IN (" CppAsString2(RELKIND_RELATION) ", " CppAsString2(RELKIND_SEQUENCE) ", " CppAsString2(RELKIND_MATVIEW) ", " CppAsString2(RELKIND_TOASTVALUE) ") " "AND c.relnamespace != %u", HEAP_TABLE_AM_OID, PG_TOAST_NAMESPACE); else appendPQExpBuffer(&sql, " AND c.relam IN (%u, %u)" "AND c.relkind IN (" CppAsString2(RELKIND_RELATION) ", " CppAsString2(RELKIND_SEQUENCE) ", " CppAsString2(RELKIND_MATVIEW) ", " CppAsString2(RELKIND_TOASTVALUE) ", " CppAsString2(RELKIND_INDEX) ") " "AND ((c.relam = %u AND c.relkind IN (" CppAsString2(RELKIND_RELATION) ", " CppAsString2(RELKIND_SEQUENCE) ", " CppAsString2(RELKIND_MATVIEW) ", " CppAsString2(RELKIND_TOASTVALUE) ")) OR " "(c.relam = %u AND c.relkind = " CppAsString2(RELKIND_INDEX) "))", HEAP_TABLE_AM_OID, BTREE_AM_OID, HEAP_TABLE_AM_OID, BTREE_AM_OID); And I'm not sure how they should be handled if we choose a space before AND. Does the following still look fine? if (opts.allrel) appendPQExpBuffer(&sql, " AND c.relam = %u" " AND c.relkind IN (" CppAsString2(RELKIND_RELATION) ", " CppAsString2(RELKIND_SEQUENCE) ", " CppAsString2(RELKIND_MATVIEW) ", " CppAsString2(RELKIND_TOASTVALUE) ")" " AND c.relnamespace != %u", HEAP_TABLE_AM_OID, PG_TOAST_NAMESPACE); What do you think? Best regards, Karina Litskevich Postgres Professional: http://postgrespro.com/
pgsql-hackers by date: