Re: [PATCHES] Proposed patch for operator lookup caching

2007-11-27 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


 Since Simon seems intent on hacking something in there, here is a patch
 that I think is actually sane for improving operator lookup speed.

+1 on the patch (reviewed and tested), and +1 for rolling it into RC.

- --
Greg Sabino Mullane [EMAIL PROTECTED]
PGP Key: 0x14964AC8 200711270954
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8

-BEGIN PGP SIGNATURE-

iD8DBQFHTC+rvJuQZxSWSsgRA7RdAJ9nGwaRPeUXLeLjBGsPfLi64dTmOwCeK/40
W/7/8n2Q1YvLyNABFHnv7No=
=o5Vy
-END PGP SIGNATURE-



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


Re: [PATCHES] Proposed patch for operator lookup caching

2007-11-27 Thread Simon Riggs
On Mon, 2007-11-26 at 21:13 -0500, Tom Lane wrote:

 Since Simon seems intent on hacking something in there, here is a patch
 that I think is actually sane for improving operator lookup speed.
 This patch caches all lookups, exact or ambiguous (since even the exact
 ones require multiple cache searches in common cases); and behaves sanely
 in the presence of search_path, pg_operator, or pg_cast changes.
 
 I see about a 45% speedup (2110 vs 1445 tps) on Guillame Smet's test case.
 On straight pgbench --- which has no ambiguous operators, plus it's not
 read-only --- it's hard to measure any consistent speedup, but I can say
 that it's not slower.  Some other test cases would be nice.

I see 45% speedup also on my previously published tests.

No noticeable difference on the integer test, so looks good.

 I went through the code that's being bypassed in some detail, to see what
 dependencies were being skipped over.  I think that as long as we assume
 that no *existing* type changes its domain base type, typtype, array
 status, type category, or preferred-type status, we don't need to flush
 the cache on pg_type changes.  This is a good thing since pg_type changes
 frequently (eg, at temp table create or drop).
 
 The only case that I believe to be unhandled is that the cache doesn't pay
 attention to ALTER TABLE ... INHERIT / NO INHERIT events.  This means it
 is theoretically possible to return the wrong operator if an operator
 takes a complex type as input and the calling situation involves another
 complex type whose inheritance relationship to that one changes.  That's
 sufficiently far out of the normal case that I'm not very worried about it
 (in fact, we probably have bugs in that area even without this patch,
 since for instance cached plans don't respond to such changes either).
 We could plug the hole by forcing a system-wide cache reset during ALTER
 TABLE ... INHERIT / NO INHERIT, if anyone insists.

No, thats enough.

 I'm not entirely happy about applying a patch like this so late in
 the beta cycle, but I'd much rather do this than than any of the
 less-than-half-baked ideas that have been floated in the discussion
 so far.

Well, as long as we fix this, I don't mind how we do it.

The reason for writing the other patch was your requirement for a
minimally invasive patch. If we're willing to lift that requirement then
I'm happy to go with your patch. Personally, I am.

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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

   http://archives.postgresql.org