| From: | Michael Paquier <michael(at)paquier(dot)xyz> | 
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> | 
| Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> | 
| Subject: | Re: Use more CppAsString2() in pg_amcheck.c | 
| Date: | 2024-10-19 01:16:40 | 
| Message-ID: | ZxMIeKUwwbfNEMSh@paquier.xyz | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
On Fri, Oct 18, 2024 at 12:22:26PM -0500, Nathan Bossart wrote:
> On Fri, Oct 18, 2024 at 06:50:50PM +0900, Michael Paquier wrote:
>> -	appendPQExpBufferStr(&sql, "\nWHERE c.relpersistence != 't'");
>> +	appendPQExpBufferStr(&sql, "\nWHERE c.relpersistence != "
>> +						 CppAsString2(RELPERSISTENCE_TEMP) " ");
> 
> I think these whitespace changes may cause invalid statements to be
> produced in some code paths.  IMHO those should be reserved for a separate
> patch, anyway.
I may be missing something, of course, but I don't quite see how this
matters for this specific one.  The possible paths after the temporary
persistence check involve a qual on ep.pattern_id, then an addition
based on opts.allrel.  You are right that the extra whitespace after
the RELPERSISTENCE_TEMP bit makes little sense.
Removed that in the v2 attached.
--
Michael
| Attachment | Content-Type | Size | 
|---|---|---|
| v2-0001-Use-more-CppAsString2-in-pg_amcheck.c.patch | text/x-diff | 5.7 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2024-10-19 01:32:56 | Re: [BUG FIX] Fix validation of COPY options FORCE_NOT_NULL/FORCE_NULL | 
| Previous Message | Corey Huinker | 2024-10-19 01:00:16 | Re: Statistics Import and Export |