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
From 7ef323354f1710ee755876132ae719b02c3e0468 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Sat, 19 Oct 2024 10:15:53 +0900
Subject: [PATCH v2] Use more CppAsString2() in pg_amcheck.c
---
src/bin/pg_amcheck/pg_amcheck.c | 42 +++++++++++++++++++++++----------
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 0c05cf58bc..27a7d5e925 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -16,6 +16,7 @@
#include <time.h>
#include "catalog/pg_am_d.h"
+#include "catalog/pg_class_d.h"
#include "catalog/pg_namespace_d.h"
#include "common/logging.h"
#include "common/username.h"
@@ -857,7 +858,7 @@ prepare_heap_command(PQExpBuffer sql, RelationInfo *rel, PGconn *conn)
appendPQExpBuffer(sql,
"\n) v WHERE c.oid = %u "
- "AND c.relpersistence != 't'",
+ "AND c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP),
rel->reloid);
}
@@ -890,7 +891,7 @@ prepare_btree_command(PQExpBuffer sql, RelationInfo *rel, PGconn *conn)
"\nFROM pg_catalog.pg_class c, pg_catalog.pg_index i "
"WHERE c.oid = %u "
"AND c.oid = i.indexrelid "
- "AND c.relpersistence != 't' "
+ "AND c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP) " "
"AND i.indisready AND i.indisvalid AND i.indislive",
rel->datinfo->amcheck_schema,
(opts.heapallindexed ? "true" : "false"),
@@ -905,7 +906,7 @@ prepare_btree_command(PQExpBuffer sql, RelationInfo *rel, PGconn *conn)
"\nFROM pg_catalog.pg_class c, pg_catalog.pg_index i "
"WHERE c.oid = %u "
"AND c.oid = i.indexrelid "
- "AND c.relpersistence != 't' "
+ "AND c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP) " "
"AND i.indisready AND i.indisvalid AND i.indislive",
rel->datinfo->amcheck_schema,
(opts.heapallindexed ? "true" : "false"),
@@ -1952,7 +1953,8 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
* until firing off the amcheck command, as the state of an index may
* change by then.
*/
- appendPQExpBufferStr(&sql, "\nWHERE c.relpersistence != 't'");
+ appendPQExpBufferStr(&sql, "\nWHERE c.relpersistence != "
+ CppAsString2(RELPERSISTENCE_TEMP));
if (opts.excludetbl || opts.excludeidx || opts.excludensp)
appendPQExpBufferStr(&sql, "\nAND ep.pattern_id IS NULL");
@@ -1972,15 +1974,29 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
if (opts.allrel)
appendPQExpBuffer(&sql,
" AND c.relam = %u "
- "AND c.relkind IN ('r', 'S', 'm', 't') "
+ "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 ('r', 'S', 'm', 't', 'i') "
- "AND ((c.relam = %u AND c.relkind IN ('r', 'S', 'm', 't')) OR "
- "(c.relam = %u AND c.relkind = 'i'))",
+ "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);
@@ -2007,7 +2023,7 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
"\nAND (t.relname ~ ep.rel_regex OR ep.rel_regex IS NULL)"
"\nAND ep.heap_only"
"\nWHERE ep.pattern_id IS NULL"
- "\nAND t.relpersistence != 't'");
+ "\nAND t.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP));
appendPQExpBufferStr(&sql,
"\n)");
}
@@ -2026,7 +2042,7 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
"ON r.oid = i.indrelid "
"INNER JOIN pg_catalog.pg_class c "
"ON i.indexrelid = c.oid "
- "AND c.relpersistence != 't'");
+ "AND c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP));
if (opts.excludeidx || opts.excludensp)
appendPQExpBufferStr(&sql,
"\nINNER JOIN pg_catalog.pg_namespace n "
@@ -2041,7 +2057,7 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
"\nWHERE true");
appendPQExpBuffer(&sql,
" AND c.relam = %u "
- "AND c.relkind = 'i'",
+ "AND c.relkind = " CppAsString2(RELKIND_INDEX),
BTREE_AM_OID);
if (opts.no_toast_expansion)
appendPQExpBuffer(&sql,
@@ -2065,7 +2081,7 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
"ON t.oid = i.indrelid"
"\nINNER JOIN pg_catalog.pg_class c "
"ON i.indexrelid = c.oid "
- "AND c.relpersistence != 't'");
+ "AND c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP));
if (opts.excludeidx)
appendPQExpBufferStr(&sql,
"\nLEFT OUTER JOIN exclude_pat ep "
@@ -2078,7 +2094,7 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
"\nWHERE true");
appendPQExpBuffer(&sql,
" AND c.relam = %u"
- " AND c.relkind = 'i')",
+ " AND c.relkind = " CppAsString2(RELKIND_INDEX) ")",
BTREE_AM_OID);
}
--
2.45.2
signature.asc
Description: PGP signature
