Re: [PATCHES] Proposed patch to change TOAST compression strategy
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
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);
Re: [PATCHES] As proposed the complete changes to pg_trigger and pg_rewrite
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
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
On 10/22/2004 4:09 PM, Kenneth Marshall wrote: On Fri, Oct 22, 2004 at 03:35:49PM -0400, Jan Wieck wrote: 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 Jan, From the articles that I have seen on the ARC algorithm, I do not think that using the effective cache size to set C would be a win. The design of the ARC process is to allow the cache to optimize its use in response to the actual workload. It may be the best use of the cache in some cases to have the entire cache allocated to T1 and similarly for T2. If fact, the ability to alter the behavior as needed is one of the key advantages. Only the "working set" of the database, that is the pages that are very frequently used, are worth holding in shared memory at all. The rest should be copied in and out of the OS disc buffers. The problem is, with a too small directory ARC cannot guesstimate what might be in the kernel buffers. Nor can it guesstimate what recently was in the kernel buffers and got pushed out from there. That results in a way too small B1 list, and therefore we don't get B1 hits when in fact the data was found in memory. B1 hits is what increases the T1target, and since we are missing them with a too small directory size, our implementation of ARC is propably using a T2 size larger than the working set. That is not optimal. If we would replace the dynamic T1 buffers with a max_backends*2 area of shared buffers, use a C value representing the effective cache size and limit the T1target on the lower bound to effective cache size - shared buffers, then we basically moved the T1 cache into the OS buffers. This all only holds water, if the OS is allowed to swap out shared memory. And that was my initia
Re: [PATCHES] [HACKERS] ARC Memory Usage analysis
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] [HACKERS] ARC Memory Usage analysis
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_LEN
Re: [PATCHES] log_filename_prefix --> log_filename + strftime()
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] log_filename_prefix --> log_filename + strftime()
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()
On 8/27/2004 2:41 PM, Tom Lane wrote: "Ed L." <[EMAIL PROTECTED]> writes: On Friday August 27 2004 12:08, Tom Lane wrote: [ justification please ] Yes, should have said more on that item. First, I didn't see how to easily make it configurable in combination with strftime() without doing more work, and it didn't appear to be worth the effort. By its addition, hard-coding the PID into the filename deviates from what I would argue is the de facto standard of Apache's rotatelogs and forces a naming convention where none existed before. That creates work for us as we have a considerable infrastructure setup to deal with logs; I suspect that may be the case with others. I looked, but did not find, justification for why it was introduced; I would assume it was added to allow for multiple postmasters sharing the same log directory. I had difficulty fathoming the usefulness of this being hard-coded, as it seems one could compensate easily through the configurable 'log_filename' if one chose to share a log directory among postmasters. Not by including the PID, but by some other postmaster-unique naming approach. Given its a new 'feature', I'm hoping it can be altered to return the freedom of filenaming to the administrator. Or you could use different log_directory settings for different PMs. Fair enough. Anyone else have an opinion pro or con about this change? IMHO it's in the gray area between bug fix and feature addition. If we want to do it, though, doing it now is certainly better than holding it for 8.1, since by then people would have gotten used to the present behavior. Which is thw way we have treated things the like before. I remember that by the time lztext was in the backend we knew already about toast in the next release and therefore discouraged the use. This is not possible here, so I say do the change now. 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] contrib/dbmirror
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
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
[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
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
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] "ALSO" keyword to "CREATE RULE" patch
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] Vacuum Delay feature
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
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] win32 signals, part 4
Magnus Hagander wrote: Ok, here we go again. Taking into account Claudios comments on the previous patch, as well as some more fooling around here of my own, here's a fourth (and final?) one. If there are no further comments from Claudio or anyone else, I feel this is now ready to be applied. Differences from last version: 1) Per Claudios suggestion, create a "babysitter thread" for each process that waits on the process and signals ourselves. This reduces the amount of code (=good) and most importantly removes all the synchronisation issues (=even better). The only thing left to sync is the signal delivery, and that has alreay been taken care of in previous patches. IIRC a separate "babysitter thread" just to handle message passing is exactly what Katie Ward did for UltraSQL ... the Win32 port done at NuSphere. Glad to see she was right about 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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] unified diffs, PLEASE?
Wiseguy wrote: Is there a good reason why minor source revisions are not posted to the ftp mirrors as unified diff patchfiles? I mean, both 7.4.0 and 7.4.1 were nearly 10MB in size as bz2 archives. That's a 40 minute download per file for a 56kb modem connection. Whereas, I created a bz2 patchfile from 7.4.0 to 7.4.1 that was a mere 100KB: 1% that size. My suggestion: post minor revision changes as unified diff patchfiles on the mirror sites. The sourcecode base is getting large enough to justify this. You can use anon-CVS to get that diff. Or even cvsup after every release tagging to have the entire revision history with all commit messages. 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 8: explain analyze is your friend
Re: [PATCHES] pltcl - "Cache lookup for attribute" error - version
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] pltcl - "Cache lookup for attribute" error - version
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] fork/exec patch: pre-CreateProcess finalization
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] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: Jan Wieck <[EMAIL PROTECTED]> writes: 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. Actually, on further reflection a separate array to store PIDs and cancel keys is probably a better idea. If we put this stuff in PGPROC then the postmaster will need to be able to obtain the ProcStructLock (or whatever it's called this week) in order to examine/modify that data structure. That gets us into the usual concerns about backend bugs locking up the postmaster, etc. But if it's a separate array then we can just have the rule that no one but the postmaster gets to write in it. I still think it's unnecessary to make a separate shmem segment for it, though. I'd like to avoid the additional shmem segment if possible. The postmaster can keep the stuff he needs in local memory. I did not mean to rip everything out of postmaster local memory, and that little bit of redundancy does not hurt. The pid's of processes aren't likely to change very often. 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
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
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 href="http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(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] Doc patch--clarifying $1 in PL/PgSQL
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] Doc patch--clarifying $1 in PL/PgSQL
Tom Lane wrote:
David Fetter <[EMAIL PROTECTED]> writes:
On Mon, Dec 22, 2003 at 05:50:12PM -0500, Tom Lane wrote:
David Fetter <[EMAIL PROTECTED]> writes:
+ Note that it is not possible to assign function arguments during
+ a DECLARE block.
Seems to me this is a bug that should be fixed, not documented.
I got the impression from Jan Wieck that it wasn't fixable, or at
least not without a major rewrite of the plpgsql engine. I'm sure
somebody will correct me if I got a mistaken impression, though :)
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?
Jan
regards, tom lane
*** src/pl/plpgsql/src/gram.y.orig Sat Nov 29 14:52:12 2003
--- src/pl/plpgsql/src/gram.y Mon Dec 22 18:50:35 2003
***
*** 628,679
{ $$ = NULL; }
| decl_defkey
{
! inttok;
! intlno;
! PLpgSQL_dstring ds;
! PLpgSQL_expr *expr;
!
! lno = plpgsql_scanner_lineno();
! expr = malloc(sizeof(PLpgSQL_expr));
! plpgsql_dstring_init(&ds);
! plpgsql_dstring_append(&ds, "SELECT ");
!
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->plan = NULL;
! expr->nparams = 0;
!
! tok = yylex();
! switch (tok)
! {
! case 0:
! yyerror("unexpected end of function");
! case K_NULL:
! if (yylex() != ';')
! yyerror("expected \";\" after \"NULL\"");
!
! free(expr);
! plpgsql_dstring_free(&ds);
!
! $$ = NULL;
! break;
!
! default:
! plpgsql_dstring_append(&ds, yytext);
! while ((tok = yylex()) != ';')
! {
! if (tok == 0)
! yyerror("unterminated default value");
!
! if (plpgsql_SpaceScanned)
! plpgsql_dstring_append(&ds, " ");
! plpgsql_dstring_append(&ds, yytext);
! }
! expr->query = strdup(plpgsql_dstring_get(&ds));
! plpgsql_dstring_free(&ds);
!
! $$ = expr;
! break;
! }
}
;
--- 628,636
{ $$ = NULL; }
| decl_defkey
{
! plpgsql_ns_setlocal(false);
! $$ = plpgsql_read_expression(';', ";");
! plpgsql_ns_setlocal(true);
}
;
---(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
--
#==#
# 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] SRA Win32 sync() code
Tom Lane wrote: Jan Wieck <[EMAIL PROTECTED]> writes: Removing sync() entirely requires very accurate fsync()'ing in the background writer, the checkpointer and the backends. Basically none of them can mark a block "clean" if he fails to fsync() the relation later! This will be a mess to code. Not really. The O_SYNC solution for example would be trivial to code. Well, the bgwriter has basically the same chance the checkpointer has ... mdblindwrt() in the end, because he doesn't have the relcache handy. So you want to open(O_SYNC), write(), close() every single block? I don't expect that to be much better than a global sync(). 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] SRA Win32 sync() code
Tom Lane wrote: Bruce Momjian <[EMAIL PROTECTED]> writes: Tom Lane wrote: Seriously though, if we can move the bulk of the writing work into background processes then I don't believe that there will be any significant penalty for regular backends. If the background writer starts using fsync(), we can have normal backends that do a write() set a shared memory boolean. We can then test that boolean and do sync() only if other backends had to do their own writes. That seems like the worst of both worlds --- you still are depending on sync() for correctness. Also, as long as backends only *seldom* do writes, making them fsync a write when they do make one will be less of an impact on overall system performance than having a sync() ensue shortly afterwards. I think you are focusing too narrowly on the idea that backends shouldn't ever wait for writes, and failing to see the bigger picture. What we need to optimize is overall system performance, not an arbitrary restriction that certain processes never wait for certain things. Removing sync() entirely requires very accurate fsync()'ing in the background writer, the checkpointer and the backends. Basically none of them can mark a block "clean" if he fails to fsync() the relation later! This will be a mess to code. 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: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] BEGIN vs START TRANSACTION
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
Re: [PATCHES] bufmgr code cleanup
Neil Conway wrote:
This patch cleans up some of the bufmgr code:
- replace uses of
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
ReleaseBuffer(buf);
with the equivalent, but more concise:
UnlockAndReleaseBuffer(buf);
- analogous changes were made by replacing LockBuffer() + WriteBuffer()
with UnlockAndWriteBuffer()
- remove a bunch of #ifdef BMTRACE code, since it was ugly and broken
anyway
- remove an unused buffer descriptor bit flag (BM_PRIVATE)
- move the definition of INVALID_DESCRIPTOR to out of bufmgr.h and into
freelist.c, since it is the only file that uses it
- remove another unused function, and fix a few comments
Please apply to the CVS HEAD.
Can this be held off a little while we're experimenting with
improvements to the buffer algorithms?
Jan
-Neil
Index: src/backend/access/hash/hashpage.c
===
RCS file: /var/lib/cvs/pgsql-server/src/backend/access/hash/hashpage.c,v
retrieving revision 1.42
diff -c -r1.42 hashpage.c
*** src/backend/access/hash/hashpage.c 4 Sep 2003 22:06:27 - 1.42
--- src/backend/access/hash/hashpage.c 31 Oct 2003 22:55:59 -
***
*** 135,142
void
_hash_relbuf(Relation rel, Buffer buf)
{
! LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buf);
}
/*
--- 135,141
void
_hash_relbuf(Relation rel, Buffer buf)
{
! UnlockAndReleaseBuffer(buf);
}
/*
***
*** 166,173
void
_hash_wrtbuf(Relation rel, Buffer buf)
{
! LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! WriteBuffer(buf);
}
/*
--- 165,171
void
_hash_wrtbuf(Relation rel, Buffer buf)
{
! UnlockAndWriteBuffer(buf);
}
/*
Index: src/backend/access/heap/heapam.c
===
RCS file: /var/lib/cvs/pgsql-server/src/backend/access/heap/heapam.c,v
retrieving revision 1.157
diff -c -r1.157 heapam.c
*** src/backend/access/heap/heapam.c 1 Oct 2003 21:30:52 - 1.157
--- src/backend/access/heap/heapam.c 31 Oct 2003 22:59:14 -
***
*** 899,906
*/
if (!ItemIdIsUsed(lp))
{
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
*userbuf = InvalidBuffer;
tuple->t_datamcxt = NULL;
tuple->t_data = NULL;
--- 899,905
*/
if (!ItemIdIsUsed(lp))
{
! UnlockAndReleaseBuffer(buffer);
*userbuf = InvalidBuffer;
tuple->t_datamcxt = NULL;
tuple->t_data = NULL;
***
*** 1006,1013
}
if (invalidBlock)
{
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
return NULL;
}
--- 1005,1011
}
if (invalidBlock)
{
! UnlockAndReleaseBuffer(buffer);
return NULL;
}
***
*** 1033,1040
!ItemPointerEquals(tid, &ctid))
linkend = false;
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
if (!valid)
{
--- 1031,1037
!ItemPointerEquals(tid, &ctid))
linkend = false;
! UnlockAndReleaseBuffer(buffer);
if (!valid)
{
***
*** 1174,1181
END_CRIT_SECTION();
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! WriteBuffer(buffer);
/*
* If tuple is cachable, mark it for invalidation from the caches in
--- 1171,1177
END_CRIT_SECTION();
! UnlockAndWriteBuffer(buffer);
/*
* If tuple is cachable, mark it for invalidation from the caches in
***
*** 1253,1260
if (result == HeapTupleInvisible)
{
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
elog(ERROR, "attempted to delete invisible tuple");
}
else if (result == HeapTupleBeingUpdated && wait)
--- 1249,1255
if (result == HeapTupleInvisible)
{
! UnlockAndReleaseBuffer(buffer);
elog(ERROR, "attempted to delete invisible tuple");
}
else if (result == HeapTupleBeingUpdated && wait)
***
*** 1301,1308
result == HeapTupleUpdated ||
result == HeapTupleBeingUpdated);
*ctid = tp.t_data->t_ctid;
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
return result;
}
--- 1296,1302
result == HeapTupleUpdated ||
result == HeapTupleBeingUpdated);
*ctid = tp.t_data->t_ctid;
! UnlockAndReleaseBuffer(buffer);
return result;
}
***
*** 1483,1490
if (result == HeapTupleInvisible)
{
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! ReleaseBuffer(buffer);
elog(ERROR, "attempted to update invisible tuple");
}
else if (result == HeapTupleBeingUpdated && wait)
--- 1477,1483
if (result == HeapTupleInvisible)
{
! UnlockAndReleaseBuffer(buffer);
elog(ERROR, "attempted to update invisible tuple");
}
else if (result == HeapTupleBeingUpdated && wait)
***
*** 1531,1538
result == HeapTupleUpdated ||
Re: [PATCHES] [HACKERS] Are we losing momentum?
Bruce Momjian wrote: I assume we agreed against adding a MySQL mode --- just verifying. We agreed that applications that need schema information are much better off using the schema views. Jan --- Sean Chittenden wrote: > > That's a pretty reasonable thought. I work for a shop that sells > > Postgres support, and even we install MySQL for the Q&D ticket > > tracking system we recommend because we can't justify the cost to > > port it to postgres. If the postgres support were there, we would > > surely be using it. > > > How to fix such a situation, I'm not sure. "MySQL Compatability > > Mode," anyone? :-) > > What issues are creating a compatibility problem for you? I don't think these should be hacked into the backend/libpq, but I think it'd be a huge win to hack in "show *" support into psql for MySQL users so they can type: SHOW (databases|tables|views|functions|triggers|schemas); I have yet to meet a MySQL user who understands the concept of system catalogs even though it's just the 'mysql' database (this irritates me enough as is)... gah, f- it: mysql users be damned, I have three developers that think that postgresql is too hard to use because they can't remember "\d [table name]" and I'm tired of hearing them bitch when I push using PostgreSQL instead of MySQL. I have better things to do with my time than convert their output to PostgreSQL. Here goes nothing... I've tainted psql and added a MySQL command compatibility layer for the family of SHOW commands (psql [-m | --mysql]). The attached patch does a few things: 1) Implements quite a number of SHOW commands (AGGREGATES, CASTS, CATALOGS, COLUMNS, COMMENTS, CONSTRAINTS, CONVERSIONS, DATABASES, DOMAINS, FUNCTIONS, HELP, INDEX, LARGEOBJECTS, NAMES, OPERATORS, PRIVILEGES, PROCESSLIST, SCHEMAS, SEQUENCES, SESSION, STATUS, TABLES, TRANSACTION, TYPES, USERS, VARIABLES, VIEWS) SHOW thing SHOW thing LIKE pattern SHOW thing FROM pattern SHOW HELP ON (topic || ALL); etc. Some of these don't have \ command eqiv's. :( I was tempted to add them, but opted not to for now, but it'd certainly be a nice to have. 2) Implements the necessary tab completion for the SHOW commands for the tab happy newbies/folks out there. psql is more friendly than mysql's CLI now in terms of tab completion for the show commands. 3) Few trailing whitespace characters were nuked 4) guc.c is now in sync with the list of available variables used for tab completion Few things to note: 1) SHOW INDEXES is the same as SHOW INDEX, I think MySQL is wrong in this regard and that it should be INDEXES to be plural along with the rest of the types, but INDEX is preserved for compatibility. 2) There are two bugs that I have yet to address 1) SHOW VARIABLES doesn't work, but "SHOW [TAB][TAB]y" does 2) "SHOW [variable_of_choice];" doesn't work, but "SHOW [variable_of_choice]\n;" does work... not sure where this problem is coming from 3) I think psql is more usable as a result of this more verbose syntax, but it's not the prettiest thing on the planet (wrote a small parser outside of the backend or libraries: I don't want to get those dirty with MySQL's filth). 4) In an attempt to wean people over to PostgreSQL's syntax, I included translation tips on how to use the psql equiv of the SHOW commands. Going from SHOW foo to \d foo is easy, going from \d foo to SHOW foo is hard and drives me nuts. This'll help userbase retention of newbies/converts. :) 5) The MySQL mode is just a bounce layer that provides different syntax wrapping exec_command() so it should provide little in the way of maintenance headaches. Some of the SHOW commands, however, don't have \ couterparts, but once they do and that code is centralized, this feature should come for zero cost. 6) As an administrator, I'd be interested in having an environment variable that I could set that'd turn on MySQL mode for some of my bozo users that way they don't complain if they forget the -m switch. Thoughts? I'll try and iron out the last of those two bugs/features, but at this point, would like to see this patch get wider testing/feedback. Comments, as always, are welcome. PostgreSQL_usability++ -sc -- Sean Chittenden [ Attachment, skipping... ] ---(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 -- #==# # 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)-
