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

Commitdfc843d

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 parentac9099f commitdfc843d

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,19 @@ spgNewHeapItem(SpGistScanOpaque so, int level, ItemPointer heapPtr,
463463

464464
item->level=level;
465465
item->heapPtr=*heapPtr;
466-
/* copy value to queue cxt out of tmp cxt */
467-
/* caution: "leafValue" is of type attType not leafType */
468-
item->value=isnull ? (Datum)0 :
469-
datumCopy(leafValue,so->state.attType.attbyval,
470-
so->state.attType.attlen);
466+
467+
/*
468+
* If we need the reconstructed value, copy it to queue cxt out of tmp
469+
* cxt. Caution: the leaf_consistent method may not have supplied a value
470+
* if we didn't ask it to, and mildly-broken methods might supply one of
471+
* the wrong type. The correct leafValue type is attType not leafType.
472+
*/
473+
if (so->want_itup)
474+
item->value=isnull ? (Datum)0 :
475+
datumCopy(leafValue,so->state.attType.attbyval,
476+
so->state.attType.attlen);
477+
else
478+
item->value= (Datum)0;
471479
item->traversalValue=NULL;
472480
item->isLeaf= true;
473481
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