Обсуждение: Re: [HACKERS] Bug in 6.4 release
> Bruce Momjian wrote:
> >
> > >
> > > thplus=> select count(*) from envelope where state_id='H' or
> > > state_id='E';
> > > pqReadData() -- backend closed the channel unexpectedly.
> > > This probably means the backend terminated abnormally before or
> > > while processing the request.
> > > We have lost the connection to the backend, so further processing is
> > > impossible. Terminating.
> >
> > I need help with this one. Attached is a patch that also fails, but it
> > looks closer than the original code. The problem appears to be that I
> > can't get a slot that matches the items of the Var node I am trying to
> > evaluate. If I used one that matches the heap tuple, that fails,
> > because if the index is on the second column of the tuple, the attnum is
> > 1, while it is actually 2nd in the tuple slot.
> >
> > Does anyone know the executor well enough to find me that slot that
> > matches the Var node? I can't figure it out.
>
> Hi, Bruce!
>
> I'll take a look...
>
> Vadim
>
Thanks.
First, let me give you a reproducible example:create table test (x int, y text);insert into test values
(1,'fred');insertinto test values (1,'barney');insert into test select * from test;..repeat the above several
timescreateindex i_test on test(y);vacuum;select count(*) from test where y='fred' or y='barney';..crash
This only crashes if the OR field is not the first field in the table.
The error appears to be that the indexqual has a varno of 1, while the
heap location of the column in the above example is obviously 2. My
guess is that the varno matches the index varno, and not the heap varno.
I not think previous patch is wrong. The original code is better. You
have to compare the qualification against the actual heap row, because
you could be using a different index on the second OR than the first:
This new patch uses scankeys instead of throwing the indexqual to the
executor. This is probably more efficient, but I am not sure about the
other ramifications. It still fails.
Obviously, the idea that I want to use indexqual to test a heap tuple
was never anticipated by the original coders. I wonder why more people
have not reported OR problems?
Good luck. I am kind of stumped on this.
---------------------------------------------------------------------------
Index: src/backend/executor/nodeIndexscan.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/executor/nodeIndexscan.c,v
retrieving revision 1.27
diff -c -r1.27 nodeIndexscan.c
*** nodeIndexscan.c 1998/09/01 04:28:32 1.27
--- nodeIndexscan.c 1998/11/22 05:30:40
***************
*** 39,44 ****
--- 39,45 ---- #include "access/skey.h" #include "access/heapam.h"
+ #include "access/valid.h" #include "access/genam.h" #include "utils/palloc.h" #include "utils/mcxt.h"
***************
*** 95,101 **** TupleTableSlot *slot; Buffer buffer = InvalidBuffer; int numIndices;
! /* ---------------- * extract necessary information from index scan node * ----------------
--- 96,104 ---- TupleTableSlot *slot; Buffer buffer = InvalidBuffer; int numIndices;
! int *numScanKeys;
! ScanKey *scanKeys;
! /* ---------------- * extract necessary information from index scan node * ----------------
***************
*** 109,114 ****
--- 112,119 ---- heapRelation = scanstate->css_currentRelation; numIndices = indexstate->iss_NumIndices;
slot= scanstate->css_ScanTupleSlot;
+ numScanKeys = indexstate->iss_NumScanKeys;
+ scanKeys = indexstate->iss_ScanKeys; /* ---------------- * ok, now that we have what we need, fetch
anindex tuple.
***************
*** 153,161 **** for (prev_index = 0; prev_index < indexstate->iss_IndexPtr;
prev_index++) {
! scanstate->cstate.cs_ExprContext->ecxt_scantuple = slot;
! if (ExecQual(nth(prev_index, node->indxqual),
! scanstate->cstate.cs_ExprContext)) {
prev_matches= true; break;
--- 158,169 ---- for (prev_index = 0; prev_index < indexstate->iss_IndexPtr;
prev_index++) {
! bool result;
!
! HeapKeyTest(tuple, RelationGetDescr(heapRelation),
! numScanKeys[prev_index],
! scanKeys[prev_index], result);
! if (result) { prev_matches = true;
break;
-- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
Bruce Momjian wrote:
>
> This new patch uses scankeys instead of throwing the indexqual to the
> executor. This is probably more efficient, but I am not sure about the
> other ramifications. It still fails.
This wouldn't handle functional indices in OR...
So, I added indexqualorig list to the IndexScan node:
indexqual = fix_indxqual_references (indexqualorig)
- indxqualorig' Var-s references heap tuple...
Regression tests are ok.
Patch made for CURRENT tree - let me know if there will
be problems with 6.4...
It's better to gmake clean in backend dir...
Patch also fixes EXPLAIN for indices in OR: all indices
used are explained now.
Vadim*** src/include/nodes/plannodes.h.orig Sun Nov 22 15:54:23 1998
--- src/include/nodes/plannodes.h Sun Nov 22 15:58:02 1998
***************
*** 174,179 ****
--- 174,180 ----
Scan scan;
List *indxid;
List *indxqual;
+ List *indxqualorig;
IndexScanState *indxstate;
} IndexScan;
*** src/backend/commands/explain.c.orig Sun Nov 22 16:36:14 1998
--- src/backend/commands/explain.c Sun Nov 22 16:40:27 1998
***************
*** 223,231 ****
{
case T_IndexScan:
appendStringInfo(str, " using ");
! l = ((IndexScan *) plan)->indxid;
! relation = RelationIdCacheGetRelation((int) lfirst(l));
! appendStringInfo(str, (RelationGetRelationName(relation))->data);
case T_SeqScan:
if (((Scan *) plan)->scanrelid > 0)
{
--- 223,236 ----
{
case T_IndexScan:
appendStringInfo(str, " using ");
! i = 0;
! foreach (l, ((IndexScan *) plan)->indxid)
! {
! relation = RelationIdCacheGetRelation((int) lfirst(l));
! if (++i > 1)
! appendStringInfo(str, ", ");
! appendStringInfo(str, (RelationGetRelationName(relation))->data);
! }
case T_SeqScan:
if (((Scan *) plan)->scanrelid > 0)
{
*** src/backend/executor/nodeIndexscan.c.orig Sun Nov 22 16:00:23 1998
--- src/backend/executor/nodeIndexscan.c Sun Nov 22 15:59:10 1998
***************
*** 154,160 ****
prev_index++)
{
scanstate->cstate.cs_ExprContext->ecxt_scantuple = slot;
! if (ExecQual(nth(prev_index, node->indxqual),
scanstate->cstate.cs_ExprContext))
{
prev_matches = true;
--- 154,160 ----
prev_index++)
{
scanstate->cstate.cs_ExprContext->ecxt_scantuple = slot;
! if (ExecQual(nth(prev_index, node->indxqualorig),
scanstate->cstate.cs_ExprContext))
{
prev_matches = true;
*** src/backend/nodes/copyfuncs.c.orig Sun Nov 22 16:05:20 1998
--- src/backend/nodes/copyfuncs.c Sun Nov 22 16:03:58 1998
***************
*** 247,252 ****
--- 247,253 ----
*/
newnode->indxid = listCopy(from->indxid);
Node_Copy(from, newnode, indxqual);
+ Node_Copy(from, newnode, indxqualorig);
Node_Copy(from, newnode, indxstate);
return newnode;
*** src/backend/nodes/outfuncs.c.orig Sun Nov 22 16:07:31 1998
--- src/backend/nodes/outfuncs.c Sun Nov 22 16:07:06 1998
***************
*** 517,522 ****
--- 517,525 ----
appendStringInfo(str, " :indxqual ");
_outNode(str, node->indxqual);
+ appendStringInfo(str, " :indxqualorig ");
+ _outNode(str, node->indxqualorig);
+
}
/*
*** src/backend/nodes/readfuncs.c.orig Sun Nov 22 16:09:41 1998
--- src/backend/nodes/readfuncs.c Sun Nov 22 16:09:08 1998
***************
*** 546,551 ****
--- 546,554 ----
token = lsptok(NULL, &length); /* eat :indxqual */
local_node->indxqual = nodeRead(true); /* now read it */
+ token = lsptok(NULL, &length); /* eat :indxqualorig */
+ local_node->indxqualorig = nodeRead(true); /* now read it */
+
return local_node;
}
*** src/backend/optimizer/plan/createplan.c.orig Sun Nov 22 16:00:59 1998
--- src/backend/optimizer/plan/createplan.c Sun Nov 22 16:02:41 1998
***************
*** 63,69 ****
static Temp *make_temp(List *tlist, List *keys, Oid *operators,
Plan *plan_node, int temptype);
static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
! List *indxid, List *indxqual, Cost cost);
static NestLoop *make_nestloop(List *qptlist, List *qpqual, Plan *lefttree,
Plan *righttree);
static HashJoin *make_hashjoin(List *tlist, List *qpqual,
--- 63,69 ----
static Temp *make_temp(List *tlist, List *keys, Oid *operators,
Plan *plan_node, int temptype);
static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
! List *indxid, List *indxqual, List *indxqualorig, Cost cost);
static NestLoop *make_nestloop(List *qptlist, List *qpqual, Plan *lefttree,
Plan *righttree);
static HashJoin *make_hashjoin(List *tlist, List *qpqual,
***************
*** 405,410 ****
--- 405,411 ----
lfirsti(best_path->path.parent->relids),
best_path->indexid,
fixed_indxqual,
+ indxqual,
best_path->path.path_cost);
return scan_node;
***************
*** 937,942 ****
--- 938,944 ----
Index scanrelid,
List *indxid,
List *indxqual,
+ List *indxqualorig,
Cost cost)
{
IndexScan *node = makeNode(IndexScan);
***************
*** 951,956 ****
--- 953,959 ----
node->scan.scanrelid = scanrelid;
node->indxid = indxid;
node->indxqual = indxqual;
+ node->indxqualorig = indxqualorig;
node->scan.scanstate = (CommonScanState *) NULL;
return node;
Vadim Mikheev wrote: > > Bruce Momjian wrote: > > > > This new patch uses scankeys instead of throwing the indexqual to the > > executor. This is probably more efficient, but I am not sure about the > > other ramifications. It still fails. > > This wouldn't handle functional indices in OR... > > So, I added indexqualorig list to the IndexScan node: > > indexqual = fix_indxqual_references (indexqualorig) > > - indxqualorig' Var-s references heap tuple... > > Regression tests are ok. Great job! Many thanks. And you helped me to prove to my collegues that usually bugs in OpenSource software are fixed no later than the next weekend ;) > Patch made for CURRENT tree - let me know if there will > be problems with 6.4... I did'nt run regress but the patches did apply cleanly on 6.4.0 and also all my apps run fine, even the one that did have the OR problems after upgrading 6.3.2 -> 6.4. And of course they run faster ;) Thank you once more ! ------------- Hannu Krosing
> Bruce Momjian wrote: > > > > This new patch uses scankeys instead of throwing the indexqual to the > > executor. This is probably more efficient, but I am not sure about the > > other ramifications. It still fails. > > This wouldn't handle functional indices in OR... > > So, I added indexqualorig list to the IndexScan node: > > indexqual = fix_indxqual_references (indexqualorig) > > - indxqualorig' Var-s references heap tuple... > > Regression tests are ok. > Patch made for CURRENT tree - let me know if there will > be problems with 6.4... > > It's better to gmake clean in backend dir... > > Patch also fixes EXPLAIN for indices in OR: all indices > used are explained now. > > Vadim Thanks very much. I have applied this to the stable branch, because without it, we get backend crashes. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian wrote: > > Thanks very much. I have applied this to the stable branch, because > without it, we get backend crashes. Thanks, Bruce! I have only CURRENT tree... Vadim
> Bruce Momjian wrote: > > > > Thanks very much. I have applied this to the stable branch, because > > without it, we get backend crashes. > > Thanks, Bruce! I have only CURRENT tree... Glad to hear you are OK about appying it to stable tree. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026