Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit48b319e

Browse files
committed
Fix more confusion in SP-GiST.
spg_box_quad_leaf_consistent unconditionally returned the leafdatum as leafValue, even though in its usage for poly_ops thatvalue is of completely the wrong type.In versions before 12, that was harmless because the core code didnothing with leafValue in non-index-only scans ... but since commit2a63683, if we were doing a KNN-style scan, spgNewHeapItem wouldunconditionally try to copy the value using the wrong datatypeparameters. Said copying is a waste of time and space if we're notgoing to return the data, but it accidentally failed to fail untilI fixed the datatype confusion inac9099f.Hence, change spgNewHeapItem to not copy the datum unless we'reactually going to return it later. This saves cycles and dodgesthe question of whether lossy opclasses are returning the righttype. Also change spg_box_quad_leaf_consistent to not returndata that might be of the wrong type, as insurance againstsomebody introducing a similar bug into the core code in future.It seems like a good idea to back-patch these two changes intov12 and v13, although I'm afraid to change spgNewHeapItem'smistaken idea of which datatype to use in those branches.Per buildfarm results fromac9099f.Discussion:https://postgr.es/m/3728741.1617381471@sss.pgh.pa.us
1 parent6972d91 commit48b319e

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

‎src/backend/access/spgist/spgscan.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,22 @@ spgNewHeapItem(SpGistScanOpaque so, int level, ItemPointer heapPtr,
451451

452452
item->level=level;
453453
item->heapPtr=*heapPtr;
454-
/* copy value to queue cxt out of tmp cxt */
455-
item->value=isnull ? (Datum)0 :
456-
datumCopy(leafValue,so->state.attLeafType.attbyval,
457-
so->state.attLeafType.attlen);
454+
455+
/*
456+
* If we need the reconstructed value, copy it to queue cxt out of tmp
457+
* cxt. Caution: the leaf_consistent method may not have supplied a value
458+
* if we didn't ask it to, and mildly-broken methods might supply one of
459+
* the wrong type. Also, while the correct leafValue type is attType not
460+
* leafType, pre-v14 Postgres versions have historically used attLeafType
461+
* here; let's not confuse matters even more by changing that in a minor
462+
* release.
463+
*/
464+
if (so->want_itup)
465+
item->value=isnull ? (Datum)0 :
466+
datumCopy(leafValue,so->state.attLeafType.attbyval,
467+
so->state.attLeafType.attlen);
468+
else
469+
item->value= (Datum)0;
458470
item->traversalValue=NULL;
459471
item->isLeaf= true;
460472
item->recheck=recheck;

‎src/backend/utils/adt/geo_spgist.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,13 @@ spg_box_quad_leaf_consistent(PG_FUNCTION_ARGS)
749749
/* All tests are exact. */
750750
out->recheck= false;
751751

752-
/* leafDatum is what it is... */
753-
out->leafValue=in->leafDatum;
752+
/*
753+
* Don't return leafValue unless told to; this is used for both box and
754+
* polygon opclasses, and in the latter case the leaf datum is not even of
755+
* the right type to return.
756+
*/
757+
if (in->returnData)
758+
out->leafValue=leaf;
754759

755760
/* Perform the required comparison(s) */
756761
for (i=0;i<in->nkeys;i++)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp