Re: [PATCHES] [HACKERS] TRUNCATE TABLE with IDENTITY
On Fri, 2008-05-16 at 21:50 -0400, Tom Lane wrote: Neil Conway [EMAIL PROTECTED] writes: Ugh. The fact that the RESTART IDENTITY part of TRUNCATE is non-transactional is a pretty unsightly wort. Actually, I agree. Shall we just revert that feature? The ALTER SEQUENCE part of this patch is clean and useful, but I'm less than enamored of the TRUNCATE part. Perhaps, but we should also take into account that TRUNCATE is not and never will be MVCC compliant, so its not something you'd expect to run except as a maintenance action. If we do keep it, I would suggest that we add a WARNING so that the effects are clearly recorded. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] libpq object hooks (libpq events)
Here is an updated patch for what was called object hooks. This is now
called libpq events. If someone has a better name or hates ours, let us
know.
I am continuing to use the object hooks thread to avoid confusing anyone.
Terminology:
I got rid of calling it object events because it is possible to add
other non-object-related events in the future; maybe a query event.
This system can be used for any type of event, I think it is pretty generic.
event proc - the callback procedure/function implemented outside of
libpq ... PGEventProc. The address of the event proc is used as a
lookup key for getting a conn/result's event data.
event data - the state data managed by the event proc but tracked by
libpq. This allows the event proc implementor to basically add a
dynamic struct member to a conn or result. This is an instance based
value, unlike the arg pointer.
arg pointer - this is the passthrough/user pointer. I called it 'arg'
as other libpq callbacks used this term for this type of value. This
value can be retrieved via PQeventData, PQresultEventData ... its an
optional double pointer argument.
event state - an internal structure, PGEventState, which contains the
event data, event proc and the 'arg' pointer. Internally, PGconn and
PGresult contain arrays of event states.
event id - PGEventId enum values, PGEVT_xxx. This tells the event proc
which event has occurred.
event info - These are the structures for event ids, like
PGEVT_RESULTDESTROY has a corresponding PGEventResultDestroy structure.
The PGEventProc function's 2nd argument is void *info. The first
argument is an event id which tells the proc implementor how to cast the
void *. This replaced the initial va_arg idea.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 17 May 2008 12:06:10 -
***
*** 138,143
--- 138,149
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQregisterEventProc 145
+ PQeventData 146
+ PQresultEventData 147
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.358
diff -C6 -r1.358 fe-connect.c
*** fe-connect.c16 May 2008 18:30:53 - 1.358
--- fe-connect.c17 May 2008 12:06:11 -
***
*** 998,1009
--- 998,1014
* We really shouldn't have been polled in these two
cases, but we
* can handle it.
*/
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
return PGRES_POLLING_OK;
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827
--- 1821,1837
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+ }
return PGRES_POLLING_OK;
}
case CONNECTION_SETENV:
/*
***
*** 1848,1859
--- 1858,1874
default:
goto error_return;
}
/* We are open for business! */
conn-status = CONNECTION_OK;
+ if(!pqRegisterGlobalEvents(conn))
+ {
+ conn-status = CONNECTION_BAD;
+ return PGRES_POLLING_FAILED;
+
Re: [PATCHES] [HACKERS] TRUNCATE TABLE with IDENTITY
Simon Riggs [EMAIL PROTECTED] writes:
On Fri, 2008-05-16 at 21:50 -0400, Tom Lane wrote:
Actually, I agree. Shall we just revert that feature?
Perhaps, but we should also take into account that TRUNCATE is not and
never will be MVCC compliant, so its not something you'd expect to run
except as a maintenance action.
Good point. I had a couple of further thoughts this morning:
1. The case Neil is worried about is something like
BEGIN;
TRUNCATE TABLE foo RESTART IDENTITY;
COPY foo FROM ...;
COMMIT;
If the COPY fails partway through, the old table contents are restored,
but the sequences are not. However removing RESTART IDENTITY will not
remove the hazard, because there is no difference between this and
BEGIN;
TRUNCATE TABLE foo;
SELECT setval('foo_id', 1);
COPY foo FROM ...;
COMMIT;
other than the latter adding a little extra chance for pilot error in
resetting the wrong sequence. So if we revert the patch we haven't
accomplished much except to take away an opportunity to point out the
risk. I vote for leaving the patch in and rewriting the warning
to point out this risk.
2. I had first dismissed Neil's idea of transactional sequence updates
as impossible, but on second look it could be done. Suppose RESTART
IDENTITY does this for each sequence;
* obtain AccessExclusiveLock;
* assign a new relfilenode;
* insert a sequence row with all parameters copied except
last_value copies start_value;
* hold AccessExclusiveLock till commit.
IOW just like truncate-and-reload, but for a sequence. Within the
current backend, subsequent operations see the new sequence values.
If the transaction rolls back, the old sequence relfilenode is still
there and untouched.
It's slightly annoying to need to lock out other backends' nextval
operations, but for the use-case of TRUNCATE this doesn't seem
like it's really much of a problem.
I'm not sure if it'd be worth exposing this behavior as a separate
user-visible command (CREATE OR REPLACE SEQUENCE, maybe?), but it seems
worth doing to make TRUNCATE-and-reload less of a foot gun.
So what I think we should do is leave the patch there, revise the
warning per Neil's complaint, and add a TODO item to reimplement RESTART
IDENTITY transactionally.
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] [HACKERS] TRUNCATE TABLE with IDENTITY
On Sat, 2008-05-17 at 12:04 -0400, Tom Lane wrote: So what I think we should do is leave the patch there, revise the warning per Neil's complaint, and add a TODO item to reimplement RESTART IDENTITY transactionally. Sounds good. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] pg_dump lock timeout - resend
I originally sent this a week ago, but there was no response and I do not
see it at:
http://momjian.postgresql.org/cgi-bin/pgpatches
or
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
so I assume it got missed in all the excitement about the psql banner.
-
Subject: [PATCHES] pg_dump lock timeout
Date: Sun, 11 May 2008 04:30:47 -0700
Attached is a patch to add a commandline option to pg_dump to limit how long
pg_dump will wait for locks during startup.
The intent of this patch is to allow pg_dump to fail if a table lock cannot
be taken in a reasonable time. This allows the caller of pg_dump to retry or
otherwise correct the situation, without having locks held for long periods,
and without pg_dump having a long window during which catalog changes can
occur.
It works by setting statement_timeout to the user specified delay during
the startup phase where it is taking access share locks on all the tables.
Once all the locks are taken, it sets statement_timeout back to the default.
If a lock table statement times out, the dump fails with the statement timed
out error.
The orginal motivation was a client who runs heavy batch workloads and uses
truncate table and other DML in long transactions. This has created some
unhappy interaction scenarios with pg_dump:
- pg_dump ends up waiting hours on a DML table lock that is part of a long
transaction. Once the lock is released, pg_dump runs only to find
some table later in the list has been dropped. So pg_dump fails.
- pg_dump waits on a lock while holding access share locks on most of the
tables. Other processes that want to do DML wait on pg_dump. After a
while, large parts of the application are blocked while pg_dump waits
on locks. Eventually the operations staff notice that pg_dump is
blocking production and kill the dump.
Please have a look and consider it for merging.
-
I'll even include the patch in the original mail this time, instead of a
hurried followup.
Thanks again,
-dg
--
David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.
*** pgsql/src/bin/pg_dump/pg_dump.c.orig2008-05-11 03:23:06.0
-0700
--- pgsql/src/bin/pg_dump/pg_dump.c 2008-05-11 03:44:58.0 -0700
***
*** 71,76
--- 71,77
bool schemaOnly;
bool dataOnly;
bool aclsSkip;
+ const char*lockWaitTimeout;
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
***
*** 238,243
--- 239,245
{column-inserts, no_argument, NULL, 'D'},
{host, required_argument, NULL, 'h'},
{ignore-version, no_argument, NULL, 'i'},
+ {lock-wait-timeout, required_argument, NULL, 'l'},
{no-reconnect, no_argument, NULL, 'R'},
{oids, no_argument, NULL, 'o'},
{no-owner, no_argument, NULL, 'O'},
***
*** 278,283
--- 280,286
strcpy(g_opaque_type, opaque);
dataOnly = schemaOnly = dumpInserts = attrNames = false;
+ lockWaitTimeout = NULL;
progname = get_progname(argv[0]);
***
*** 299,305
}
}
! while ((c = getopt_long(argc, argv,
abcCdDE:f:F:h:in:N:oOp:RsS:t:T:U:vWxX:Z:,
long_options,
optindex)) != -1)
{
switch (c)
--- 302,308
}
}
! while ((c = getopt_long(argc, argv,
abcCdDE:f:F:h:il:n:N:oOp:RsS:t:T:U:vWxX:Z:,
long_options,
optindex)) != -1)
{
switch (c)
***
*** 350,355
--- 353,362
/* ignored, deprecated option */
break;
+ case 'l': /* lock wait time */
+ lockWaitTimeout = optarg;
+ break;
+
case 'n': /* include schema(s) */
simple_string_list_append(schema_include_patterns, optarg);
include_everything = false;
***
*** 755,760
--- 762,769
printf(_(\nGeneral options:\n));
printf(_( -f, --file=FILENAME output file name\n));
printf(_( -F, --format=c|t|p output file format (custom, tar,
plain text)\n));
+ printf(_( -l, --lock-wait-timeout=DELAY\n
+timeout and fail after
delay waiting for a table share lock\n));
printf(_( -v, --verboseverbose mode\n));
printf(_( -Z, --compress=0-9 compression level for compressed
formats\n));
printf(_( --help show
Re: [PATCHES] [COMMITTERS] pgsql: Don't call rm with empty file list.
Peter Eisentraut wrote: Log Message: --- Don't call rm with empty file list. Modified Files: -- pgsql/src: nls-global.mk (r1.12 - r1.13) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/nls-global.mk?r1=1.12r2=1.13) FYI, I had to also apply the attached patch to fix it, but your example on the line above was very helpful. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/nls-global.mk === RCS file: /cvsroot/pgsql/src/nls-global.mk,v retrieving revision 1.13 diff -c -c -r1.13 nls-global.mk *** src/nls-global.mk 17 May 2008 20:24:05 - 1.13 --- src/nls-global.mk 17 May 2008 20:59:27 - *** *** 78,84 clean-po: $(if $(MO_FILES),rm -f $(MO_FILES)) ! @rm -f $(addsuffix .old, $(PO_FILES)) rm -f po/$(CATALOG_NAME).pot --- 78,84 clean-po: $(if $(MO_FILES),rm -f $(MO_FILES)) ! @$(if $(PO_FILES),rm -f $(addsuffix .old, $(PO_FILES))) rm -f po/$(CATALOG_NAME).pot -- 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] pg_dump lock timeout - resend
daveg wrote: I originally sent this a week ago, but there was no response and I do not see it Nope. FYI, the right link is [1] and your patch [2] is in the queue for July Commit Fest. [1] http://wiki.postgresql.org/wiki/Development_information [2] http://wiki.postgresql.org/wiki/CommitFest:July -- Euler Taveira de Oliveira http://www.timbira.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] psql command aliases support
Bernd Helmle [EMAIL PROTECTED] writes:
please find attached a patch which implements psql command aliases. They
work the same way as on bash, zsh and others, for example:
I'm still not convinced that we want this sort of feature at all.
I quote from the current bash manual page:
ALIASES
...
The rules concerning the definition and use of aliases are somewhat
confusing.
...
For almost every purpose, aliases are superseded by shell functions.
The bash authors seem to wish they'd never adopted the feature, which
makes me wonder whether we really want to copy it slavishly.
Having said that, though, here are some code-review points:
* The patch intends that the expansion of an alias could be either a
backslash command or SQL command text, but I don't think you've thought
through the interactions of these cases. In particular, the patch doesn't
seem to behave very sanely if the backslash command comes when there is
already text on the current line or previous lines of the query buffer:
it just wipes that text out, which is surely not the desirable thing.
* Use of a VariableSpace for the stack of aliases-being-expanded seems
exceedingly klugy: variable spaces are intended for persistent storage
not transient state, so this fails to satisfy the principle of least
astonishment for readers of the code. Also, you're failing to manipulate
it as a stack: you shouldn't wipe a stack entry upon matching it, only
when control returns from having expanded it. Another problem is that
it doesn't get reset at all if the alias expansion is just SQL text and
not another backslash command, meaning successive uses of such an alias
won't work. I think you'd be better off if the expansion were done as a
separate recursive routine with the set of active aliases just passed as
a list threaded through local variables of the routine invocations.
* I think you should lose the separate PSQL_CMD_ALIAS status code and
just use PSQL_CMD_NEWEDIT. The one place where the codes act different
looks like a mistake to me anyway.
* Don't feed non-constant strings through gettext():
+ if (cmd)
+ {
+ puts(_(cmd));
+ success = true;
+ }
The best you can hope for is that it doesn't do anything.
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] [HACKERS] use of pager on Windows psql
Andrew Dunstan wrote:
Not sure why ware are not. Should we enabled that code on Win32 and see
how it works? Can you test it? Was it some MinGW limitation? I do see
isatty() being used on lots of platforms.
This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there. I don't see the Win32 checks around
isatty() anywhere else.
In fact, it looks to me like it would be much more sensible to #include
settings.h and then simply test pset.notty for all platforms.
Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?
Good point. I think the best thing would just be to remove the #ifndef
WIN32 / #endif lines
OK, patch applied to remove the Win32 test in both places.
I was wrong about \g filename changing stdout, I think. It keeps stdout
but uses a different output stream. I am just unsure, given all the
features of psql, wether it could change stdin/stdout while running in
some way.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/print.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.105
diff -c -c -r1.105 print.c
*** src/bin/psql/print.c 17 May 2008 21:40:44 - 1.105
--- src/bin/psql/print.c 17 May 2008 23:32:36 -
***
*** 1912,1924
PageOutput(int lines, unsigned short int pager)
{
/* check whether we need / can / are supposed to use pager */
! if (pager
! #ifndef WIN32
!
! isatty(fileno(stdin))
! isatty(fileno(stdout))
! #endif
! )
{
const char *pagerprog;
FILE *pagerpipe;
--- 1912,1918
PageOutput(int lines, unsigned short int pager)
{
/* check whether we need / can / are supposed to use pager */
! if (pager isatty(fileno(stdin)) isatty(fileno(stdout)))
{
const char *pagerprog;
FILE *pagerpipe;
Index: src/interfaces/libpq/fe-print.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-print.c,v
retrieving revision 1.75
diff -c -c -r1.75 fe-print.c
*** src/interfaces/libpq/fe-print.c 1 Jan 2008 19:46:00 - 1.75
--- src/interfaces/libpq/fe-print.c 17 May 2008 23:32:36 -
***
*** 147,159
if (fout == NULL)
fout = stdout;
! if (po-pager fout == stdout
! #ifndef WIN32
!
! isatty(fileno(stdin))
! isatty(fileno(stdout))
! #endif
! )
{
/*
* If we think there'll be more than one screen of output, try to
--- 147,154
if (fout == NULL)
fout = stdout;
! if (po-pager fout == stdout isatty(fileno(stdin))
! isatty(fileno(stdout)))
{
/*
* If we think there'll be more than one screen of output, try to
--
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] pg_dump lock timeout - resend
On Sat, May 17, 2008 at 06:55:27PM -0300, Euler Taveira de Oliveira wrote: daveg wrote: I originally sent this a week ago, but there was no response and I do not see it Nope. FYI, the right link is [1] and your patch [2] is in the queue for July Commit Fest. [1] http://wiki.postgresql.org/wiki/Development_information [2] http://wiki.postgresql.org/wiki/CommitFest:July Thanks for the pointers. I tried finding this from the main postgresql.org developer section, so perhaps I am obtuse, or perhaps the commitfest info is not that easy to find. -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- 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] pg_dump lock timeout - resend
daveg wrote: On Sat, May 17, 2008 at 06:55:27PM -0300, Euler Taveira de Oliveira wrote: daveg wrote: I originally sent this a week ago, but there was no response and I do not see it Nope. FYI, the right link is [1] and your patch [2] is in the queue for July Commit Fest. [1] http://wiki.postgresql.org/wiki/Development_information [2] http://wiki.postgresql.org/wiki/CommitFest:July Thanks for the pointers. I tried finding this from the main postgresql.org developer section, so perhaps I am obtuse, or perhaps the commitfest info is not that easy to find. The pages could certainly stand an updating to reflect how development currently commences. I will work up a patch next week. Joshua D. Drake -dg -- 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] pg_dump lock timeout - resend
Joshua D. Drake wrote: The pages could certainly stand an updating to reflect how development currently commences. I will work up a patch next week. IMHO, this development information needs to be at [1]. [1] http://www.postgresql.org/developer/roadmap -- Euler Taveira de Oliveira http://www.timbira.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] lc_time and localized dates
I have reviewed this patch. I like the method you used, but I did find a few things I had to change. First, I found the abbreviated variable names confusing so I used longer ones, like: extern char *localized_abbrev_days[7]; extern char *localized_full_days[7]; extern char *localized_abbrev_months[12]; extern char *localized_full_months[12]; Second, I found that the code doing upper/lower case didn't work. It was copying the buffer into a 'result' variable, but then incrementing 'result' so by the end 'result' pointed to only the null byte, and that is the pointer that was returned. Third, there were a few places where the code assumed str_toupper() modified the passed buffer, rather than returning a new one. And finally, while you used strdup() to save values in the cache (good), you never free()'ed the old values when you were reloading the cache. I have fixed all these items and the updated patch is at: ftp://momjian.us/pub/postgresql/mypatches/lc_time The original patch was here: http://archives.postgresql.org/message-id/[EMAIL PROTECTED] --- Euler Taveira de Oliveira wrote: Bruce Momjian wrote: Euler, have you updated this patch yet? Here is an updated patch. It follows the Oracle behaviour and uses a cache mechanism to avoid calling setlocale() all the time. I unified the localized_* and str_* functions. I didn't test it on Windows. I would appreciate some feedback. euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu Qui Quinta apr ABR abril 2008 (1 registro) euler=# set lc_time to 'it_IT.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char - thu Gio Gioved? apr APR aprile 2008 (1 registro) euler=# set lc_time to 'es_ES.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu Jue Jueves apr ABR abril 2008 (1 registro) euler=# set lc_time to 'fr_FR.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char -- thu Jeu Jeudi apr AVR avril 2008 (1 registro) euler=# set lc_time to 'cs_CZ.UTF-8'; SET euler=# select to_char(now(), 'dy tmDy tmDay mon tmMON tmmonth '); to_char --- thu ?t ?tvrtek apr DUB duben 2008 (1 registro) -- Euler Taveira de Oliveira http://www.timbira.com/ [ application/x-gzip is not supported, skipping... ] -- 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] lc_time and localized dates
Bruce Momjian wrote: I have reviewed this patch. I like the method you used, but I did find a few things I had to change. Good catch. I tested here and it seems ok. Thanks for your review. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
