NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit13d53aa

committed

Doc/improve confusing, inefficient tests to locate CTID variable.

The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.clook buggy at first sight: they aren't checking that the varnomatches the table to be scanned. Actually they're safe becauseany Var in a scan-level qual must be for the correct table ...but if we're depending on that, it's pretty pointless to verifyvarlevelsup. (Besides which, varlevelsup is *always* zero atexecution, since we've flattened the rangetable long since.)Remove the useless varlevelsup check, and instead add somecommentary explaining why we don't need to check varno.Noted while fooling with a planner change that causes the orderof "t1.ctid = t2.ctid" to change in some tidscan.sql tests;I was briefly fooled into thinking there was a live bug here.

1 parent0e972f5 commit13d53aaCopy full SHA for 13d53aa

File tree

2 files changed

+14

-4

lines changed

src/backend/executor
- nodeTidrangescan.c
- nodeTidscan.c

2 files changed

+14

-4

lines changed

`‎src/backend/executor/nodeTidrangescan.c`

Lines changed: 7 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -25,11 +25,16 @@`
`25`	`25`	`#include"utils/rel.h"`
`26`	`26`
`27`	`27`
	`28`	`+/*`
	`29`	`+ * It's sufficient to check varattno to identify the CTID variable, as any`
	`30`	`+ * Var in the relation scan qual must be for our table. (Even if it's a`
	`31`	`+ * parameterized scan referencing some other table's CTID, the other table's`
	`32`	`+ * Var would have become a Param by the time it gets here.)`
	`33`	`+ */`
`28`	`34`	`#defineIsCTIDVar(node) \`
`29`	`35`	`((node) != NULL && \`
`30`	`36`	`IsA((node), Var) && \`
`31`		`- ((Var *) (node))->varattno == SelfItemPointerAttributeNumber && \`
`32`		`- ((Var *) (node))->varlevelsup == 0)`
	`37`	`+ ((Var *) (node))->varattno == SelfItemPointerAttributeNumber)`
`33`	`38`
`34`	`39`	`typedefenum`
`35`	`40`	`{`

`‎src/backend/executor/nodeTidscan.c`

Lines changed: 7 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -35,11 +35,16 @@`
`35`	`35`	`#include"utils/rel.h"`
`36`	`36`
`37`	`37`
	`38`	`+/*`
	`39`	`+ * It's sufficient to check varattno to identify the CTID variable, as any`
	`40`	`+ * Var in the relation scan qual must be for our table. (Even if it's a`
	`41`	`+ * parameterized scan referencing some other table's CTID, the other table's`
	`42`	`+ * Var would have become a Param by the time it gets here.)`
	`43`	`+ */`
`38`	`44`	`#defineIsCTIDVar(node) \`
`39`	`45`	`((node) != NULL && \`
`40`	`46`	`IsA((node), Var) && \`
`41`		`- ((Var *) (node))->varattno == SelfItemPointerAttributeNumber && \`
`42`		`- ((Var *) (node))->varlevelsup == 0)`
	`47`	`+ ((Var *) (node))->varattno == SelfItemPointerAttributeNumber)`
`43`	`48`
`44`	`49`	`/* one element in tss_tidexprs */`
`45`	`50`	`typedefstructTidExpr`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit13d53aa

File tree

2 files changed

2 files changed

`‎src/backend/executor/nodeTidrangescan.c`

`‎src/backend/executor/nodeTidscan.c`

0 commit comments