Re: [PATCHES] pg_postmaster_reload_time() patch
On Wed, Apr 30, 2008 at 9:53 AM, George Gensure [EMAIL PROTECTED] wrote: I've done a quick write up for reload time reporting from the administration TODO. I was a little paranoid with the locking, but didn't want problems to occur with signals on the postmaster and the read side. IMHO, the function should return NULL if the postmaster never reloaded; that is, it was started, but never signaled to reload. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
Re: [PATCHES] [HACKERS] [Fwd: Index Advisor]
On 4/12/07, Bruce Momjian [EMAIL PROTECTED] wrote: Gurjeet Singh wrote: The interface etc. may not be beautiful, but it isn't ugly either! It is a lot better than manually creating pg_index records and inserting them into cache; we use index_create() API to create the index (build is deferred), and then 'rollback to savepoint' to undo those changes when the advisor is done. index_create() causes pg_depends entries too, so a 'RB to SP' is far much safer than going and deleting cache records manually. My complaint was not that the API used in the code was non-optimal(which I think was Tom's issue), but that the _user_ API was not very clean. Not sure what to recommend, but I will think about it later. That can be fixed/improved with minimal efforts, but if it is the internal API usage, or the architecture we're bothered about, then IMO just an overhaul of the code will not be sufficient, rather, it will require rework from scratch. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com 17°29'34.37N 78°30'59.76E - Hyderabad 18°32'57.25N 73°56'25.42E - Pune *
Re: [PATCHES] [HACKERS] [Fwd: Index Advisor]
. (A total of three places to touch). And then we'll need some extra code in the core to generate the advisory (in some form; maybe into a user table, or as part of the EXPLAIN output; but remember, not every query can be EXPLAINed!). Then, we also need logic in all these places to differentiate the normal run from the v-index enabled run, else we'll end up generating IndexOptInfo everytime we enter get_relation_info(). And this differentiation needs to be done in EXPLAIN code too. Also, although the whole plan-tree is available in get_relation_info(), but it wouldn't be the right place to scan other tables, for eg., for generating JOIN-INDEXes or materializing some intermediate joins. (sometime in the future we may support them!). If we don't run the planner twice, then the developer will have to run it manually twice, and compare the costs manually (with and without v-indexes); virtually impossible for lage applications and introduction of another human-error possibility. (I just noticed that you quoted the line from the mail where I submitted version 23 of the patch, the plugin architecture wasn't utilized; please refer to the mail that has 'pg_post_planner_plugin-HEAD_20070116-v2.patch.gz' and 'pg_index_adviser-HEAD_20070116-v26.patch.gz' as attachments; dated 20.Jan.07) About the right place to call the plugin... calling it immediately after the planner is done with normal planning phase seems to be right. At this point planner is done and no other part of the backend yet knows about what plan is generated; so the plugin has a chance to modify the plan in place and do it's trickery in a completely isolated time-space. (maybe we can pass a reference to the plan pointer, and let the plugin replace the whole plan itself using this reference!) I surely agree that it is time-consuming (less efficient), but it is completely automated, with the least of human interference or application change required; hence, on the whole, it must be a million times faster than a human sitting down, extracting every query - prepending EXPLAIN to it - and executing it - twice - comparing the resulting cost!!! Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com 17°29'34.37N 78°30'59.76E - Hyderabad 18°32'57.25N 73°56'25.42E - Pune * On 4/6/07, Tom Lane [EMAIL PROTECTED] wrote: Gurjeet Singh [EMAIL PROTECTED] writes: Please find attached the latest version of the patch. It applies cleanly on REL8_2_STABLE. The interface to the planner in this seems rather brute-force. To run a plan involving a hypothetical index, you have to make a bunch of catalog entries, run the planner, and then roll back the transaction to get rid of the entries. Slow, ugly, and you still need another kluge to keep the planner from believing the index has zero size. It strikes me that there is a better way to do it, because 99% of the planner does not look at the system catalog entries --- all it cares about is the IndexOptInfo structs set up by plancat.c. So there's not really any need to make catalog entries at all AFAICS. Rather, the best thing would be a plugin hook at the end of get_relation_info() that would have a chance to editorialize on the constructed IndexOptInfo list (and maybe other properties of the RelOptInfo too). You could remove existing index entries or insert new ones. I'm dissatisfied with the hard-wired hook into planner() also. That doesn't provide any extensibility nor allow the index adviser to be implemented as a loadable plugin. I'm inclined to think it's in the wrong place anyway; you've got thrashing around there to avoid recursion but it's very fragile. Having to dump the results into the postmaster log isn't a nice user API either. Perhaps what would be better is a hook in EXPLAIN to call a plugin that can add more lines to EXPLAIN's output, and is passed the original query and plan so that it can re-call the planner with hypothetical indexes prepared for insertion by the other hook. regards, tom lane
Re: [PATCHES] \prompt for psql
On 2/9/07, Alvaro Herrera [EMAIL PROTECTED] wrote: At least it'd help those poor people trying to do conditional COMMIT or ROLLBACK based on the transaction status. The user doesn't need to check the status of the transaction if he just needs to end it. Just fire the END command and it'll take care of whether to COMMIT or ROLLBACK the transaction: edb=# begin; BEGIN edb=# select count(*) from pg_class; count --- 280 (1 row) edb=# select count(*) from pg_class_xyz; ERROR: relation pg_class_xyz does not exist edb=# end; ROLLBACK edb=# Regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [PATCHES] \prompt for psql
On 2/8/07, Chad Wagner [EMAIL PROTECTED] wrote: This adds the ability to prompt for internal variable input, below are examples: In help.c:slashUsage(), the comment says: /* if you add/remove a line here, change the row count above */ So I guess, the patch should also include a change to the line: output = PageOutput(67, pager); = output = PageOutput(68, pager); Regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
On 1/20/07, Bruce Momjian [EMAIL PROTECTED] wrote: I can't read a 7z file on my end. Please email me the file and I will put it at a URL. --- Gurjeet Singh wrote: Please find attached the patches ported to HEAD as of now. The patch to the contrib modules is the same as before; the version number has been kept but branch designator has been changed. 1) pg_post_planner_plugin-HEAD_20070116-v2.patch.gz 2) pg_index_adviser-HEAD_20070116-v26.7z I am attaching the .gz versions of both the patches, and CC'ing to -patches also. If it doesn't turn up on -patches even this time, then please do the needful. Thanks and best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_post_planner_plugin-HEAD_20070116-v2.patch.gz Description: GNU Zip compressed data pg_index_adviser-HEAD_20070116-v26.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
On 1/15/07, Andrew Dunstan [EMAIL PROTECTED] wrote: Gurjeet Singh wrote: 1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz 2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz Why are these patches against 8.2 rather than CVS HEAD? Is this not a new feature? We never backport new features to the stable branches - that's what makes them stable ;-) Please find attached the patches ported to HEAD as of now. The patch to the contrib modules is the same as before; the version number has been kept but branch designator has been changed. 1) pg_post_planner_plugin-HEAD_20070116-v2.patch.gz 2) pg_index_adviser-HEAD_20070116-v26.7z Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_post_planner_plugin-HEAD_20070116-v2.patch.gz Description: GNU Zip compressed data pg_index_adviser-HEAD_20070116-v26.7z Description: Binary data ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
Hi Bruce, I have not been able to send this file across since the last two days. In the past I have been able to send 31KB attachments to patches, but I donno why it's not getting through this time. I have tried different levels of compression in different formats, and still it won't let the mail through. If it doesn't get through to the list even this time, can you please do the needful and get it posted on the patches. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com This is the second installment of the Index Adviser patch. It contains the item number (2) mentioned below. On 1/13/07, Gurjeet Singh [EMAIL PROTECTED] wrote: Hi All, Please find attached two patches: 1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz 2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_index_adviser-REL8_2_STABLE-v26.7z Description: Binary data ---(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: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
Please find attached two patches: 1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz 2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz Why are these patches against 8.2 rather than CVS HEAD? Is this not a new feature? We never backport new features to the stable branches - that's what makes them stable ;-) Good point... I always worked only on a tag or a branch, knowing that it is in some stable state, and won't have to be bothered by critical (and sometimes un-compilable) changes to the source tree. These patches *should* apply cleanly to the head too, since the first one touches the code in very stable places and the second one contains only new files. I'll port these patches to the head and get back to the list. Thanks, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [pgsql-patches] [PATCHES] [HACKERS] [Fwd: Index Advisor]
It seems the size restriction has blocked my previous attempt. Please find the first patch attached, and the second one will be in the next mail. Best Regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com On 1/13/07, Gurjeet Singh [EMAIL PROTECTED] wrote: On 1/9/07, Gurjeet Singh [EMAIL PROTECTED] wrote: Now that there's just one call to the Index Adviser (from planner()) we can now move forward in making it a plugin. Hi All, Please find attached two patches: 1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz 2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz Patch 1 introduces the infrastructure to call plugins from the tail-end of the planner() function. The planner looks for a list of PPPFunctions (PostPlannerPluginFunctions) in a rendezvous variable, and then calls the 'driver' callback into the plugin. This patch also adds a new function in explain.c that can be used to generate a string similar to the output of the EXPLAIN command. It also adds a harmless DLLIMPORT to some global variables that were needed by the Index Adviser Plugin. Patch 2 is the plugin version of the Index Adviser and the advise tool. It creates two folders in the contrib module: pg_index_adviser and pg_advise_index. The pg_index_adviser folder contains the updated README. Both the folders contain their respective updated sample_*.[sql|txt] files. Theres one point that needs attention in the patch 1. The code enclosed in GLOBAL_CAND_LIST is a hack, which I couldn't get rid of. In plancat.c we have two options to estimate the number of pages that would be occupied by a virtual index: i) Make a call back into the plugin to get the estimation. The code enabled by GLOBAL_CAND_LIST implements this. ii) We can allow the plugin to update the pg_class.relpages entry for each virtual index, and the planner will pickup the values from there. The code disabled by GLOBAL_CAND_LIST implements this. Option (ii) would be ideal but the core members can be a better judge of this. Is there any other way of doing this? Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] [Fwd: Index Advisor]
On 1/9/07, Gurjeet Singh [EMAIL PROTECTED] wrote: I have another idea for making the hooks a bit more cleaner; I will try that and run it through you guys later today. Please find attached the latest version of the patch. It applies cleanly on REL8_2_STABLE. The following restrictions are applied before generating an index candidate (this is not mentioned in the README): .) It should be a base relation; We do not generate advisory for a view or Set Returning Function or something else. .) It should not be a system table; like pg_class etc. .) It should not be a temporary table. (May be we can allow these; opinions please!!) .) We do not recommend indexes on system columns; like oid, ctid etc. .) The relation should have at least two pages. .) The relation should have at least two tuples. The last two restrictions put the onus on the user to keep the table ANALYZEd or VACUUMed. I have moved the calls to index_adviser() from two other places to the end of planner(). IMO, that is the best place to place a call to the Adviser; instead of duplicating code in every caller of planner(). index_adviser() makes sure that it doesn't get called recursively. This change however costs us the loss of ability to append suggested plan to the existing plan, if being called by the EXPLAIN command. Instead, it now uses the newly written function explain_getPlanString() in explain.cto get the string representation of the plan, and then emits it as elog( LOG,...). The only kludge left now is the code enclosed in '#if GLOBAL_CAND_LIST' in plancat.c. We need to decide whether we need the 'if' part or the 'else' part! I already see a strong objection to the 'else' option, since it is very close to the core of the optimizer! Opinions needed. After adding the CREATE TABLE advise_index(...) script to src/test/regress/sql/create_table.sql and enabling the GUC in the conf file, 'make installcheck' runs fine, with a few acceptable diffs. Moreover, post the 'make' run, we can see there are a few advises for the tables involved in the test run. Four of them are ob serial columns of hash-index-testing tables, so they don't make much point; but the rest three of them are on big tables, and one of them is a multi-column-index suggestion. Now that there's just one call to the Index Adviser (from planner()) we can now move forward in making it a plugin. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_index_adviser-REL8_2_STABLE-v23.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] A patch to pg_regress for Windows port
On Windows, if logged in as an Administrator, 'make check' fails with our standard error, saying: quote Execution of PostgreSQL by a user with administrative permissions is not permitted. The server must be started under an unprivileged user ID to prevent possible system security compromises. See the documentation for more information on how to properly start the server. /quote But, for some time now, a Windows' Administrator *can* run a postgres without having to create a normal user; he just has to use pg_ctl to do so. Thanks to code written by Magnus, pg_ctl gives up it's administrative privilges before starting the postmaster, and hence the postmaster runs without critical privileges. So I thought that 'make check' could also make use of that functionality, and hence this patch. Is this new pg_ctl behavior mentioned anywhere in the docs? Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_regress.c.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] A patch to pg_regress for Windows port
This patch removes double-quotes from around the listen_addresses=%s part; I couldn't find a way of doing that. But then, the questions is, can the %s (hostname) have spaces embedded in it? -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com On 1/6/07, Gurjeet Singh [EMAIL PROTECTED] wrote: On Windows, if logged in as an Administrator, 'make check' fails with our standard error, saying: quote Execution of PostgreSQL by a user with administrative permissions is not permitted. The server must be started under an unprivileged user ID to prevent possible system security compromises. See the documentation for more information on how to properly start the server. /quote But, for some time now, a Windows' Administrator *can* run a postgres without having to create a normal user; he just has to use pg_ctl to do so. Thanks to code written by Magnus, pg_ctl gives up it's administrative privilges before starting the postmaster, and hence the postmaster runs without critical privileges. So I thought that 'make check' could also make use of that functionality, and hence this patch. Is this new pg_ctl behavior mentioned anywhere in the docs? Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [PATCHES] A patch to pg_regress for Windows port
cool... On 1/6/07, Magnus Hagander [EMAIL PROTECTED] wrote: Gurjeet Singh wrote: On Windows, if logged in as an Administrator, 'make check' fails with our standard error, saying: quote Execution of PostgreSQL by a user with administrative permissions is not permitted. The server must be started under an unprivileged user ID to prevent possible system security compromises. See the documentation for more information on how to properly start the server. /quote But, for some time now, a Windows' Administrator *can* run a postgres without having to create a normal user; he just has to use pg_ctl to do so. Thanks to code written by Magnus, pg_ctl gives up it's administrative privilges before starting the postmaster, and hence the postmaster runs without critical privileges. So I thought that 'make check' could also make use of that functionality, and hence this patch. Per previous discussion with Tom (in the archives, but I can't get there right now), this is the wrong way to do it. We lose the ability to kill the postmaster if it fails. I have a proper working solution in my tree that I will submit soon along with the changes required to make pg_regress work in a non-msys environment using MSVC. //Magnus -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [PATCHES] A patch to pg_regress for Windows port
On 1/6/07, Tom Lane [EMAIL PROTECTED] wrote:
Gurjeet Singh [EMAIL PROTECTED] writes:
This patch removes double-quotes from around the listen_addresses=%s
part; I
couldn't find a way of doing that. But then, the questions is, can the
%s
(hostname) have spaces embedded in it?
Yes, because it can be more than one hostname.
But the code in postmaster.c expects the list to be comma separated.
if (!SplitIdentifierString(rawstring, ',', elemlist))
{
/* syntax error in list */
ereport(FATAL,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(invalid list syntax for
\listen_addresses\)));
}
Why do you want to
remove the quotes?
No, I didn't want to, but I was getting FATAL: the database system is
starting up in the log if I surrounded that var=val with double-quotes.
Later I remedied that by attaching a -w option to the pg_ctl command.
PS: there wasn't any patch attached,
Please refer
http://archives.postgresql.org/pgsql-patches/2007-01/msg00056.php
but I doubt we'd have accepted it
anyway ...
Any particular reason?
A few minutes after the submission, after I read more code, I wouldn't have
either! I noticed (and Magnus also pointed it out) that we use the returned
PID to kill the postmaster if it doesn't respond. There's no easy way of
doing that if we use pg_ctl to start the postmaster! Let's wait for Magnus'
patch to make pg_regress independent of MinGW.
Best regards,
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | yahoo }.com
Re: [PATCHES] [HACKERS] [Fwd: Index Advisor]
Hi All, Please find attached the latest version of the patch attached. It is based on REL8_2_STABLE. It includes a few bug fixes and an improvement to the size estimation function. It also includes a work-around to circumvent the problem we were facing earlier in xact.c; it now fakes itself to be a PL/xxx module by surrounding the BIST()/RARCST() calls inside an SPI_connect()/SPI_finish() block. Please note that the sample_*.txt files in the contrib module, which show a few different sample runs, may be a little out of date. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | yahoo }.com pg_index_adviser-REL8_2_STABLE20061208-v22.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
I have made the changes to implement this TODO item. I wish to know the standard procedure (command) to generate a patch; and from which level in the source directory should I execute it? On 5/18/06, Bruce Momjian pgman@candle.pha.pa.us wrote: Added to TODO: * Add a GUC to control whether BEGIN inside a transcation should abort the transaction. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
On 5/26/06, Alvaro Herrera [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
I wish to
know the standard procedure (command) to generate a patch; and from
which level in the source directory should I execute it?
The toplevel directory. The command is
cvs -q diff -cp
If you created new files to implement a patch, include them separately,
indicating in the patch description in which directory they are meant to
reside.
Thanks.
Here's the patch:
Index: src/backend/access/transam/xact.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.220
diff -c -p -r1.220 xact.c
*** src/backend/access/transam/xact.c 25 Apr 2006 00:25:17 - 1.220
--- src/backend/access/transam/xact.c 21 May 2006 15:40:00 -
*** boolXactReadOnly;
*** 59,64
--- 59,65
intCommitDelay = 0;/* precommit delay in
microseconds */
intCommitSiblings = 5; /* # concurrent xacts needed to
sleep */
+ bool BeginInXactIsError = true;
/*
* transaction states - transaction state from server perspective
*** BeginTransactionBlock(void)
*** 2725,2731
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg(there is already a transaction in
progress)));
break;
--- 2726,2732
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(ERROR,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg(there is already a transaction in
progress)));
break;
Index: src/backend/utils/misc/guc.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.318
diff -c -p -r1.318 guc.c
*** src/backend/utils/misc/guc.c2 May 2006 11:28:55 - 1.318
--- src/backend/utils/misc/guc.c21 May 2006 15:14:22 -
***
*** 65,70
--- 65,71
#include utils/pg_locale.h
#include pgstat.h
#include access/gin.h
+ #include access/xact.h
#ifndef PG_KRB_SRVTAB
#define PG_KRB_SRVTAB
*** static struct config_bool ConfigureNames
*** 1005,1010
--- 1006,1021
false, NULL, NULL
},
+ {
+ {begin_inside_transaction_is_error, PGC_USERSET,
COMPAT_OPTIONS,
+gettext_noop(A BEGIN statement inside a transaction raises
ERROR.),
+gettext_noop(It is provided to catch buggy applications.
+ Disable it to let the buggy application
run.)
+ },
+ BeginInXactIsError,
+ true, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
Index: src/include/access/xact.h
===
RCS file: /projects/cvsroot/pgsql/src/include/access/xact.h,v
retrieving revision 1.82
diff -c -p -r1.82 xact.h
*** src/include/access/xact.h 25 Apr 2006 00:25:19 - 1.82
--- src/include/access/xact.h 21 May 2006 15:13:10 -
*** extern int XactIsoLevel;
*** 41,46
--- 41,49
extern bool DefaultXactReadOnly;
extern bool XactReadOnly;
+ /* A BEGIN statement inside a transaction raises ERROR */
+ extern bool BeginInXactIsError;
+
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
*/
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [HACKERS] BEGIN inside transaction should be an error
On 5/26/06, Andrew Dunstan [EMAIL PROTECTED] wrote:
Please read the developers FAQ:
http://www.postgresql.org/docs/faqs.FAQ_DEV.html
For the most part, patches are probably best generated relative to the
root directory of your CVS checkout.
cheers
andrew
Gurjeet Singh wrote:
I wish to
know the standard procedure (command) to generate a patch; and from
which level in the source directory should I execute it?
On 5/18/06, Bruce Momjian pgman@candle.pha.pa.us wrote:
Added to TODO:
* Add a GUC to control whether BEGIN inside a transcation
should abort
the transaction.
Thanks Andrew.
Reposting the patch since my version on guc.c wasn't at the HEAD.
Index: src/backend/access/transam/xact.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.220
diff -c -p -r1.220 xact.c
*** src/backend/access/transam/xact.c 25 Apr 2006 00:25:17 - 1.220
--- src/backend/access/transam/xact.c 21 May 2006 15:40:00 -
*** boolXactReadOnly;
*** 59,64
--- 59,65
intCommitDelay = 0;/* precommit delay in
microseconds */
intCommitSiblings = 5; /* # concurrent xacts needed to
sleep */
+ bool BeginInXactIsError = true;
/*
* transaction states - transaction state from server perspective
*** BeginTransactionBlock(void)
*** 2725,2731
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg(there is already a transaction in
progress)));
break;
--- 2726,2732
case TBLOCK_SUBINPROGRESS:
case TBLOCK_ABORT:
case TBLOCK_SUBABORT:
! ereport(ERROR,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg(there is already a transaction in
progress)));
break;
Index: src/backend/utils/misc/guc.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.320
diff -c -p -r1.320 guc.c
*** src/backend/utils/misc/guc.c21 May 2006 20:10:42 - 1.320
--- src/backend/utils/misc/guc.c26 May 2006 16:10:40 -
***
*** 66,71
--- 66,72
#include utils/pg_locale.h
#include pgstat.h
#include access/gin.h
+ #include access/xact.h
#ifndef PG_KRB_SRVTAB
#define PG_KRB_SRVTAB
*** static struct config_bool ConfigureNames
*** 1008,1013
--- 1009,1024
false, NULL, NULL
},
+ {
+ {begin_inside_transaction_is_error, PGC_USERSET,
COMPAT_OPTIONS,
+gettext_noop(A BEGIN statement inside a transaction raises
ERROR.),
+gettext_noop(It is provided to catch buggy applications.
+ Disable it to let the buggy application
run.)
+ },
+ BeginInXactIsError,
+ true, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
Index: src/include/access/xact.h
===
RCS file: /projects/cvsroot/pgsql/src/include/access/xact.h,v
retrieving revision 1.82
diff -c -p -r1.82 xact.h
*** src/include/access/xact.h 25 Apr 2006 00:25:19 - 1.82
--- src/include/access/xact.h 21 May 2006 15:13:10 -
*** extern int XactIsoLevel;
*** 41,46
--- 41,49
extern bool DefaultXactReadOnly;
extern bool XactReadOnly;
+ /* A BEGIN statement inside a transaction raises ERROR */
+ extern bool BeginInXactIsError;
+
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
*/
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
