aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2022-10-25 17:35:19 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2022-10-25 17:35:19 -0400
commit13d53aa7a83383c9d1343e7d725e615f8678aea8 (patch)
tree1bd462e7b0ca9171de592bde2f5ae2c1f7f67673
parent0e972f50fdc9f33b01d02bbcc2f26aa32f1c58ab (diff)
downloadpostgresql-13d53aa7a83383c9d1343e7d725e615f8678aea8.tar.gz
postgresql-13d53aa7a83383c9d1343e7d725e615f8678aea8.zip
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.
-rw-r--r--src/backend/executor/nodeTidrangescan.c9
-rw-r--r--src/backend/executor/nodeTidscan.c9
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