> 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');
	insert into test values (1,'barney');
	insert into test select * from test;
	..repeat the above several times
	create index 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 an index 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, Pennsylvania 19026


Sun, 22 Nov 1998 00:49:57 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Bug in 6.4 release
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
Sun, 22 Nov 1998 17:31:49 +0700
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Bug in 6.4 release
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

Sun, 22 Nov 1998 21:53:37 +0200
Hannu Krosing <hannu@trust.ee>
Re: [HACKERS] Bug in 6.4 release - thanks for for fixing it!
> 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, Pennsylvania 19026

Mon, 23 Nov 1998 22:51:21 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Bug in 6.4 release
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

Tue, 24 Nov 1998 10:53:53 +0700
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Bug in 6.4 release
> 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, Pennsylvania 19026

Mon, 23 Nov 1998 23:04:39 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Bug in 6.4 release