diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-10-25 17:35:19 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-10-25 17:35:19 -0400 |
commit | 13d53aa7a83383c9d1343e7d725e615f8678aea8 (patch) | |
tree | 1bd462e7b0ca9171de592bde2f5ae2c1f7f67673 | |
parent | 0e972f50fdc9f33b01d02bbcc2f26aa32f1c58ab (diff) | |
download | postgresql-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.c | 9 | ||||
-rw-r--r-- | 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 |