Re: [PATCHES] options for RAISE statement
Hello I am sending enhanced version of original patch. 2008/5/5 Tom Lane <[EMAIL PROTECTED]>: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> this patch adds possibility to set additional options (SQLSTATE, >> DETAIL, DETAIL_LOG and HINT) for RAISE statement, > > I looked this over briefly. A couple of comments: > > * Raising errors via hard-coded SQLSTATEs seems pretty unfriendly, > at least for cases where we are reporting built-in errors. Wouldn't > it be better to be able to raise errors using the same SQLSTATE names > that are recognized in EXCEPTION clauses? There are new attribut CONDITION - all defined condition are possible without duplicit names and category conditions. example: RAISE NOTICE 'custom unique violation' USING (CONDITION = 'unique_violation'); > > * If we are going to let people throw random SQLSTATEs, there had better > be a way to name those same SQLSTATEs in EXCEPTION. > we can trap EXCEPTION defined via SQLSTATE now: EXCEPTION WHEN SQLSTATE 22001 THEN ... > * I don't really like exposing DETAIL_LOG in this. That was a spur of > the moment addition and we might take it out again; I think it's way > premature to set it in stone by exposing it as a plpgsql feature. removed > > * Please avoid using errstart() directly. This is unwarranted intimacy > with elog.h's implementation and I also think it will have unpleasant > behavior if an error occurs while evaluating the RAISE arguments. > (In fact, I think a user could easily force a backend PANIC that way.) > The approved way to deal with ereport options that might not be there > is like this: > >ereport(ERROR, >( ..., > have_sqlstate ? errcode(...) : 0, > ... > > That is, you should evaluate all the options into local variables > and then do one normal ereport call. > changed > * // comments are against our coding conventions. > >regards, tom lane > removed Regards Pavel Stehule *** ./doc/src/sgml/plpgsql.sgml.orig 2008-05-06 11:05:05.0 +0200 --- ./doc/src/sgml/plpgsql.sgml 2008-05-10 01:09:54.0 +0200 *** *** 2184,2192 --- 2184,2197 WHEN condition OR condition ... THEN handler_statements WHEN condition OR condition ... THEN + handler_statements + WHEN SQLSTATE x OR SQLSTATE x ... THEN + handler_statements + WHEN SQLSTATE x OR condition ... THEN handler_statements ... END; + *** *** 2215,2221 condition name OTHERS matches every error type except QUERY_CANCELED. (It is possible, but often unwise, to trap QUERY_CANCELED by name.) Condition names are ! not case-sensitive. --- 2220,2227 condition name OTHERS matches every error type except QUERY_CANCELED. (It is possible, but often unwise, to trap QUERY_CANCELED by name.) Condition names are ! not case-sensitive. Any condition can be subtituted by SQLSTATE ! value. *** *** 2243,2248 --- 2249,2262 RAISE NOTICE 'caught division_by_zero'; RETURN x; END; + + ... + -- or same with SQLSTATE specification + EXCEPTION + WHEN SQLSTATE 22012 THEN + RAISE NOTICE 'caught division_by_zero'; + RETURN x; + END; When control reaches the assignment to y, it will *** *** 2832,2838 raise errors. ! RAISE level 'format' , expression , ...; Possible levels are DEBUG, --- 2846,2852 raise errors. ! RAISE level 'format' , expression , ... USING ( option = expression , ... ) ; Possible levels are DEBUG, *** *** 2875,2891 This example will abort the transaction with the given error message: ! RAISE EXCEPTION 'Nonexistent ID --> %', user_id; ! RAISE EXCEPTION presently always generates ! the same SQLSTATE code, P0001, no matter what message it is invoked with. It is possible to trap this exception with EXCEPTION ... WHEN RAISE_EXCEPTION THEN ... but there is no way to tell one RAISE from another. --- 2889,2919 This example will abort the transaction with the given error message: ! RAISE EXCEPTION 'Nonexistent ID --> %', user_id USING (hint = 'Please, check your user id'); ! RAISE EXCEPTION presently generates ! the same SQLSTATE code, P0001 , no matter what message it is invoked with. It is possible to trap this exception with EXCEPTION ... WHEN RAISE_EXCEPTION THEN ... but there is no way to tell one RAISE from another. + + + With additional options is possible set some log informaition related to + raised exception. Possible options are SQLSTATE, + CONDTION, D
Re: [PATCHES] Database owner installable modules patch
Where are we on this?
---
Tom Dunstan wrote:
> Hi all
>
> Here is a patch that provides an initial implementation of the module
> idea that was kicked around over the last few days. While there
> certainly wasn't consensus on list, enough people seemed interested in
> the idea of database-owner-installable modules that I thought it was
> worth having a play with.
>
> The general idea, to recap, is to have modules, whether included in
> the distribution a la contrib or installed separately, installed under
> a directory such as $pkglib_dir/modules/foo. A typical module
> directory might contain:
> - foo.so/foo.dll
> - install.sql
> - uninstall.sql
> - foo.conf
> - some-other-file-needed-by-foo-module.dat
> The module would be installed on the system, but the necessary scripts
> to install it in a particular database have not been run. In
> particular, the modules would not usually be install in template1.
> Database owners themselves can then opt to enable a particular
> installed module in their own database - they do not have to hassle a
> sysadmin to do it for them.
>
>
> Features of the patch:
> - A database owner can issue the command "INSTALL MODULE foo", and
> pgsql will look for a $pkglib_dir/modules/foo/install.sql file to run,
> and run it.
>
> - The install script can do pretty much anything - the user is
> treated as the superuser for the duration of the script. The main and
> obvious usage is to create C language functions required by the
> module.
>
> - An entry is created in a new pg_module catalog. This is mainly to
> guard against someone trying to install a module twice at this point,
> but it may have other uses in the future (see below).
>
> - "UNINSTALL MODULE foo" looks for and executes
> $pkglib_dir/modules/foo/uninstall.sql and cleans up the catalog.
>
>
>
> Here is a list of things that are either still to do before I'd
> consider it worthy of inclusion (should the general approach be
> considered acceptable), or which I'd like some guidance on:
>
> - Currently the script is executed in one big SPI_execute call, and
> so errors and NOTICEs print out the entire script as context. I'm not
> sure how to break it up without writing a full parser - we must have
> something available in the backend to break a string up into multiple
> statements to execute, but I'm not sure where to look. Also, is there
> a better way to do this than SPI?
>
> - I've hacked in a bit of an end-run around permissions checks to
> make the current user look like a super-user while a module script is
> running. Is there a better way to do this?
>
> - I can't create C language functions from dlls under the modules
> dir. I'd like to be able to specify 'modules/foo/foo' as the library
> name, but the backend sees a slash and decides that must mean the path
> is absolute. I see two ways to fix this: change the existing code in
> dfmgr.c to *really* check for absolute/relative paths rather than the
> current hack, or I could stick in a special case for when it starts
> with "modules/". I thought I'd get some guidance on-list. Do people
> think that sticking the dll in with other resources for the module
> under $pkglib_dir is a bad thing? (I think having everything in one
> place is a very good thing myself).Is the existing check written the
> way it is for a particular reason, or is it just "good enough"?
>
> - It would be nice to create the empty modules dir when we install
> pgsql, but while I suppose hacking a Makefile to install it is the way
> to go, I'm not sure which one would be appropriate.
>
> - Hack pgxs to install stuff into a modules dir if we give it some
> appropriate flag.
>
> - I'd like to add pg_depend entries for stuff installed by the module
> on the pd_module entry, so that you can't drop stuff required by the
> module without uninstalling the module itself. There would have to be
> either a function or more syntax to allow a script to do that, or some
> sort of module descriptor that let the backend do it itself.
>
> - Once the issue of loading a dll from inside the module's directory
> is done, I'd like to look for an e.g. module_install() function inside
> there, and execute that rather than the install.sql if found. Ditto
> for uninstall.
>
> - Maybe a basic mechanism to allow a module to require another one.
> Even just a "SELECT require_module('bar')" call at the top of a
> script.
>
> - It would be nice to suppress NOTICEs when installing stuff - the
> user almost certainly doesn't care.
>
> - Pick up config files in module directories, so that a module can
> install and pick up config for itself rather than getting the sysadmin
> to hack the global custom_variable_classes setting.
>
> - Should plperl etc be done as modules so that their config can live
> independently as well? And to allow modules to "require" them?
>
>
> Some other nice to haves for some point in the f
Re: [PATCHES] printTable API (was: Show INHERIT in \du)
Hi guys, Here's the latest version of the printTable API. This version is against the current HEAD and merges in the changes made by the recently committed psql wrap patch. This version also includes Alvaro's fix for the issue of pg_strdup not being available to programs in scripts/ (as quoted below). Clean compile and all regression tests passed on amd64 gentoo. Cheers, BJ On Thu, May 8, 2008 at 7:55 AM, Alvaro Herrera <[EMAIL PROTECTED]> wrote: > FWIW I just noticed something else. With this patch we add pg_strdup > calls into print.c. pg_strdup lives in common.c. > > This is fine as psql is concerned, but we have another problem which is > that in bin/scripts there are two programs that want to use > printQuery(). The problem is that there's no pg_strdup there :-( > > The easy solution is to add pg_strdup to bin/scripts/common.c, but there > we don't have a global progname, so the error report in the out of > memory case cannot carry the name of the program crashing. > > I don't like that, but I don't see any other solution. Ideas welcome. > > -- > Alvaro Herrerahttp://www.CommandPrompt.com/ > PostgreSQL Replication, Consulting, Custom Development, 24x7 support > print-table-5.diff.bz2 Description: BZip2 compressed data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [PATCH] sh: Add support Renesas SuperH
Where are we on this? --- Tom Lane wrote: > I wrote: > > Nobuhiro Iwamatsu <[EMAIL PROTECTED]> writes: > >> +#if defined(__sh__) /* Renesas SuperH */ > > > Do they have any longer form of that macro? > > I looked into the gcc sources, and the answer seems to be "no" :-(. > So we're stuck with __sh__. > > I'm still pretty skeptical about the adequacy of the asm parameters, > though. > > regards, tom lane > > -- > Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Replace offnum++ by OffsetNumberNext
Tom Lane wrote: > Fujii Masao <[EMAIL PROTECTED]> writes: > > This is the patch replace offnum++ by OffsetNumberNext. > > According to off.h, OffsetNumberNext is the macro prepared to > > disambiguate the different manipulations on OffsetNumbers. > > But, increment operator was used in some places instead of the macro. > > I wonder if we shouldn't go the other way, ie, use ++ everywhere. > OffsetNumberNext seems like just useless obscurantism ... The only value I saw to OffsetNumberNext was the fact is does int16 increment, with casting. There is also the comment: * Increments/decrements the argument. These macros look pointless * but they help us disambiguate the different manipulations on * OffsetNumbers (e.g., sometimes we subtract one from an * OffsetNumber to move back, and sometimes we do so to form a * real C array index). -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
On Fri, May 9, 2008 at 5:37 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Alex Hunsaker" <[EMAIL PROTECTED]> writes: >> [ patch to change inherited-check-constraint behavior ] > > Applied after rather heavy editorializations. You didn't do very well on > getting it to work in multiple-inheritance scenarios, such as > >create table p (f1 int check (f1>0)); >create table c1 (f2 int) inherits (p); >create table c2 (f3 int) inherits (p); >create table cc () inherits (c1,c2); > > Here the same constraint is multiply inherited. The base case as above > worked okay, but adding the constraint to an existing inheritance tree > via ALTER TABLE, not so much. Ouch. Ok Ill (obviously) review what you committed so I can do a lot better next time. Thanks for muddling through it! > I also didn't like the choice to add is_local/inhcount fields to > ConstrCheck: that struct is fairly heavily used, but you were leaving the > fields undefined/invalid in most code paths, which would surely lead to > bugs down the road when somebody expected them to contain valid data. > I considered extending the patch to always set them up, but rejected that > idea because ConstrCheck is essentially a creature of the executor, which > couldn't care less about constraint inheritance. After some reflection > I chose to put the fields in CookedConstraint instead, which is used only > in the table creation / constraint addition code paths. That required > a bit of refactoring of the API of heap_create_with_catalog, but I think > it ended up noticeably cleaner: constraints and defaults are fed to > heap.c in only one format now. That sounds *way* cleaner and hopefully got rid of some of those gross hacks I had to do. Interestingly enough thats similar to how I initially started doing it. But it felt way to intrusive, so i dropped it. Course I then failed the non-intrusive with the ConstrCheck changes... > I found one case that has not really worked as intended for a long time: > ALTER TABLE ADD CHECK ... (that is, ADD CONSTRAINT without specifying > a constraint name) failed to ensure that the same constraint name was used > at child tables as at the parent, and thus the constraints ended up not > being seen as related at all. Fixing this was a bit ugly since it meant > that ADD CONSTRAINT has to recurse to child tables during Phase 2 after > all, and has to be able to add work queue entries for Phase 3 at that > time, which is not something needed by any other ALTER TABLE operation. Ouch... > I'm not sure if we ought to try to back-patch that --- it'd be a > behavioral change with non-obvious implications. In the back branches, > ADD CHECK followed by DROP CONSTRAINT will end up not deleting the > child-table constraints, which is probably a bug but I wouldn't be > surprised if applications were depending on the behavior. Given the lack complaints it does not seem worth a back patch IMHO. >regards, tom lane > -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
"Alex Hunsaker" <[EMAIL PROTECTED]> writes: > [ patch to change inherited-check-constraint behavior ] Applied after rather heavy editorializations. You didn't do very well on getting it to work in multiple-inheritance scenarios, such as create table p (f1 int check (f1>0)); create table c1 (f2 int) inherits (p); create table c2 (f3 int) inherits (p); create table cc () inherits (c1,c2); Here the same constraint is multiply inherited. The base case as above worked okay, but adding the constraint to an existing inheritance tree via ALTER TABLE, not so much. I also didn't like the choice to add is_local/inhcount fields to ConstrCheck: that struct is fairly heavily used, but you were leaving the fields undefined/invalid in most code paths, which would surely lead to bugs down the road when somebody expected them to contain valid data. I considered extending the patch to always set them up, but rejected that idea because ConstrCheck is essentially a creature of the executor, which couldn't care less about constraint inheritance. After some reflection I chose to put the fields in CookedConstraint instead, which is used only in the table creation / constraint addition code paths. That required a bit of refactoring of the API of heap_create_with_catalog, but I think it ended up noticeably cleaner: constraints and defaults are fed to heap.c in only one format now. I found one case that has not really worked as intended for a long time: ALTER TABLE ADD CHECK ... (that is, ADD CONSTRAINT without specifying a constraint name) failed to ensure that the same constraint name was used at child tables as at the parent, and thus the constraints ended up not being seen as related at all. Fixing this was a bit ugly since it meant that ADD CONSTRAINT has to recurse to child tables during Phase 2 after all, and has to be able to add work queue entries for Phase 3 at that time, which is not something needed by any other ALTER TABLE operation. I'm not sure if we ought to try to back-patch that --- it'd be a behavioral change with non-obvious implications. In the back branches, ADD CHECK followed by DROP CONSTRAINT will end up not deleting the child-table constraints, which is probably a bug but I wouldn't be surprised if applications were depending on the behavior. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [HACKERS] [PATCHES] [NOVICE] encoding problems
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Well, 8.3 is already different from 8.2, and a lot of people will see >> this particular aspect of it as a regression. I'm okay with >> backpatching to 8.3 ... though the patch needed rather more testing >> than you gave it. > OK, so Alvaro and Tom want this backpatched. However, it isn't going to > match 8.2 behavior --- is that OK? Huh? 8.3 is already hugely different from 8.2 because of the newline formatting changes. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Greg Smith <[EMAIL PROTECTED]> writes:
> Turns out it wasn't so contorted. Updated patch attached that only warns
> in the exact cases where the setting is ignored, and the warning says how
> it's actually setting the scale. I tested all the run types and it
> correctly complains only when warranted, samples:
Actually that didn't work, because scale defaults to 1, so it would
*always* warn ... I applied the attached instead.
regards, tom lane
Index: pgbench.c
===
RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v
retrieving revision 1.79
diff -c -r1.79 pgbench.c
*** pgbench.c 19 Mar 2008 03:33:21 - 1.79
--- pgbench.c 9 May 2008 15:49:47 -
***
*** 1449,1454
--- 1449,1455
int ttype = 0; /* transaction type. 0:
TPC-B, 1: SELECT only,
* 2: skip
update of branches and tellers */
char *filename = NULL;
+ boolscale_given = false;
CState *state; /* status of clients */
***
*** 1552,1557
--- 1553,1559
is_connect = 1;
break;
case 's':
+ scale_given = true;
scale = atoi(optarg);
if (scale <= 0)
{
***
*** 1647,1662
remains = nclients;
- if (getVariable(&state[0], "scale") == NULL)
- {
- snprintf(val, sizeof(val), "%d", scale);
- if (putVariable(&state[0], "scale", val) == false)
- {
- fprintf(stderr, "Couldn't allocate memory for
variable\n");
- exit(1);
- }
- }
-
if (nclients > 1)
{
state = (CState *) realloc(state, sizeof(CState) * nclients);
--- 1649,1654
***
*** 1668,1675
memset(state + 1, 0, sizeof(*state) * (nclients - 1));
! snprintf(val, sizeof(val), "%d", scale);
!
for (i = 1; i < nclients; i++)
{
int j;
--- 1660,1666
memset(state + 1, 0, sizeof(*state) * (nclients - 1));
! /* copy any -D switch values to all clients */
for (i = 1; i < nclients; i++)
{
int j;
***
*** 1682,1693
exit(1);
}
}
-
- if (putVariable(&state[i], "scale", val) == false)
- {
- fprintf(stderr, "Couldn't allocate memory for
variable\n");
- exit(1);
- }
}
}
--- 1673,1678
***
*** 1743,1764
}
PQclear(res);
! snprintf(val, sizeof(val), "%d", scale);
! if (putVariable(&state[0], "scale", val) == false)
! {
! fprintf(stderr, "Couldn't allocate memory for
variable\n");
! exit(1);
! }
! if (nclients > 1)
{
! for (i = 1; i < nclients; i++)
{
! if (putVariable(&state[i], "scale", val) ==
false)
! {
! fprintf(stderr, "Couldn't allocate
memory for variable\n");
! exit(1);
! }
}
}
}
--- 1728,1753
}
PQclear(res);
! /* warn if we override user-given -s switch */
! if (scale_given)
! fprintf(stderr,
! "Scale option ignored, using branches
table count = %d\n",
! scale);
! }
! /*
!* :scale variables normally get -s or database scale, but don't
override
!* an explicit -D switch
!*/
! if (getVariable(&state[0], "scale") == NULL)
! {
! snprintf(val, sizeof(val), "%d", scale);
! for (i = 0; i < nclients; i++)
{
! if (putVariable(&state[i], "scale", val) == false)
{
! fprintf(stderr, "Couldn't allocate memory for
variable\n");
! exit(1);
}
}
}
--
Sent via pgsql-patches maili
Re: [HACKERS] [PATCHES] [NOVICE] encoding problems
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Guillaume Smet wrote: > >> I understand your point of view but I really think it's more a > >> regression fix than a behavior change. > > > If I can get other hackers to say we should backpatch we can consider > > it. > > Well, 8.3 is already different from 8.2, and a lot of people will see > this particular aspect of it as a regression. I'm okay with > backpatching to 8.3 ... though the patch needed rather more testing > than you gave it. OK, so Alvaro and Tom want this backpatched. However, it isn't going to match 8.2 behavior --- is that OK? -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Tom Lane wrote: "Heikki Linnakangas" <[EMAIL PROTECTED]> writes: Ok, that'll work. Committed, thanks. I changed the sanity check that xlogfname > restore point into an Assert, though, because that's a sign that something's wrong. Shouldn't that Assert allow the equality case? Yes. Thanks. Hmm. I can't actually think of a scenario where that would happen, but it was definitely an oversight on my part. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
"Heikki Linnakangas" <[EMAIL PROTECTED]> writes: > Ok, that'll work. Committed, thanks. I changed the sanity check that > xlogfname > restore point into an Assert, though, because that's a sign > that something's wrong. Shouldn't that Assert allow the equality case? regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] New flex warnings
Peter Eisentraut <[EMAIL PROTECTED]> writes: > With GCC 4.3, I get warnings from every flex scanner that 'input' is defined > but not used. This can be solved by adding %option noinput. I tested this > option with a current flex and with the old 2.5.4a; both accept it. See > attached patch. Does anyone see problems with this? Hm, I wonder why we didn't see those before ... [ looks at code... ] Oh: yyinput() recurses internally, so even though it's not called from anywhere else, older gcc's don't realize it's really unreferenced. I confirm 2.5.4 has the noinput option. Patch seems ok from here, regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
On Fri, 2008-05-09 at 15:37 +0100, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > if (restartWALFileName)
> > {
> > + /*
> > +* Don't do cleanup if the restartWALFileName provided
> > +* is later than the xlog file requested. This is an error
> > +* and we must not remove these files from archive.
> > +* This shouldn't happen, but better safe than sorry.
> > +*/
> > + if (strcmp(restartWALFileName, nextWALFileName) > 0)
> > + return false;
> > +
> > strcpy(exclusiveCleanupFileName, restartWALFileName);
> > return true;
> > }
>
> I committed this sanity check into pg_standy, though it really shouldn't
> happen, but it just occurred to me that the most likely reason for that
> to happen is probably that the user has screwed up his restore_command
> line, mixing up the %p and %r arguments. Should we make that an error
> instead of just not doing the cleanup?
You can't explicitly throw a pgsql error in pg_standby, so the best we
can do is get the file requested if it exists. If the file is the wrong
one then recovery will throw the error. As long as we didn't delete
anything when that happens we can just correct the mistake and retry.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Greg Smith <[EMAIL PROTECTED]> writes: > Turns out it wasn't so contorted. Updated patch attached that only warns > in the exact cases where the setting is ignored, and the warning says how > it's actually setting the scale. It looks like the code could do with some refactoring. AFAICS scale is stored into all the :scale variables at lines 1671-1691 (although not if you only have one client !?) only to be done over again at lines 1746-1763 (though not if ttype != 3). Wasteful, confusing, and there's a case where it fails to be done at all. Sigh ... regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [HACKERS] [PATCHES] [NOVICE] encoding problems
Bruce Momjian <[EMAIL PROTECTED]> writes: > Guillaume Smet wrote: >> I understand your point of view but I really think it's more a >> regression fix than a behavior change. > If I can get other hackers to say we should backpatch we can consider > it. Well, 8.3 is already different from 8.2, and a lot of people will see this particular aspect of it as a regression. I'm okay with backpatching to 8.3 ... though the patch needed rather more testing than you gave it. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote:
if (restartWALFileName)
{
+ /*
+* Don't do cleanup if the restartWALFileName provided
+* is later than the xlog file requested. This is an error
+* and we must not remove these files from archive.
+* This shouldn't happen, but better safe than sorry.
+*/
+ if (strcmp(restartWALFileName, nextWALFileName) > 0)
+ return false;
+
strcpy(exclusiveCleanupFileName, restartWALFileName);
return true;
}
I committed this sanity check into pg_standy, though it really shouldn't
happen, but it just occurred to me that the most likely reason for that
to happen is probably that the user has screwed up his restore_command
line, mixing up the %p and %r arguments. Should we make that an error
instead of just not doing the cleanup?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Verified fix for Bug 4137
Simon Riggs wrote: I've extended the patch without introducing another new status variable, which was my original concern with what you suggested previously. Ok, that'll work. Committed, thanks. I changed the sanity check that xlogfname > restore point into an Assert, though, because that's a sign that something's wrong. I noted that there's no reason why the truncation point calculation had to be moved outside the case-statement, but it does look better that way so I did apply that change. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] New flex warnings
With GCC 4.3, I get warnings from every flex scanner that 'input' is defined but not used. This can be solved by adding %option noinput. I tested this option with a current flex and with the old 2.5.4a; both accept it. See attached patch. Does anyone see problems with this? diff --git a/src/backend/bootstrap/bootscanner.l b/src/backend/bootstrap/bootscanner.l index 89772f8..f82e0e9 100644 --- a/src/backend/bootstrap/bootscanner.l +++ b/src/backend/bootstrap/bootscanner.l @@ -52,6 +52,7 @@ static int yyline = 1; /* line number for error reporting */ %option 8bit %option never-interactive %option nodefault +%option noinput %option nounput %option noyywrap %option prefix="boot_yy" diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l index 99f8546..012b120 100644 --- a/src/backend/parser/scan.l +++ b/src/backend/parser/scan.l @@ -101,6 +101,7 @@ static unsigned char unescape_single_char(unsigned char c); %option 8bit %option never-interactive %option nodefault +%option noinput %option nounput %option noyywrap %option prefix="base_yy" diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 3a4b63b..706da59 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -59,6 +59,7 @@ static char *GUC_scanstr(const char *s); %option 8bit %option never-interactive %option nodefault +%option noinput %option nounput %option noyywrap %option prefix="GUC_yy" diff --git a/src/bin/psql/psqlscan.l b/src/bin/psql/psqlscan.l index 02c7f40..965c41b 100644 --- a/src/bin/psql/psqlscan.l +++ b/src/bin/psql/psqlscan.l @@ -125,6 +125,7 @@ static void emit(const char *txt, int len); %option 8bit %option never-interactive %option nodefault +%option noinput %option nounput %option noyywrap diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l index bd0a7d2..9b6feb7 100644 --- a/src/interfaces/ecpg/preproc/pgc.l +++ b/src/interfaces/ecpg/preproc/pgc.l @@ -76,6 +76,7 @@ static struct _if_value %option 8bit %option never-interactive %option nodefault +%option noinput %option noyywrap %option yylineno diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l index 0ec8d53..fb9ef4b 100644 --- a/src/pl/plpgsql/src/scan.l +++ b/src/pl/plpgsql/src/scan.l @@ -47,6 +47,7 @@ bool plpgsql_SpaceScanned = false; %option 8bit %option never-interactive %option nodefault +%option noinput %option nounput %option noyywrap %option prefix="plpgsql_base_yy" -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [NOVICE] encoding problems
Bruce Momjian escribió: > Guillaume Smet wrote: > > On Thu, May 8, 2008 at 9:11 PM, Bruce Momjian <[EMAIL PROTECTED]> wrote: > > As I mentioned it before, is there any chance for this fix to be > > backported to 8.3 branch? IMHO it's a usability regression. > > No, we don't change behaviors in back branches unless we get lots of > complaints, and we haven't in this case. complaints++ -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
On Wed, 7 May 2008, Bruce Momjian wrote: Tom Lane wrote: Greg Smith <[EMAIL PROTECTED]> writes: The way the option parsing code is done would make complaining in the case where your parameter is ignored a bit of a contortion. Yeah. But couldn't we have that part issue a warning if -s had been set on the command line? Patch attached that issues a warning. Turns out it wasn't so contorted. Updated patch attached that only warns in the exact cases where the setting is ignored, and the warning says how it's actually setting the scale. I tested all the run types and it correctly complains only when warranted, samples: $ ./pgbench -s 200 -i pgbench creating tables... 1 tuples done. ... $ ./pgbench -s 100 pgbench Scale setting ignored by standard tests, using database branch count starting vacuum...end. transaction type: TPC-B (sort of) scaling factor: 200 ... $ ./pgbench -s 100 -f select.sql pgbench starting vacuum...end. transaction type: Custom query scaling factor: 100 ... -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MDIndex: contrib/pgbench/pgbench.c === RCS file: /home/gsmith/cvsrepo/pgsql/contrib/pgbench/pgbench.c,v retrieving revision 1.79 diff -u -r1.79 pgbench.c --- contrib/pgbench/pgbench.c 19 Mar 2008 03:33:21 - 1.79 +++ contrib/pgbench/pgbench.c 9 May 2008 07:12:21 - @@ -1645,6 +1645,9 @@ exit(0); } + if (scale && (ttype != 3)) + fprintf(stderr,"Scale setting ignored by standard tests, using database branch count\n"); + remains = nclients; if (getVariable(&state[0], "scale") == NULL) -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
