NotificationsYou must be signed in to change notification settings
Fork5k
Star17.9k

Commit91715e8

committed

Fix management of fn_extra caching during repeated GiST index scans.

Commitd22a09d introduced official supportfor GiST consistentFns that want to cache data using the FmgrInfo fn_extrapointer: the idea was to preserve the cached values across gistrescan(),whereas formerly they'd been leaked. However, there was an oversight inthat, namely that multiple scan keys might reference the same column'sconsistentFn; the code would result in propagating the same cache valueinto multiple scan keys, resulting in crashes or wrong answers. Use aseparate array instead to ensure that each scan key keeps its own state.Per bug #8143 from Joel Roller. Back-patch to 9.2 where the bug wasintroduced.

1 parentcda7ace commit91715e8Copy full SHA for 91715e8

File tree

1 file changed

+29

-22

lines changed

src/backend/access/gist
- gistscan.c

1 file changed

+29

-22

lines changed

`‎src/backend/access/gist/gistscan.c`

Lines changed: 29 additions & 22 deletions

Original file line number	Diff line number	Diff line change
`@@ -202,20 +202,18 @@ gistrescan(PG_FUNCTION_ARGS)`
`202`	`202`	`/* Update scan key, if a new one is given */`
`203`	`203`	`if (key && scan->numberOfKeys > 0)`
`204`	`204`	`{`
	`205`	`+void **fn_extras = NULL;`
	`206`	`+`
`205`	`207`	`/*`
`206`	`208`	`* If this isn't the first time through, preserve the fn_extra`
`207`	`209`	`* pointers, so that if the consistentFns are using them to cache`
`208`	`210`	`* data, that data is not leaked across a rescan.`
`209`	`211`	`*/`
`210`	`212`	`if (!first_time)`
`211`	`213`	`{`
	`214`	`+fn_extras = (void *) palloc(scan->numberOfKeys sizeof(void *));`
`212`	`215`	`for (i = 0; i < scan->numberOfKeys; i++)`
`213`		`-{`
`214`		`-ScanKeyskey = scan->keyData + i;`
`215`		`-`
`216`		`-so->giststate->consistentFn[skey->sk_attno - 1].fn_extra =`
`217`		`-skey->sk_func.fn_extra;`
`218`		`-}`
	`216`	`+fn_extras[i] = scan->keyData[i].sk_func.fn_extra;`
`219`	`217`	`}`
`220`	`218`
`221`	`219`	`memmove(scan->keyData, key,`
`@@ -231,40 +229,43 @@ gistrescan(PG_FUNCTION_ARGS)`
`231`	`229`	`* Next, if any of keys is a NULL and that key is not marked with`
`232`	`230`	`* SK_SEARCHNULL/SK_SEARCHNOTNULL then nothing can be found (ie, we`
`233`	`231`	`* assume all indexable operators are strict).`
`234`		`- *`
`235`		`- * Note: we intentionally memcpy the FmgrInfo to sk_func rather than`
`236`		`- * using fmgr_info_copy. This is so that the fn_extra field gets`
`237`		`- * preserved across multiple rescans.`
`238`	`232`	`*/`
`239`	`233`	`so->qual_ok = true;`
`240`	`234`
`241`	`235`	`for (i = 0; i < scan->numberOfKeys; i++)`
`242`	`236`	`{`
`243`	`237`	`ScanKeyskey = scan->keyData + i;`
`244`	`238`
`245`		`-skey->sk_func = so->giststate->consistentFn[skey->sk_attno - 1];`
	`239`	`+fmgr_info_copy(&(skey->sk_func),`
	`240`	`+ &(so->giststate->consistentFn[skey->sk_attno - 1]),`
	`241`	`+ so->giststate->scanCxt);`
	`242`	`+`
	`243`	`+/* Restore prior fn_extra pointers, if not first time */`
	`244`	`+if (!first_time)`
	`245`	`+skey->sk_func.fn_extra = fn_extras[i];`
`246`	`246`
`247`	`247`	`if (skey->sk_flags & SK_ISNULL)`
`248`	`248`	`{`
`249`	`249`	`if (!(skey->sk_flags & (SK_SEARCHNULL \| SK_SEARCHNOTNULL)))`
`250`	`250`	`so->qual_ok = false;`
`251`	`251`	`}`
`252`	`252`	`}`
	`253`	`+`
	`254`	`+if (!first_time)`
	`255`	`+pfree(fn_extras);`
`253`	`256`	`}`
`254`	`257`
`255`	`258`	`/* Update order-by key, if a new one is given */`
`256`	`259`	`if (orderbys && scan->numberOfOrderBys > 0)`
`257`	`260`	`{`
	`261`	`+void **fn_extras = NULL;`
	`262`	`+`
`258`	`263`	`/* As above, preserve fn_extra if not first time through */`
`259`	`264`	`if (!first_time)`
`260`	`265`	`{`
	`266`	`+fn_extras = (void *) palloc(scan->numberOfOrderBys sizeof(void *));`
`261`	`267`	`for (i = 0; i < scan->numberOfOrderBys; i++)`
`262`		`-{`
`263`		`-ScanKeyskey = scan->orderByData + i;`
`264`		`-`
`265`		`-so->giststate->distanceFn[skey->sk_attno - 1].fn_extra =`
`266`		`-skey->sk_func.fn_extra;`
`267`		`-}`
	`268`	`+fn_extras[i] = scan->orderByData[i].sk_func.fn_extra;`
`268`	`269`	`}`
`269`	`270`
`270`	`271`	`memmove(scan->orderByData, orderbys,`
`@@ -276,21 +277,27 @@ gistrescan(PG_FUNCTION_ARGS)`
`276`	`277`	`* function in the form of its strategy number, which is available`
`277`	`278`	`* from the sk_strategy field, and its subtype from the sk_subtype`
`278`	`279`	`* field.`
`279`		`- *`
`280`		`- * See above comment about why we don't use fmgr_info_copy here.`
`281`	`280`	`*/`
`282`	`281`	`for (i = 0; i < scan->numberOfOrderBys; i++)`
`283`	`282`	`{`
`284`	`283`	`ScanKeyskey = scan->orderByData + i;`
`285`		`-`
`286`		`-skey->sk_func = so->giststate->distanceFn[skey->sk_attno - 1];`
	`284`	`+FmgrInfo *finfo = &(so->giststate->distanceFn[skey->sk_attno - 1]);`
`287`	`285`
`288`	`286`	`/* Check we actually have a distance function ... */`
`289`		`-if (!OidIsValid(skey->sk_func.fn_oid))`
	`287`	`+if (!OidIsValid(finfo->fn_oid))`
`290`	`288`	`elog(ERROR, "missing support function %d for attribute %d of index \"%s\"",`
`291`	`289`	`GIST_DISTANCE_PROC, skey->sk_attno,`
`292`	`290`	`RelationGetRelationName(scan->indexRelation));`
	`291`	`+`
	`292`	`+fmgr_info_copy(&(skey->sk_func), finfo, so->giststate->scanCxt);`
	`293`	`+`
	`294`	`+/* Restore prior fn_extra pointers, if not first time */`
	`295`	`+if (!first_time)`
	`296`	`+skey->sk_func.fn_extra = fn_extras[i];`
`293`	`297`	`}`
	`298`	`+`
	`299`	`+if (!first_time)`
	`300`	`+pfree(fn_extras);`
`294`	`301`	`}`
`295`	`302`
`296`	`303`	`PG_RETURN_VOID();`

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!

Commit91715e8

File tree

1 file changed

1 file changed

`‎src/backend/access/gist/gistscan.c`

0 commit comments