NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit6401583

committed

Call set_rel_pathlist_hook before generate_gather_paths, not after.

The previous ordering of these steps satisfied the nominal requirementthat set_rel_pathlist_hook could editorialize on the whole set of Pathsconstructed for a base relation. In practice, though, trying to changethe set of partial paths was impossible. Adding one didn't work because(a) it was too late to be included in Gather paths made by the core code,and (b) calling add_partial_path after generate_gather_paths is unsafe,because it might try to delete a path it thinks is dominated, but thatis already embedded in some Gather path(s). Nor could the hook safelyremove partial paths, for the same reason that they might already beembedded in Gathers.Better to call extensions first, let them add partial paths as desired,and then gather. In v11 and up, we already doubled down on that orderingby postponing gathering even further for single-relation queries; so evenif the hook wished to editorialize on Gather path construction, it couldnot.Report and patch by KaiGai Kohei. Back-patch to 9.6 where Gather pathswere added.Discussion:https://postgr.es/m/CAOP8fzahwpKJRTVVTqo2AE=mDTz_efVzV6Get_0=U3SO+-ha1A@mail.gmail.com

1 parent4446565 commit6401583Copy full SHA for 6401583

File tree

2 files changed

+17

-13

lines changed

doc/src/sgml
- custom-scan.sgml
src/backend/optimizer/path
- allpaths.c

2 files changed

+17

-13

lines changed

`‎doc/src/sgml/custom-scan.sgml`

Lines changed: 3 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,9 @@`
`37`	`37`	`<para>`
`38`	`38`	`A custom scan provider will typically add paths for a base relation by`
`39`	`39`	`setting the following hook, which is called after the core code has`
`40`		`- generated what it believes to be the complete and correct set of access`
`41`		`- paths for the relation.`
	`40`	`+ generated all the access paths it can for the relation (except for`
	`41`	`+ Gather paths, which are made after this call so that they can use`
	`42`	`+ partial paths added by the hook):`
`42`	`43`	`<programlisting>`
`43`	`44`	`typedef void (set_rel_pathlist_hook_type) (PlannerInfo root,`
`44`	`45`	`RelOptInfo *rel,`

`‎src/backend/optimizer/path/allpaths.c`

Lines changed: 14 additions & 11 deletions

Original file line number	Diff line number	Diff line change
`@@ -529,31 +529,34 @@ set_rel_pathlist(PlannerInfo root, RelOptInfo rel,`
`529`	`529`	`}`
`530`	`530`	`}`
`531`	`531`
	`532`	`+/*`
	`533`	`+ * Allow a plugin to editorialize on the set of Paths for this base`
	`534`	`+ * relation. It could add new paths (such as CustomPaths) by calling`
	`535`	`+ * add_path(), or add_partial_path() if parallel aware. It could also`
	`536`	`+ * delete or modify paths added by the core code.`
	`537`	`+ */`
	`538`	`+if (set_rel_pathlist_hook)`
	`539`	`+(*set_rel_pathlist_hook) (root,rel,rti,rte);`
	`540`	`+`
`532`	`541`	`/*`
`533`	`542`	`* If this is a baserel, we should normally consider gathering any partial`
`534`		`- * paths we may have created for it.`
	`543`	`+ * paths we may have created for it. We have to do this after calling the`
	`544`	`+ * set_rel_pathlist_hook, else it cannot add partial paths to be included`
	`545`	`+ * here.`
`535`	`546`	`*`
`536`	`547`	`* However, if this is an inheritance child, skip it. Otherwise, we could`
`537`	`548`	`* end up with a very large number of gather nodes, each trying to grab`
`538`	`549`	`* its own pool of workers. Instead, we'll consider gathering partial`
`539`	`550`	`* paths for the parent appendrel.`
`540`	`551`	`*`
`541`	`552`	`* Also, if this is the topmost scan/join rel (that is, the only baserel),`
`542`		`- * we postponethis until the final scan/jointargelist is available (see`
`543`		`- * grouping_planner).`
	`553`	`+ * we postponegathering until the final scan/jointargetlist is available`
	`554`	`+ *(seegrouping_planner).`
`544`	`555`	`*/`
`545`	`556`	`if (rel->reloptkind==RELOPT_BASEREL&&`
`546`	`557`	`bms_membership(root->all_baserels)!=BMS_SINGLETON)`
`547`	`558`	`generate_gather_paths(root,rel, false);`
`548`	`559`
`549`		`-/*`
`550`		`- * Allow a plugin to editorialize on the set of Paths for this base`
`551`		`- * relation. It could add new paths (such as CustomPaths) by calling`
`552`		`- * add_path(), or delete or modify paths added by the core code.`
`553`		`- */`
`554`		`-if (set_rel_pathlist_hook)`
`555`		`-(*set_rel_pathlist_hook) (root,rel,rti,rte);`
`556`		`-`
`557`	`560`	`/* Now find the cheapest of the paths for this rel */`
`558`	`561`	`set_cheapest(rel);`
`559`	`562`

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!

Commit6401583

File tree

2 files changed

2 files changed

`‎doc/src/sgml/custom-scan.sgml`

`‎src/backend/optimizer/path/allpaths.c`

0 commit comments