[postgis-devel] Strange result about spatial indexes?
Tom Lane
tgl at sss.pgh.pa.us
Fri Aug 14 21:38:24 PDT 2009
Paul Ramsey <pramsey at cleverelephant.ca> writes:
> Frankly, now that we know this trick, since most of our index calls
> are now hidden inside SQL facades (eg ST_Intersects()), we can further
> wrap the index operations in no-op immutable functions to force this
> behavior. And add an ST_MBRIntersects() function which does nothing
> more than toss in a wrapper function. It's a hack, but c'est la vie.
> Most of the time the effect won't be 10:1 like this example data set
> showed -- this data just happens to have some high degrees of overlap,
> so the inefficiency gets lots of exercise.
This is definitely the exact same issue that Mark Cave-Ayland complained
about last year:
http://archives.postgresql.org/pgsql-hackers/2008-06/msg00384.php
The proposal I had to fix it
http://archives.postgresql.org/pgsql-hackers/2008-06/msg00709.php
more or less crashed and burned, because it was too complicated and
didn't buy enough in return. But I wonder if we couldn't fix your
problem with a less ambitious, less invasive approach, like the
attached. Which is actually a three-statement patch, but it looks
bigger because I had to rearrange the existing code a trifle.
This is against CVS HEAD, but applies cleanly to 8.4, and could be
applied to 8.3 with only minor work.
regards, tom lane
-------------- next part --------------
Index: src/backend/executor/nodeIndexscan.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeIndexscan.c,v
retrieving revision 1.133
diff -c -r1.133 nodeIndexscan.c
*** src/backend/executor/nodeIndexscan.c 18 Jul 2009 19:15:41 -0000 1.133
--- src/backend/executor/nodeIndexscan.c 15 Aug 2009 04:28:55 -0000
***************
*** 237,244 ****
--- 237,247 ----
ExecIndexEvalRuntimeKeys(ExprContext *econtext,
IndexRuntimeKeyInfo *runtimeKeys, int numRuntimeKeys)
{
+ MemoryContext oldContext;
int j;
+ oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
+
for (j = 0; j < numRuntimeKeys; j++)
{
ScanKey scan_key = runtimeKeys[j].scan_key;
***************
*** 256,273 ****
* econtext->ecxt_per_tuple_memory. We assume that the outer tuple
* will stay put throughout our scan. If this is wrong, we could copy
* the result into our context explicitly, but I think that's not
! * necessary...
*/
! scanvalue = ExecEvalExprSwitchContext(key_expr,
! econtext,
! &isNull,
! NULL);
! scan_key->sk_argument = scanvalue;
if (isNull)
scan_key->sk_flags |= SK_ISNULL;
else
scan_key->sk_flags &= ~SK_ISNULL;
}
}
/*
--- 259,290 ----
* econtext->ecxt_per_tuple_memory. We assume that the outer tuple
* will stay put throughout our scan. If this is wrong, we could copy
* the result into our context explicitly, but I think that's not
! * necessary.
! *
! * It's also entirely possible that the result of the eval is a
! * toasted value. In this case we should forcibly detoast it,
! * to avoid repeat detoastings each time the value is examined
! * by an index support function.
*/
! scanvalue = ExecEvalExpr(key_expr,
! econtext,
! &isNull,
! NULL);
if (isNull)
+ {
+ scan_key->sk_argument = scanvalue;
scan_key->sk_flags |= SK_ISNULL;
+ }
else
+ {
+ if (runtimeKeys[j].key_toastable)
+ scanvalue = PointerGetDatum(PG_DETOAST_DATUM(scanvalue));
+ scan_key->sk_argument = scanvalue;
scan_key->sk_flags &= ~SK_ISNULL;
+ }
}
+
+ MemoryContextSwitchTo(oldContext);
}
/*
***************
*** 795,800 ****
--- 812,819 ----
runtime_keys[n_runtime_keys].scan_key = this_scan_key;
runtime_keys[n_runtime_keys].key_expr =
ExecInitExpr(rightop, planstate);
+ runtime_keys[n_runtime_keys].key_toastable =
+ TypeIsToastable(op_righttype);
n_runtime_keys++;
scanvalue = (Datum) 0;
}
***************
*** 844,849 ****
--- 863,893 ----
varattno = ((Var *) leftop)->varattno;
/*
+ * We have to look up the operator's associated btree support
+ * function
+ */
+ opno = lfirst_oid(opnos_cell);
+ opnos_cell = lnext(opnos_cell);
+
+ if (index->rd_rel->relam != BTREE_AM_OID ||
+ varattno < 1 || varattno > index->rd_index->indnatts)
+ elog(ERROR, "bogus RowCompare index qualification");
+ opfamily = index->rd_opfamily[varattno - 1];
+
+ get_op_opfamily_properties(opno, opfamily,
+ &op_strategy,
+ &op_lefttype,
+ &op_righttype);
+
+ if (op_strategy != rc->rctype)
+ elog(ERROR, "RowCompare index qualification contains wrong operator");
+
+ opfuncid = get_opfamily_proc(opfamily,
+ op_lefttype,
+ op_righttype,
+ BTORDER_PROC);
+
+ /*
* rightop is the constant or variable comparison value
*/
rightop = (Expr *) lfirst(rargs_cell);
***************
*** 867,902 ****
runtime_keys[n_runtime_keys].scan_key = this_sub_key;
runtime_keys[n_runtime_keys].key_expr =
ExecInitExpr(rightop, planstate);
n_runtime_keys++;
scanvalue = (Datum) 0;
}
/*
- * We have to look up the operator's associated btree support
- * function
- */
- opno = lfirst_oid(opnos_cell);
- opnos_cell = lnext(opnos_cell);
-
- if (index->rd_rel->relam != BTREE_AM_OID ||
- varattno < 1 || varattno > index->rd_index->indnatts)
- elog(ERROR, "bogus RowCompare index qualification");
- opfamily = index->rd_opfamily[varattno - 1];
-
- get_op_opfamily_properties(opno, opfamily,
- &op_strategy,
- &op_lefttype,
- &op_righttype);
-
- if (op_strategy != rc->rctype)
- elog(ERROR, "RowCompare index qualification contains wrong operator");
-
- opfuncid = get_opfamily_proc(opfamily,
- op_lefttype,
- op_righttype,
- BTORDER_PROC);
-
- /*
* initialize the subsidiary scan key's fields appropriately
*/
ScanKeyEntryInitialize(this_sub_key,
--- 911,923 ----
runtime_keys[n_runtime_keys].scan_key = this_sub_key;
runtime_keys[n_runtime_keys].key_expr =
ExecInitExpr(rightop, planstate);
+ runtime_keys[n_runtime_keys].key_toastable =
+ TypeIsToastable(op_righttype);
n_runtime_keys++;
scanvalue = (Datum) 0;
}
/*
* initialize the subsidiary scan key's fields appropriately
*/
ScanKeyEntryInitialize(this_sub_key,
Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.206
diff -c -r1.206 execnodes.h
*** src/include/nodes/execnodes.h 6 Aug 2009 20:44:31 -0000 1.206
--- src/include/nodes/execnodes.h 15 Aug 2009 04:28:55 -0000
***************
*** 1084,1089 ****
--- 1084,1090 ----
{
ScanKey scan_key; /* scankey to put value into */
ExprState *key_expr; /* expr to evaluate to get value */
+ bool key_toastable; /* is expr's result a toastable datatype? */
} IndexRuntimeKeyInfo;
typedef struct
More information about the postgis-devel
mailing list