From 13d53aa7a83383c9d1343e7d725e615f8678aea8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 25 Oct 2022 17:35:19 -0400 Subject: [PATCH] Doc/improve confusing, inefficient tests to locate CTID variable. The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.c look buggy at first sight: they aren't checking that the varno matches the table to be scanned. Actually they're safe because any Var in a scan-level qual must be for the correct table ... but if we're depending on that, it's pretty pointless to verify varlevelsup. (Besides which, varlevelsup is *always* zero at execution, since we've flattened the rangetable long since.) Remove the useless varlevelsup check, and instead add some commentary explaining why we don't need to check varno. Noted while fooling with a planner change that causes the order of "t1.ctid = t2.ctid" to change in some tidscan.sql tests; I was briefly fooled into thinking there was a live bug here. --- src/backend/executor/nodeTidrangescan.c | 9 +++++++-- src/backend/executor/nodeTidscan.c | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c index d5bf1be787f..0455819e73c 100644 --- a/src/backend/executor/nodeTidrangescan.c +++ b/src/backend/executor/nodeTidrangescan.c @@ -25,11 +25,16 @@ #include "utils/rel.h" +/* + * It's sufficient to check varattno to identify the CTID variable, as any + * Var in the relation scan qual must be for our table. (Even if it's a + * parameterized scan referencing some other table's CTID, the other table's + * Var would have become a Param by the time it gets here.) + */ #define IsCTIDVar(node) \ ((node) != NULL && \ IsA((node), Var) && \ - ((Var *) (node))->varattno == SelfItemPointerAttributeNumber && \ - ((Var *) (node))->varlevelsup == 0) + ((Var *) (node))->varattno == SelfItemPointerAttributeNumber) typedef enum { diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index a1c6325d649..ee4c0d1ea01 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -35,11 +35,16 @@ #include "utils/rel.h" +/* + * It's sufficient to check varattno to identify the CTID variable, as any + * Var in the relation scan qual must be for our table. (Even if it's a + * parameterized scan referencing some other table's CTID, the other table's + * Var would have become a Param by the time it gets here.) + */ #define IsCTIDVar(node) \ ((node) != NULL && \ IsA((node), Var) && \ - ((Var *) (node))->varattno == SelfItemPointerAttributeNumber && \ - ((Var *) (node))->varlevelsup == 0) + ((Var *) (node))->varattno == SelfItemPointerAttributeNumber) /* one element in tss_tidexprs */ typedef struct TidExpr