Re: [PATCHES] [HACKERS] quote_literal with NULL

2007-10-15 Thread Brendan Jurd
On 10/12/07, Simon Riggs [EMAIL PROTECTED] wrote:
 I think you should add some examples to show how we would handle an
 INSERT or an UPDATE SET with quite_nullable() and a SELECT WHERE clause
 with quote_literal. The difference is a subtle one, which is why nobody
 mentioned it before, so it needs some better docs too.

 A cross-ref to the functions page would help also.

Alright, I've improved the documentation along the lines suggested by
Simon.  There's a full example on doing a null-safe dynamic UPDATE, as
well as a brief discussion about being wary of using comparison
operators with NULLs (e.g., in WHERE clauses).  Cross references
abound.

I did make a version of the patch which has the pg_proc entries for
quote_literal and quote_nullable both pointing to the same internal
function.  I thought this was a tidier solution, but it failed
regression test #5 in opr_sanity; apparently two entries in pg_proc
can't have the same prosrc and differing proisstrict?

Cheers,
BJ


quote-nullable_1.diff.bz2
Description: BZip2 compressed data

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] Updated patch for tsearch contrib examples

2007-10-15 Thread Heikki Linnakangas
Tom Lane wrote:
 I've reviewed and tested Sergey Karpov's proposed contrib modules
 dict_int, dict_xsyn, and test_parser.

Isn't dict_xsym like the built in synonym dictionary, but with support
for multiple replacement words? Why don't we just replace the built in
synonym dictionary with this?

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Updated patch for tsearch contrib examples

2007-10-15 Thread Tom Lane
Heikki Linnakangas [EMAIL PROTECTED] writes:
 Isn't dict_xsym like the built in synonym dictionary, but with support
 for multiple replacement words? Why don't we just replace the built in
 synonym dictionary with this?

Well, we'd still need a contrib example that uses a config file, so that
there's an example of the right way to do that.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] [HACKERS] quote_literal with NULL

2007-10-15 Thread Tom Lane
Brendan Jurd [EMAIL PROTECTED] writes:
 I'm all for the prevalance of sanity, but I'm not really clear on what
 about the above scenario is not sane.

Well, a situation like that just calls into question whether there's
been a mistake --- in particular whether the underlying function is
really null-safe or not.

We could remove the opr_sanity test, but to me that'd be akin to
disabling a compiler warning.  Our project policy has generally been
to write warning-free code, instead.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Assertion failure with small block sizes

2007-10-15 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the same
 line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the
 assertion then it passes all regression tests even if I push
 TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as
 possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as
 well.

Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
heapam.c, so that it explicitly tries to toast only in plain tables,
rather than adding more exclusion cases.  Thoughts?

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Assertion failure with small block sizes

2007-10-15 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 Gregory Stark [EMAIL PROTECTED] writes:
 If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the 
 same
 line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the
 assertion then it passes all regression tests even if I push
 TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as
 possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as
 well.

 Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
 heapam.c, so that it explicitly tries to toast only in plain tables,
 rather than adding more exclusion cases.  Thoughts?

Well RELKIND_UNCATALOGED can be usefully toasted in that data can be
compressed internally. I suppose we know none normally receive such treatment
at normal block sizes. If we want to make the tuple threshold configurable
down the line would we want it affecting initdb bootstrapping? My experiments
show it isn't a problem but I don't particularly see any reason why it's
advantageous. 

We may want some future relkinds to be toastable but I don't see a problem
adding new cases to the test.


-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] Assertion failure with small block sizes

2007-10-15 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 Tom Lane [EMAIL PROTECTED] writes:
 Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
 heapam.c, so that it explicitly tries to toast only in plain tables,
 rather than adding more exclusion cases.  Thoughts?

 Well RELKIND_UNCATALOGED can be usefully toasted in that data can be
 compressed internally.

But by the time we are inserting any data that needs compression, the
relkind should not be that anymore.  This would only be an issue if
pg_proc.h itself contained DATA() lines long enough to need toasting.
I argue that that isn't true and isn't likely to become true.  (See
ts_debug() for an example of deliberately avoiding such a case...)

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


[PATCHES] Avoid needless copy in nodeMaterial

2007-10-15 Thread Neil Conway
Attached is a patch that avoids a needless copy of the result tuple in
nodeMaterial, in the case that we don't have a previously-materialized
tuple to return. We can just return the TTS produced by executing our
child node, rather than returning a copy of it.

I didn't bother pulling the MinimalTuple out of outerslot and stuffing
it back into the nodeMaterial's result slot, as AFAICS that is not
necessary. Although I suppose you could make a cleanliness argument that
that would be worth doing instead.

(This is 8.4 material...)

-Neil

Index: source/src/backend/executor/nodeMaterial.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/nodeMaterial.c,v
retrieving revision 1.59
diff -p -c -r1.59 nodeMaterial.c
*** source/src/backend/executor/nodeMaterial.c	21 May 2007 17:57:33 -	1.59
--- source/src/backend/executor/nodeMaterial.c	16 Oct 2007 04:00:46 -
*** ExecMaterial(MaterialState *node)
*** 98,103 
--- 98,105 
  			eof_tuplestore = true;
  	}
  
+ 	ExecClearTuple(slot);
+ 
  	/*
  	 * If necessary, try to fetch another row from the subplan.
  	 *
*** ExecMaterial(MaterialState *node)
*** 124,147 
  		}
  
  		/*
! 		 * Append returned tuple to tuplestore.  NOTE: because the tuplestore
! 		 * is certainly in EOF state, its read position will move forward over
! 		 * the added tuple.  This is what we want.
  		 */
  		if (tuplestorestate)
  			tuplestore_puttupleslot(tuplestorestate, outerslot);
  
! 		/*
! 		 * And return a copy of the tuple.	(XXX couldn't we just return the
! 		 * outerslot?)
! 		 */
! 		return ExecCopySlot(slot, outerslot);
  	}
  
! 	/*
! 	 * Nothing left ...
! 	 */
! 	return ExecClearTuple(slot);
  }
  
  /* 
--- 126,143 
  		}
  
  		/*
! 		 * Append a copy of the returned tuple to tuplestore.  NOTE: because
! 		 * the tuplestore is certainly in EOF state, its read position will
! 		 * move forward over the added tuple.  This is what we want.
  		 */
  		if (tuplestorestate)
  			tuplestore_puttupleslot(tuplestorestate, outerslot);
  
! 		return outerslot;
  	}
  
! 	/* Nothing left, return empty slot */
! 	return slot;
  }
  
  /* 

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [PATCHES] Avoid needless copy in nodeMaterial

2007-10-15 Thread Neil Conway
On Tue, 2007-10-16 at 00:34 -0400, Tom Lane wrote:
 Seems like this needs more comments about what's happening, rather
 than less ...

Fair point.

 Also, it looks to me like the plan node's own resultslot might never be
 assigned to at all, when the subplan returns zero rows.  Does this
 corner case still work correctly?

ISTM the node's own result slot wouldn't be assigned to in this case
regardless: (nodeMaterial.c, circa 116)

outerslot = ExecProcNode(outerNode);
if (TupIsNull(outerslot))
{
node-eof_underlying = true;
return NULL;
}

There's no requirement that we must assign to the result slot, AFAICS.

-Neil



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match