Re: [PATCHES] Proposed patch to change TOAST compression strategy

[Jan Wieck]

On 2/18/2008 5:33 AM, Gregory Stark wrote:

Tom Lane [EMAIL PROTECTED] writes:


* Adds an early-failure path to the compressor as suggested by Jan:
if it's been unable to find even one compressible substring in the
first 1KB (parameterizable), assume we're looking at incompressible
input and give up.  (There are various ways this could be done, but
this way seems to add the least overhead to the compressor's inner
loop.)


I'm not sure how to test the rest of it, but this bit seems testable. I fear
this may be too conservative. Even nigh incompressible data will find a few
backreferences.


One could argue that storing JPEG in a bytea column and not configuring 
the column to skip compression is a pilot error. But I guess this 
happens much more often than accidentally finding some data that has 
zero backref within the first KB and changes pattern thereafter. Do you 
have any example for data that is like that?



Jan

--
Anyone who trades liberty for security deserves neither
liberty nor security. -- Benjamin Franklin


---(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] As proposed the complete changes to pg_trigger and pg_rewrite

[Jan Wieck]

On 3/16/2007 12:12 AM, Jan Wieck wrote:

On 3/15/2007 11:16 PM, Tom Lane wrote:

The SPI_savedplans part of this is pretty bletcherous, and has been
overtaken by events anyway.  I'd suggest testing whether plancache.c
has anything in its list.


[...]

After a quick look it seems that another little function in there, 
checking if (cached_plans_list == NIL), would do exactly that.


Changed patch is attached.


Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
Index: src/backend/commands/tablecmds.c
===
RCS file: /usr/local/pgsql/CvsRoot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.217
diff -c -r1.217 tablecmds.c
*** src/backend/commands/tablecmds.c13 Mar 2007 00:33:39 -  1.217
--- src/backend/commands/tablecmds.c16 Mar 2007 12:06:33 -
***
*** 53,58 
--- 53,59 
  #include parser/parse_relation.h
  #include parser/parse_type.h
  #include parser/parser.h
+ #include rewrite/rewriteDefine.h
  #include rewrite/rewriteHandler.h
  #include storage/smgr.h
  #include utils/acl.h
***
*** 253,259 
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
  static void ATExecSetRelOptions(Relation rel, List *defList, bool isReset);
  static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
!  bool enable, bool 
skip_system);
  static void ATExecAddInherit(Relation rel, RangeVar *parent);
  static void ATExecDropInherit(Relation rel, RangeVar *parent);
  static void copy_relation_data(Relation rel, SMgrRelation dst);
--- 254,262 
  static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
  static void ATExecSetRelOptions(Relation rel, List *defList, bool isReset);
  static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
!  char fires_when, bool 
skip_system);
! static void ATExecEnableDisableRule(Relation rel, char *rulename,
!  char fires_when);
  static void ATExecAddInherit(Relation rel, RangeVar *parent);
  static void ATExecDropInherit(Relation rel, RangeVar *parent);
  static void copy_relation_data(Relation rel, SMgrRelation dst);
***
*** 1955,1965 
--- 1958,1974 
pass = AT_PASS_MISC;
break;
case AT_EnableTrig: /* ENABLE TRIGGER variants */
+   case AT_EnableAlwaysTrig:
+   case AT_EnableReplicaTrig:
case AT_EnableTrigAll:
case AT_EnableTrigUser:
case AT_DisableTrig:/* DISABLE TRIGGER variants */
case AT_DisableTrigAll:
case AT_DisableTrigUser:
+   case AT_EnableRule: /* ENABLE/DISABLE RULE variants 
*/
+   case AT_EnableAlwaysRule:
+   case AT_EnableReplicaRule:
+   case AT_DisableRule:
case AT_AddInherit: /* INHERIT / NO INHERIT */
case AT_DropInherit:
ATSimplePermissions(rel, false);
***
*** 2127,2150 
case AT_ResetRelOptions:/* RESET (...) */
ATExecSetRelOptions(rel, (List *) cmd-def, true);
break;
!   case AT_EnableTrig: /* ENABLE TRIGGER name */
!   ATExecEnableDisableTrigger(rel, cmd-name, true, false);
break;
case AT_DisableTrig:/* DISABLE TRIGGER name */
!   ATExecEnableDisableTrigger(rel, cmd-name, false, 
false);
break;
case AT_EnableTrigAll:  /* ENABLE TRIGGER ALL */
!   ATExecEnableDisableTrigger(rel, NULL, true, false);
break;
case AT_DisableTrigAll: /* DISABLE TRIGGER ALL */
!   ATExecEnableDisableTrigger(rel, NULL, false, false);
break;
case AT_EnableTrigUser: /* ENABLE TRIGGER USER */
!   ATExecEnableDisableTrigger(rel, NULL, true, true);
break;
case AT_DisableTrigUser:/* DISABLE TRIGGER USER 
*/
!   ATExecEnableDisableTrigger(rel, NULL, false, true);
break;
case AT_AddInherit:
ATExecAddInherit(rel, (RangeVar *) cmd-def);
break;
--- 2136,2192 
case AT_ResetRelOptions:/* RESET

Re: [PATCHES] As proposed the complete changes to pg_trigger and pg_rewrite

[Jan Wieck]

On 3/15/2007 11:16 PM, Tom Lane wrote:

Jan Wieck [EMAIL PROTECTED] writes:

Attached is the completed patch that changes pg_trigger and extends
pg_rewrite in order to allow triggers and rules to be defined with
different, per session controllable, behaviors for replication purposes.


The SPI_savedplans part of this is pretty bletcherous, and has been
overtaken by events anyway.  I'd suggest testing whether plancache.c
has anything in its list.


Hehe,

after today's commit by you it also throws a cvs merge conflict exactly 
there ...


After a quick look it seems that another little function in there, 
checking if (cached_plans_list == NIL), would do exactly that.


Easy enough to adjust to that.


Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #

---(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] New features for pgbench

[Jan Wieck]

On 2/12/2007 11:43 AM, Tom Lane wrote:

Greg Smith [EMAIL PROTECTED] writes:
Right now when you run pgbench, the results vary considerably from run to 
run even if you completely rebuild the database every time.  I've found 
that a lot of that variation comes from two things:


This is a real issue, but I think your proposed patch does not fix it.
A pgbench run will still be penalized according to the number of
checkpoints or autovacuums that happen while it occurs.  Guaranteeing
that there's at least one is maybe a bit more fair than allowing the
possibility of having none, but it's hardly a complete fix.  Also,
this approach means that short test runs will have artificially lower
TPS results than longer ones, because the fixed part of the maintenance
overhead is amortized over fewer transactions.


Anything that doesn't run exclusively on the server, is given enough 
data in size and enough time to similarly populate the buffer cache for 
each run, WILL report more or less random TPS results. Real benchmarks 
on considerable sized hardware have ramp-up times that are measured in 
hours if not days, with the sole purpose of populating the cache and 
thus smoothing out the transaction response profile. I think this change 
is an entirely misleading approach to tackle the problem at hand.



Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #

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

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

Re: [PATCHES] [HACKERS] ARC Memory Usage analysis

[Jan Wieck]

On 10/22/2004 2:50 PM, Simon Riggs wrote:
I've been using the ARC debug options to analyse memory usage on the
PostgreSQL 8.0 server. This is a precursor to more complex performance
analysis work on the OSDL test suite.
I've simplified some of the ARC reporting into a single log line, which
is enclosed here as a patch on freelist.c. This includes reporting of:
- the total memory in use, which wasn't previously reported
- the cache hit ratio, which was slightly incorrectly calculated
- a useful-ish value for looking at the B lists in ARC
(This is a patch against cvstip, but I'm not sure whether this has
potential for inclusion in 8.0...)
The total memory in use is useful because it allows you to tell whether
shared_buffers is set too high. If it is set too high, then memory usage
will continue to grow slowly up to the max, without any corresponding
increase in cache hit ratio. If shared_buffers is too small, then memory
usage will climb quickly and linearly to its maximum.
The last one I've called turbulence in an attempt to ascribe some
useful meaning to B1/B2 hits - I've tried a few other measures though
without much success. Turbulence is the hit ratio of B1+B2 lists added
together. By observation, this is zero when ARC gives smooth operation,
and goes above zero otherwise. Typically, turbulence occurs when
shared_buffers is too small for the working set of the database/workload
combination and ARC repeatedly re-balances the lengths of T1/T2 as a
result of near-misses on the B1/B2 lists. Turbulence doesn't usually
cut in until the cache is fully utilized, so there is usually some delay
after startup.
We also recently discussed that I would add some further memory analysis
features for 8.1, so I've been trying to figure out how.
The idea that B1, B2 represent something really useful doesn't seem to
have been borne out - though I'm open to persuasion there.
I originally envisaged a shadow list operating in extension of the
main ARC list. This will require some re-coding, since the variables and
macros are all hard-coded to a single set of lists. No complaints, just
it will take a little longer than we all thought (for me, that is...)
My proposal is to alter the code to allow an array of memory linked
lists. The actual list would be [0] - other additional lists would be 
created dynamically as required i.e. not using IFDEFs, since I want this
to be controlled by a SIGHUP GUC to allow on-site tuning, not just lab
work. This will then allow reporting against the additional lists, so
that cache hit ratios can be seen with various other prototype
shared_buffer settings.
All the existing lists live in shared memory, so that dynamic approach 
suffers from the fact that the memory has to be allocated during ipc_init.

What do you think about my other theory to make C actually 2x effective 
cache size and NOT to keep T1 in shared buffers but to assume T1 lives 
in the OS buffer cache?

Jan
Any thoughts?


Index: freelist.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/storage/buffer/freelist.c,v
retrieving revision 1.48
diff -d -c -r1.48 freelist.c
*** freelist.c	16 Sep 2004 16:58:31 -	1.48
--- freelist.c	22 Oct 2004 18:15:38 -
***
*** 126,131 
--- 126,133 
  	if (StrategyControl-stat_report + DebugSharedBuffers  now)
  	{
  		long		all_hit,
+ buf_used,
+ b_hit,
  	b1_hit,
  	t1_hit,
  	t2_hit,
***
*** 155,161 
  		}
  
  		if (StrategyControl-num_lookup == 0)
! 			all_hit = b1_hit = t1_hit = t2_hit = b2_hit = 0;
  		else
  		{
  			b1_hit = (StrategyControl-num_hit[STRAT_LIST_B1] * 100 /
--- 157,163 
  		}
  
  		if (StrategyControl-num_lookup == 0)
! 			all_hit = buf_used = b_hit = b1_hit = t1_hit = t2_hit = b2_hit = 0;
  		else
  		{
  			b1_hit = (StrategyControl-num_hit[STRAT_LIST_B1] * 100 /
***
*** 166,181 
  	  StrategyControl-num_lookup);
  			b2_hit = (StrategyControl-num_hit[STRAT_LIST_B2] * 100 /
  	  StrategyControl-num_lookup);
! 			all_hit = b1_hit + t1_hit + t2_hit + b2_hit;
  		}
  
  		errcxtold = error_context_stack;
  		error_context_stack = NULL;
! 		elog(DEBUG1, ARC T1target=%5d B1len=%5d T1len=%5d T2len=%5d B2len=%5d,
  			 T1_TARGET, B1_LENGTH, T1_LENGTH, T2_LENGTH, B2_LENGTH);
! 		elog(DEBUG1, ARC total   =%4ld%% B1hit=%4ld%% T1hit=%4ld%% T2hit=%4ld%% B2hit=%4ld%%,
  			 all_hit, b1_hit, t1_hit, t2_hit, b2_hit);
! 		elog(DEBUG1, ARC clean buffers at LRU   T1=   %5d T2=   %5d,
  			 t1_clean, t2_clean);
  		error_context_stack = errcxtold;
  
--- 168,187 
  	  StrategyControl-num_lookup);
  			b2_hit = (StrategyControl-num_hit[STRAT_LIST_B2] * 100 /
  	  StrategyControl-num_lookup);
! 			all_hit = t1_hit + t2_hit;
!			b_hit = b1_hit + b2_hit;
! buf_used = T1_LENGTH + T2_LENGTH;
  		}
  
  		errcxtold 

Re: [PATCHES] [HACKERS] ARC Memory Usage analysis

[Jan Wieck]

On 10/22/2004 4:21 PM, Simon Riggs wrote:
On Fri, 2004-10-22 at 20:35, Jan Wieck wrote:
On 10/22/2004 2:50 PM, Simon Riggs wrote:
 
 My proposal is to alter the code to allow an array of memory linked
 lists. The actual list would be [0] - other additional lists would be 
 created dynamically as required i.e. not using IFDEFs, since I want this
 to be controlled by a SIGHUP GUC to allow on-site tuning, not just lab
 work. This will then allow reporting against the additional lists, so
 that cache hit ratios can be seen with various other prototype
 shared_buffer settings.

All the existing lists live in shared memory, so that dynamic approach 
suffers from the fact that the memory has to be allocated during ipc_init.

[doh] - dreaming again. Yes of course, server startup it is then. [That
way, we can include the memory for it at server startup, then allow the
GUC to be turned off after a while to avoid another restart?]
What do you think about my other theory to make C actually 2x effective 
cache size and NOT to keep T1 in shared buffers but to assume T1 lives 
in the OS buffer cache?
Summarised like that, I understand it.
My observation is that performance varies significantly between startups
of the database, which does indicate that the OS cache is working well.
So, yes it does seem as if we have a 3 tier cache. I understand you to
be effectively suggesting that we go back to having just a 2-tier cache.
Effectively yes, just with the difference that we keep a pseudo T1 list 
and hope that what we are tracking there is what the OS is caching. As 
said before, if the effective cache size is set properly, that is what 
should happen.

I guess we've got two options:
1. Keep ARC as it is, but just allocate much of the available physical
memory to shared_buffers, so you know that effective_cache_size is low
and that its either in T1 or its on disk.
2. Alter ARC so that we experiment with the view that T1 is in the OS
and T2 is in shared_buffers, we don't bother keeping T1. (as you say)
Hmmm...I think I'll pass on trying to judge its effectiveness -
simplifying things is likely to make it easier to understand and predict
behaviour. It's well worth trying, and it seems simple enough to make a
patch that keeps T1target at zero.
Not keeping T1target at zero, because that would keep T2 at the size of 
shared_buffers. What I suspect is that in the current calculation the 
T1target is underestimated. It is incremented on B1 hits, but B1 is only 
of T2 size. What it currently tells is what got pushed from T1 into the 
OS cache. It could well be that it would work much more effective if it 
would fuzzily tell what got pushed out of the OS cache to disk.

Jan
i.e. Scientific method: conjecture + experimental validation = theory
If you make up a patch, probably against BETA4, Josh and I can include it in the 
performance testing that I'm hoping we can do over the next few weeks.
Whatever makes 8.0 a high performance release is well worth it.
Best Regards, 

Simon Riggs

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings

Re: [PATCHES] log_filename_prefix -- log_filename + strftime()

[Jan Wieck]

On 8/29/2004 5:12 AM, Andreas Pflug wrote:
Tom Lane wrote:
Andreas Pflug [EMAIL PROTECTED] writes:
Tom Lane wrote:
I can see the value of not needing any cron daemon to remove old logs.

No other logs on your system to purge?

The DBA isn't necessarily also root.
Interesting this argument comes from you.. :-)
Tasks like purging old log files is certainly not a job that needs to be 
implemented in the backend; instead, an external database maintenance 
agent should do that.
You must have misunderstood something here. The proposal doesn't 
implement any logfile purging feature, but allows to setup a 
configuration that automatically overwrites files in a rotating manner, 
if the DBA so desires. I can't see how you're jumping to logfile purging 
from that.

Jan
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings

Re: [PATCHES] log_filename_prefix -- log_filename + strftime()

[Jan Wieck]

On 8/29/2004 2:06 PM, Tom Lane wrote:
Andreas Pflug [EMAIL PROTECTED] writes:
Jan Wieck wrote:
  but allows to setup a
configuration that automatically overwrites files in a rotating manner, 
if the DBA so desires.

... which can't work because it will overwrite the logfile on server 
start, and thus will overwrite the very latest logfile when performing 
multiple restarts.
You are ignoring a critical part of the proposal, which is to overwrite
only during a time-based rotation; at logger startup or size-based
rotation, the rule would be to append.
which then has a problem when you startup the postmaster after 10 hours 
of downtime ... hmmm.

Jan
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Re: [PATCHES] contrib/dbmirror

[Jan Wieck]

On 7/1/2004 12:39 AM, Tom Lane wrote:
Joe Conway [EMAIL PROTECTED] writes:
[EMAIL PROTECTED] wrote:
Attached is a 1 line bug fix for dbmirror that was submitted.
It fixes a bug where some transactions could be dropped when writing 
mirrored SQL statements to files.

I know that there were discussions regarding removing the replication 
contribs (rserv and dbmirror) prior to 7.5 release, but given that that 
has not happened yet, any objections to me applying this?
There was talk of removing rserv, because it's seriously obsolete and
not maintained, but I don't think the same argument applies to
dbmirror.  Patch away.
There was never any intention to remove them. They should be relocated 
to the pgfoundry. The reason for this is that up to today, people 
looking for replication solutions find rserv in contrib and waste time 
with it. Others try dbmirror and later on apply their results with 
trigger based replication to Slony and think must be slow.

dbmirror is well maintained, and I know that it can and will do things 
that Slony is not planned to do (like keyrange based partial 
replication). It should be kept, but from the past discussions we know 
that contrib is a location that makes a lot of people assume that those 
things are recommended, preferred or some such.

Jan
regards, tom lane
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 6: Have you searched our list archives?
  http://archives.postgresql.org

Re: [PATCHES] [HACKERS] Vacuum Delay feature

[Jan Wieck]

Bruce Momjian wrote:
Jan Wieck wrote:
Attached is a corrected version that solves the query cancel problem by 
not napping any more and going full speed as soon as any signal is 
pending. If nobody objects, I'm going to commit this tomorrow.
Jan, three questions.  First, is this useful now that we have the new
cache replacement code, second, do we need this many parameters (can't
any of them be autotuned), and third, what about documentation?
You mean if stopping to nap is useful when a signal is pending or if 
napping during vacuum itself is useful at all?

I am willing to make it all self tuning and automagic. Just tell me how.
Documentation is missing so far. Will work on that.
Jan
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
   (send unregister YourEmailAddressHere to [EMAIL PROTECTED])

Re: [PATCHES] contrib/dbmirror typo fix

[Jan Wieck]

[EMAIL PROTECTED] wrote:
Please apply this minor patch to the cvs HEAD of dbmirror
It fixes a typo in a define
Thanks
Applied.
Jan



Index: pending.c
===
RCS file: /projects/cvsroot/pgsql-server/contrib/dbmirror/pending.c,v
retrieving revision 1.17
diff -u -r1.17 pending.c
--- pending.c	22 Apr 2004 03:48:38 -	1.17
+++ pending.c	24 May 2004 16:30:38 -
@@ -76,7 +76,7 @@
 #else
 #define debug_msg2(x,y) 
 #define debug_msg(x)
-#define debug_msg(x,y,z)
+#define debug_msg3(x,y,z)
 
 #endif
 


---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 3: 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] new aggregate functions v1

[Jan Wieck]

Fabien COELHO wrote:
Dear Alvaro,
 (2) bitwise integer aggregates named bit_and and bit_or for
 int2, int4, int8 and bit types. They are not standard,
 however they exist in other db (eg mysql), and I needed them
 for some other stuff.
I'm sure people won't like to add functions just because some other DB
has them.
I develop them because I NEEDED them, NOT because they are available
in mysql and I would want compatibility. I needed them for some queries
over pg_catalog, as acl are encoded as bitfields.
So the addition is about functionnality, not compatibility.
The argument about mysql is rather to show that other people in the world
found these functions also useful, and to justify the name I chose.
The argument about MySQL (tm) is IMHO a good one, because in contrast to 
MySQL (tm), PostgreSQL is designed and implemented as a catalog based 
system. We carry the performance burden we always got blamed for 
(unjustified because MySQL (tm) is slow as hell anyway) for a reason, 
and the reason is flexibility. In MySQL (tm) you don't have a chance, 
the functionality you want must be the functionality linked into the 
server. With PostgreSQL you can add the functionality you need where and 
when you need it.

You may also notice that the impact in close to void, there are two lines
added for each of these bit_* functions. I'm not going to develop an
external package for 16 lines of code.
Maybe I'm missing something, but what is wrong with installing these 
functions on demand as user defined functions? After all, even PL/pgSQL 
is still an external package ... sort of at least.

Jan
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Re: [PATCHES] ALSO keyword to CREATE RULE patch

[Jan Wieck]

Tom Lane wrote:

Fabien COELHO [EMAIL PROTECTED] writes:
Most of the patch deals with the documentation, which is rather ugly
because it keeps telling about INSTEAD vs non-INSTEAD rules, as there
is no name for the default behavior. I think ALSO fixes this issue as it
clarifies the explanations.
Hmm ... I find that argument much more convincing than any of the others
...
Jan, what do you think?  You invented this command's syntax IIRC.
I did not. We inherited it from Postgres 4.2. If people think an ALSO 
keyword will clearify things, what about renaming the whole CREATE RULE 
into something along the line CREATE QUERY REWRITE MACRO?

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 6: Have you searched our list archives?
  http://archives.postgresql.org

Re: [PATCHES] ALSO keyword to CREATE RULE patch

[Jan Wieck]

Bruce Momjian wrote:

Jan Wieck wrote:
Tom Lane wrote:

 Fabien COELHO [EMAIL PROTECTED] writes:
 Most of the patch deals with the documentation, which is rather ugly
 because it keeps telling about INSTEAD vs non-INSTEAD rules, as there
 is no name for the default behavior. I think ALSO fixes this issue as it
 clarifies the explanations.
 
 Hmm ... I find that argument much more convincing than any of the others
 ...
 
 Jan, what do you think?  You invented this command's syntax IIRC.

I did not. We inherited it from Postgres 4.2. If people think an ALSO 
keyword will clearify things, what about renaming the whole CREATE RULE 
into something along the line CREATE QUERY REWRITE MACRO?
Are you saying you don't want ALSO added?

No, I am saying that CREATE RULE is so often misinterpreted to work like 
a trigger and when we explain that it is more like Macro expansion 
before the query is executed they understand better. CREATE RULE itself 
is just vague, with or without ALSO.

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster

Re: [PATCHES] Vacuum Delay feature

[Jan Wieck]

Attached is a corrected version that solves the query cancel problem by 
not napping any more and going full speed as soon as any signal is 
pending. If nobody objects, I'm going to commit this tomorrow.

Jan

Jan Wieck wrote:

The attached patch applies to CVS tip as of 02/05/2004 and implements 
the cost based vacuum delay feature.

A detailed description with charts of different configuration settings 
can be found here:

 http://developer.postgresql.org/~wieck/vacuum_cost/

There is a problem left that seems to be related to Toms observations in 
the shutdown behaviour of the postmaster. My current guess is that the 
napping done via select(2) somehow prevents responding to the query 
abort signal.

Jan



--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #


vacuum_cost.75devel.diff.gz
Description: GNU Zip compressed data

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

[PATCHES] Vacuum Delay feature

[Jan Wieck]

The attached patch applies to CVS tip as of 02/05/2004 and implements 
the cost based vacuum delay feature.

A detailed description with charts of different configuration settings 
can be found here:

http://developer.postgresql.org/~wieck/vacuum_cost/

There is a problem left that seems to be related to Toms observations in 
the shutdown behaviour of the postmaster. My current guess is that the 
napping done via select(2) somehow prevents responding to the query 
abort signal.

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #


vacuum_cost.75devel.diff.gz
Description: GNU Zip compressed data

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

   http://archives.postgresql.org

Re: [PATCHES] pltcl - Cache lookup for attribute error - version

[Jan Wieck]

Tom Lane wrote:

Patrick Samson [EMAIL PROTECTED] writes:
Attribute names beginning with a dot are filtered
just in one place, in pltcl_trigger_handler().
(version 7.3.5)
I am not sure why that code is there.  It is *not* there to prevent the
loop from touching dropped attributes, because the same code is in the
original 1.1 version of pltcl.c, long before we could drop attributes.
Jan, do you remember why you put this into pltcl_trigger_handler()?
/
 * Ignore pseudo elements with a dot name
 /
if (*(ret_values[i]) == '.') {
i += 2;
continue;
}
It's not documented behavior that I can see, and it doesn't seem to have
any use other than making pltcl triggers fail if a user chooses a field
name starting with a dot :-(
right, this is documented nowhere :-(

When assigning a tuple to an array, PL/Tcl creates one extra array 
element .tupno telling the SPI_tuptable index of the result tuple. I 
think I originally planned to have more of these critters ... but 
probably never really needed them. It is in there since 6.3!

Bottom line is, if one has a trigger, and inside the trigger he does an 
SPI_exec, fetches a tuple into an array and then returns [array get x] 
instead of new or old ... so from the back through the right chest into 
the left eye ... then it will fail if the .tupno isn't filtered out.

Jan


Attached is a patch to :
- Add a filter in two other places, in relation
  with the mentioned error message:
   pltcl_set_tuple_values()
   pltcl_build_tuple_argument()
This is already done in 7.4, although for some reason
pltcl_trigger_handler got overlooked - I will fix that.
- Add the same filter in the build of TG_relatts.
  This will prevent a tcl script which loops on
  TG_relattrs to fail in trying to use a dropped
  column.
This is deliberately *not* done in 7.4, because it would break the
documented behavior of TG_relatts:
$TG_relatts

 A Tcl list of the table column names, prefixed with an empty list
 element. So looking up a column name in the list with 
 Tcl's lsearch command returns the element's number starting with 1
 for the first column, the same way the 
 columns are customarily numbered in PostgreSQL. 

I think we need to preserve the relationship to column numbers.  People
who just want a list of the live columns can get it from the OLD or NEW
arrays.
			regards, tom lane


--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 3: 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] pltcl - Cache lookup for attribute error - version

[Jan Wieck]

Tom Lane wrote:

Jan Wieck [EMAIL PROTECTED] writes:
When assigning a tuple to an array, PL/Tcl creates one extra array 
element .tupno telling the SPI_tuptable index of the result tuple. I 
think I originally planned to have more of these critters ... but 
probably never really needed them. It is in there since 6.3!

Bottom line is, if one has a trigger, and inside the trigger he does an 
SPI_exec, fetches a tuple into an array and then returns [array get x] 
instead of new or old ... so from the back through the right chest into 
the left eye ... then it will fail if the .tupno isn't filtered out.
Hm.  Perhaps we should tighten the test to reject only .tupno, rather
than any name starting with dot?
Man you have worries ... aren't people who use identifiers with a 
leading dot supposed to have problems? What about changing it to .. 
instead? I mean, how does such a thing look like?

SELECT .. some column .. FROM .. the schema . a table ..
WHERE .. the schema . a table . some column ..
IN ('.oh.', '.give.', '.me.', '.a.', '.break!');
If you like to, tighten it.

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 6: Have you searched our list archives?
  http://archives.postgresql.org

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

[Jan Wieck]

Claudio Natoli wrote:

Something after 2003/11/20 enhanced the query cancel handling. Namely, 
CVS tip now responds to a query cancel with a postmaster restart 
canceling all queries. Could the fork/exec stuff be responsible for this?

Jan

Hi Bruce,

Claudio, where are we on this patch?  I see an even newer version in the
archives:

http://archives.postgresql.org/pgsql-patches/2003-12/msg00361.php
[Weird you and Google groups missed it!]

 
Anyway, Tom has looked at your newest patch and it looks good to him.
I'm happy with the patch from the link above being committed if Tom has no
more comments. Was only waiting for a final nod from him.
Once it is in the source tree, give me a couple days and I'll fire off a
patch with the actual CreateProcess calls... and then we are off into Win32
signal land [shudder].
Cheers,
Claudio
--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
a
href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em
ailpolicy.html/a

---(end of broadcast)---
TIP 6: Have you searched our list archives?
   http://archives.postgresql.org


--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
  http://www.postgresql.org/docs/faqs/FAQ.html

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

[Jan Wieck]

Bruce Momjian wrote:
Tom Lane wrote:
 Claudio Natoli wrote:
 The only things I've thought of so far are:
 a) sticking the PID/cancel key list in shared mem [yeech]
 b) rearranging the entire cancel handling to occur in the postmaster [double
 yeech]
(a) seems like the lesser of the available evils (unless someone has a
bright idea for a plan C).
Bruce Momjian [EMAIL PROTECTED] writes:
 I think we need to move in the direction of a separate fork/exec-only
 shared memory segment that holds the pids and cancel keys for all the
 backends.
That doesn't seem worth the trouble.  I'd be inclined to just stick the
cancel keys in the PGPROC structures (I believe the PIDs are already in
there).  The original motivation for keeping them only in postmaster
local memory was to keep backend A's cancel key away from the prying
eyes of backend B, but is there really any security added?  Anyone who
can inspect PGPROC hardly needs to know the cancel key --- he can just
issue a SIGINT (or worse) directly to the target backend.
Agreed.  I was going for a separate one just to be paranoid.  This will
only be done for exec(), so I don't see a problem for normal Unix use
anyway.
It doesn't hurt to keep the locations and code as much in sync as 
possible. I think Tom's idea to move the information into the PGPROC 
entry is the winner and does not need any OS specific handling.

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

[Jan Wieck]

Bruce Momjian wrote:

Claudio Natoli wrote:
Tom Lane writes:
 Actually, on further reflection a separate array to store PIDs and
cancel keys is probably a better idea.  
[snip]
 I still think it's unnecessary to make a separate shmem segment for it,
though.

Why is that? Don't we need the backends to have access to it to do a cancel
request? I think I've missed something here...
I think they are saying put the cancel key inside the existing shared
memory segment.  I don't know when we actually attach to the main shared
memory sigment in the child, but it would have to be sooner than when we
need the cancel key.
I said move it into the PGPROC structure. And keep the pid in both, the 
PGPROC structure and postmaster local memory.

The backend attaches to the shared memory during 
AttachSharedMemoryAndSemaphores() ... where else?

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 3: 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] Doc patch--clarifying $1 in PL/PgSQL

[Jan Wieck]

Tom Lane wrote:
Jan Wieck [EMAIL PROTECTED] writes:
Tom Lane wrote:
Not that hard ... just requires replacing some special-purpose code with
general-purpose code ...

Does that code cause the variables value to change from function call to 
function call (what most users would expect if they give it a default 
value based on a call argument), or will remember the value from the 
first function call for the lifetime of the backend?
I believe it will evaluate the DEFAULT expression on each entry to the
block, using the current values of outer-block variables (and also
variables declared earlier in the same block, if anyone cared to use
that behavior).  The code was already designed and documented to
evaluate DEFAULT expressions on each block entry --- what it was missing
was the ability to reference variables in these expressions.
Do you see something wrong with it?
No, I just didn't test it yet. My only concern was that it could be 
another unexpected behaviour related to caching values/plans. Unexpected 
caching is what most likely becomes FAQ's and I think we have enough of 
those.

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Re: [PATCHES] [HACKERS] BEGIN vs START TRANSACTION

[Jan Wieck]

Bruce Momjian wrote:

Tom Lane wrote:
Bruce Momjian [EMAIL PROTECTED] writes:
 Peter Eisentraut wrote:
 I object to adding unnecessary complications like that.
 Shouldn't BEGIN and START TRANSACTION have the same mechanics?  The
 changes to the code were the addition of only one line.  The rest of the
 patch was docs.
My initial reaction was the same as Peter's, but after seeing the small
size of the patch I reconsidered.  It seems to make sense that BEGIN
should be an exact synonym for START TRANSACTION.
Let me give you my logic on this --- if people think of BEGIN and START
TRANSACTION as the same, and they do \h begin, they aren't going to see
the read only and isolation options for START TRANSACTION, and I doubt
they are going to think to look there because they think they are the
same.  That's why I think it is good to add those clauses to BEGIN
WORK/TRANSACTION.
Since BEGIN isn't standard, wouldn't it be time to redirect them on the 
BEGIN manpage to the START TRANSACTION manpage and tell them there that 
BEGIN does not support the full syntax of START TRANSACTION?

Jan

--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.  #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster