Re: [HACKERS] psql wrapped format default for backslash-d commands
On Tue, May 13, 2008 at 4:12 PM, Bryce Nesbitt <[EMAIL PROTECTED]> wrote: > > Tom Lane wrote: >> >> After experimenting for a bit, I'd say "never". This output format is >> extremely ugly. Maybe if it had enough smarts not to break in the >> middle of words ... >> regards, tom lane > > Yet, wrapped is the same as aligned, if the lines fit! > > Ugly to wrap? Yes. > Uglier than the sea of pipes you get when aligned output wraps itself? > > Well I suppose you have to pick your preferred version of ugly. I find it > very hard to line up columns when presented with a sea of pipes, and highly > annoying to constantly adjust my window width to see a query, especially if > only one or two rows are too large. > I really like the idea of wrapping, but after playing with the format a bit myself, I have to agree with Tom that breaking in the middle of words produces some very nasty output. If the format could be improved to only wrap on word boudaries, that would increase its appeal dramatically. Anybody got a rough idea how difficult it would be to add word-awareness int o the wrapping code? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem returning strings with pgsql 8.3.x
On Mon, May 12, 2008 at 11:23:17PM -0600, Josh Tolley wrote: > SPI_push(); > retval = > InputFunctionCall(&flinfo, lolVarGetString(returnVal, true), > resultTypeIOParam, -1); > SPI_pop(); Won't this cause the return value to be allocated inside a new memory block which gets freeds at the SPI_pop? Have a nice day, -- Martijn van Oosterhout <[EMAIL PROTECTED]> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] psql wrapped format default for backslash-d commands
Tom Lane wrote: Bruce Momjian <[EMAIL PROTECTED]> writes: Now that psql '\pset format wrapped' is in CVS, we should consider when we want to use 'wrapped' format by default. After experimenting for a bit, I'd say "never". This output format is extremely ugly. Maybe if it had enough smarts not to break in the middle of words ... regards, tom lane Yet, wrapped is the same as aligned, if the lines fit! Ugly to wrap? Yes. Uglier than the sea of pipes you get when aligned output wraps itself? Well I suppose you have to pick your preferred version of ugly. I find it very hard to line up columns when presented with a sea of pipes, and highly annoying to constantly adjust my window width to see a query, especially if only one or two rows are too large. -Bryce PS: it is true, I don't like the missing | bars on the current version. I prefer that the grid cells be fully drawn, even if the contents don't fill the box
Re: [HACKERS] Fairly serious bug induced by latest guc enum changes
Tom Lane wrote:
Magnus Hagander <[EMAIL PROTECTED]> writes:
I still think going with the older method would be the safest, though,
for the other reasons. You agree?
Seems reasonable to me.
Since it didn't really sound like a nice option, here's a third one I
came up with later. Basically, this one splits things apart so we only
use one variable, which is sync_method. Instead of using a macro to get
the open sync bit, it uses a function. This makes the assign hook only
responsible for flushing and closing the old file.
Thoughts? And if you like it, is it enough to expect the compiler to
figure out to inline it or should we explicitly inline it?
In these, the value was previously derived from a string and set in the
hook. It's now set directly by the GUC code, and the hook only updates
"other things" (setting the actual syslog facility, and resetting the
cache, respectively).
I think that means there are no bugs there.
Yeah, that's fine. I think though that I may have created a bug inside
GUC itself: the new stacking code could conceivably fail (palloc error)
between success return from the assign hook and setting up the stack
entry that is needed to undo the assignment on abort. In this situation
the assign hook would've made its "other thing" changes but there is no
GUC state to cause the hook to be called again to undo 'em. I need to
fix it so that any palloc'ing needed is done before calling the assign
hook.
Ok. I'll leave that to you for now :) I still need to figure out how the
stacking works because I want to add the "this setting came from file
X line Y" field to pg_settings, but that's a separate issue.
//Magnus
Index: xlog.c
===
RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.307
diff -c -r1.307 xlog.c
*** xlog.c 12 May 2008 19:45:23 - 1.307
--- xlog.c 13 May 2008 06:03:45 -
***
*** 86,97
#define XLOGfileslop (2*CheckPointSegments + 1)
- /* these are derived from XLOG_sync_method by assign_xlog_sync_method */
- int sync_method = DEFAULT_SYNC_METHOD;
- static int open_sync_bit = DEFAULT_SYNC_FLAGBIT;
-
- #define XLOG_SYNC_BIT (enableFsync ? open_sync_bit : 0)
-
/*
* GUC support
*/
--- 86,91
***
*** 444,449
--- 438,444
static bool read_backup_label(XLogRecPtr *checkPointLoc,
XLogRecPtr *minRecoveryLoc);
static void rm_redo_error_callback(void *arg);
+ static int get_sync_bit(void);
/*
***
*** 1960,1966
*/
if (*use_existent)
{
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | XLOG_SYNC_BIT,
S_IRUSR | S_IWUSR);
if (fd < 0)
{
--- 1955,1961
*/
if (*use_existent)
{
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(),
S_IRUSR | S_IWUSR);
if (fd < 0)
{
***
*** 1986,1992
unlink(tmppath);
! /* do not use XLOG_SYNC_BIT here --- want to fsync only at end of fill */
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
S_IRUSR | S_IWUSR);
if (fd < 0)
--- 1981,1987
unlink(tmppath);
! /* do not use get_sync_bit() here --- want to fsync only at end of fill */
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
S_IRUSR | S_IWUSR);
if (fd < 0)
***
*** 2064,2070
*use_existent = false;
/* Now open original target segment (might not be file I just made) */
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | XLOG_SYNC_BIT,
S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(ERROR,
--- 2059,2065
*use_existent = false;
/* Now open original target segment (might not be file I just made) */
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(),
S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(ERROR,
***
*** 2115,2121
unlink(tmppath);
! /* do not use XLOG_SYNC_BIT here --- want to fsync only at end of fill */
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
S_IRUSR | S_IWUSR);
if (fd < 0)
--- 2110,2116
unlink(tmppath);
! /* do not use get_sync_bit() here --- want to fsync only at end of fill */
fd = BasicOpenFile(tmppath, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
S_IRUSR | S_IWUSR);
if (fd < 0)
***
*** 2297,2303
XLogFilePath(path, ThisTimeLineID, log, seg);
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | XLOG_SYNC_BIT,
S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(PANIC,
--- 2292,2298
XLogFilePath(path, ThisTimeLineID, log, seg);
! fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(),
S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(PANIC,
***
*** 3650,3656
unlink(tmppath);
! /* do not use XLOG_SYNC_BIT here --- want to fsync only at end of fill */
fd = BasicOpenFile(tmpp
Re: [HACKERS] odd output in restore mode
On Mon, 2008-05-12 at 22:40 -0400, Andrew Dunstan wrote: > > Robert Treat wrote: > > On Monday 12 May 2008 18:58:37 Andrew Dunstan wrote: > > > >> Simon Riggs wrote: > >> > Lastly, not quite related to this output, but in the same general area, > should we have an option on pg_standby to allow removing the archive > file after it has been restored? > > >>> There already is one, but its more complex than that. (%r) > >>> > >> I was using %r. But the WAL files that have been restored (according to > >> the log) are still in the archive dir. So it looks like %r isn't working > >> properly. > >> > > Are you sure you've moved passed the latest restart point? Just because a > > WAL > > file has been processed doesn't mean it can be deleted. > > > Thanks. It wasn't that, but when I ran with the very latest patches this > problem went away. > Thanks for testing. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Problem returning strings with pgsql 8.3.x
Having posted this to -general [1] per -hackers list instructions [2]
to try elsewhere first, and waited (not very long, I admit) in vain
for a response, I'm posting this to -hackers now. My apologies if my
impatience in that regard annoys.
While developing PL/LOLCODE, I've found something wrong with returning
strings from LOLCODE functions using 8.3.0 or greater. Using 8.4beta
from a few days ago, for instance, a function that should return "test
string" returns
"\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F\x7F" in
pgsql (sometimes the number of \x7F characters varies). In 8.2.4 it
works fine.
Here's the code involved, from pl_lolcode_call_handler, the call
handler function for PL/LOLCODE. First, the bit that finds the
FmgrInfo structure and typioparam for the result type:
procTup = SearchSysCache(PROCOID,
ObjectIdGetDatum(fcinfo->flinfo->fn_oid), 0, 0, 0);
if (!HeapTupleIsValid(procTup)) elog(ERROR, "Cache lookup
failed for procedure %u", fcinfo->flinfo->fn_oid);
procStruct = (Form_pg_proc) GETSTRUCT(procTup);
typeTup = SearchSysCache(TYPEOID,
ObjectIdGetDatum(procStruct->prorettype), 0, 0, 0);
if (!HeapTupleIsValid(typeTup)) elog(ERROR, "Cache lookup
failed for type %u", procStruct->prorettype);
typeStruct = (Form_pg_type) GETSTRUCT(typeTup);
resultTypeIOParam = getTypeIOParam(typeTup);
fmgr_info_cxt(typeStruct->typinput, &flinfo,
TopMemoryContext); /*CurTransactionContext); */
ReleaseSysCache(typeTup);
Here's the code that converts the return value into a Datum later on
in the function:
if (returnTypeOID != VOIDOID) {
if (returnVal != NULL) {
if (returnVal->type == ident_NOOB)
fcinfo->isnull = true;
else {
SPI_push();
if (returnTypeOID == BOOLOID)
retval =
InputFunctionCall(&flinfo, lolVarGetTroof(returnVal) == lolWIN ?
"TRUE" : "FALSE", resultTypeIOParam, -1);
else {
/* elog(NOTICE,
lolVarGetString(returnVal, true)); */
retval =
InputFunctionCall(&flinfo, lolVarGetString(returnVal, true),
resultTypeIOParam, -1);
}
SPI_pop();
}
}
else {
fcinfo->isnull = true;
}
}
SPI_finish();
/* elog(NOTICE, "PL/LOLCODE ending"); */
return retval;
returnVal is an instance of the struct PL/LOLCODE uses to store its
variables. The key line in this case is the one after the
commented-out call to elog. retval is a Datum type. lolVarGetString()
returns the string value the returnVal struct represents -- I'm
certain of that thanks to gdb and other testing. All other data types
PL/LOLCODE knows about internally seem to return just fine. I'm fairly
certain I'm screwing up memory somewhere, but I can't see what I've
done wrong.
I'm glad to provide further details, but those included above are all
the ones I thought were relevant. Thanks in advance for any help you
can provide.
- Josh / eggyknap
[1] http://archives.postgresql.org/pgsql-general/2008-05/msg00311.php
[2] http://archives.postgresql.org/pgsql-hackers/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
On Mon, 2008-05-12 at 23:03 -0400, Andrew Dunstan wrote: > Tom Lane wrote: > > Andrew Dunstan <[EMAIL PROTECTED]> writes: > >> Simon Riggs wrote: > >> > >>> Well, the patch was rejected long ago, not sure why its in this > >>> commitfest. But its an open issue on the Windows port. > >>> > > > >> Surely the right fix is to use the recently implemented > >> pgwin32_safestat() (if we aren't already - I suspect we probably are) > >> and remove the kluge in pg_standby.c. > >> > > > > I think the open issue is how to know whether pgwin32_safestat fixes the > > problem that the kluge tried to work around. > > > Well, I think we need to consider quite a number of scenarios. The > archive directory could be local, on a remote Windows machine, or on a > remote Samba server. The archive file could be copied by Windows copy, > or Unix cp, or scp, or rsync, among others. > > I'd like to know the setup that was found to produce the error, to start > with. It's a race condition, not a deterministic bug with recreatable conditions. My understanding is the current code was introduced to work around the implementation of stat on Windows which says the filesize is correct even while it is still copying it. The 1sec delay fixed that but is clearly not a foolproof fix and introduces a delay also, which was the original complaint. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> I like this syntax, but I am not if it's good idea add new similar >> statement. I don't know - but maybe it's can be better then extending >> RAISE - and way to get consensus. > > I looked a bit more at the SQL spec. It already defines a information item name> MESSAGE_TEXT, which arguably is what we should > use for the primary message item, but that seems unpleasantly long for > something that's going to be used in pretty much every SIGNAL command. > Also there's a question of whether it's supposed to mean the *complete* > message delivered to a client, which would subsume DETAIL, HINT, etc > in our scheme. So I'm a bit tempted to stick with MESSAGE, DETAIL, > and HINT as the settable options if we go with SQL/PSM-derived syntax. > We'd also want LEVEL or something to be able to specify non-ERROR > elog levels. > I agree > Also, as to the re-throw-an-error capability, SQL/PSM defines a RESIGNAL > command that does this. I propose implementing only the parameterless > variant of this, at least for the time being. (The spec appears to > intend that RESIGNAL can override selected fields of the error being > re-thrown, which doesn't strike me as a terribly good idea --- you could > end up with a completely nonsensical error report.) > ok >regards, tom lane > who write this patch? Pavel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: Simon Riggs wrote: Well, the patch was rejected long ago, not sure why its in this commitfest. But its an open issue on the Windows port. Surely the right fix is to use the recently implemented pgwin32_safestat() (if we aren't already - I suspect we probably are) and remove the kluge in pg_standby.c. I think the open issue is how to know whether pgwin32_safestat fixes the problem that the kluge tried to work around. Well, I think we need to consider quite a number of scenarios. The archive directory could be local, on a remote Windows machine, or on a remote Samba server. The archive file could be copied by Windows copy, or Unix cp, or scp, or rsync, among others. I'd like to know the setup that was found to produce the error, to start with. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
Robert Treat <[EMAIL PROTECTED]> writes: > On Monday 12 May 2008 14:40:46 Pavel Stehule wrote: >> In plpgsql I prefer PL/SQL syntax. > I think nod's toward PL/SQL compatability should be given in general. This position seems just about entirely unhelpful for resolving the problem at hand, because PL/SQL hasn't *got* syntax that does what we want. It might lead us to favor RAISE without parameter over RESIGNAL, but that's a pretty trivial point anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
Robert Treat wrote: On Monday 12 May 2008 18:58:37 Andrew Dunstan wrote: Simon Riggs wrote: Lastly, not quite related to this output, but in the same general area, should we have an option on pg_standby to allow removing the archive file after it has been restored? There already is one, but its more complex than that. (%r) I was using %r. But the WAL files that have been restored (according to the log) are still in the archive dir. So it looks like %r isn't working properly. Are you sure you've moved passed the latest restart point? Just because a WAL file has been processed doesn't mean it can be deleted. Thanks. It wasn't that, but when I ran with the very latest patches this problem went away. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Simon Riggs wrote: >> Well, the patch was rejected long ago, not sure why its in this >> commitfest. But its an open issue on the Windows port. > Surely the right fix is to use the recently implemented > pgwin32_safestat() (if we aren't already - I suspect we probably are) > and remove the kluge in pg_standby.c. I think the open issue is how to know whether pgwin32_safestat fixes the problem that the kluge tried to work around. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
On Monday 12 May 2008 14:40:46 Pavel Stehule wrote:
> 2008/5/12 Tom Lane <[EMAIL PROTECTED]>:
> > "Pavel Stehule" <[EMAIL PROTECTED]> writes:
> >> 2008/5/12 Tom Lane <[EMAIL PROTECTED]>:
> >>> It would get less annoying if we allowed user-declared exception names.
> >>
> >> Tom, it's exactly like my patch that you rejected two years ago.
> >
> > Uh, no, not "exactly like" --- that patch doesn't have anything to do
> > with the SQL/PSM syntax, and not much with the SQL/PSM semantics.
> > As I read the spec, a condition name isn't a variable and so you can't
> > do runtime assignment to it (and unlike Neil, I don't think you should
> > be able to do so).
>
> In plpgsql I prefer PL/SQL syntax. Mix SQL/PSM and PL/SQL will be
> mismas. But I like idea, so you can set dynamically SQLSTATE and other
> params - because you can write own wrapper for RAISE statement. It's
> can be usable for centralized exception management. I can do it in C,
> but there are lot of users, that could use only plpgsql.
>
I think nod's toward PL/SQL compatability should be given in general. If
people want a PSM style language, let's work on getting pl/psm better
maintained or integrated.
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
Simon Riggs wrote: On Mon, 2008-05-12 at 18:58 -0400, Andrew Dunstan wrote: No, it had to do with pg_standby waiting for a WAL file that had already gone, somehow. I will try to reproduce it when I get a spare moment. Sounds like the bug I just fixed. Yes, so I see. I didn't have that fix, so I'll test again with the patch. There is an outstanding Windows issue with pg_standby that your help would be appreciated with, shown on latest commitfest page. It's a Windows issue and I don't maintain a Windows dev environment. The patch has been rejected for now, according to the Commitfest page. Not sure what you want my help on. Well, the patch was rejected long ago, not sure why its in this commitfest. But its an open issue on the Windows port. Surely the right fix is to use the recently implemented pgwin32_safestat() (if we aren't already - I suspect we probably are) and remove the kluge in pg_standby.c. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
On Monday 12 May 2008 18:58:37 Andrew Dunstan wrote:
> Simon Riggs wrote:
> >> Lastly, not quite related to this output, but in the same general area,
> >> should we have an option on pg_standby to allow removing the archive
> >> file after it has been restored?
> >
> > There already is one, but its more complex than that. (%r)
>
> I was using %r. But the WAL files that have been restored (according to
> the log) are still in the archive dir. So it looks like %r isn't working
> properly.
>
Are you sure you've moved passed the latest restart point? Just because a WAL
file has been processed doesn't mean it can be deleted.
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
On Mon, 2008-05-12 at 18:58 -0400, Andrew Dunstan wrote: > No, it had to do with pg_standby waiting for a WAL file that had already > gone, somehow. I will try to reproduce it when I get a spare moment. Sounds like the bug I just fixed. > > There is an outstanding Windows issue with pg_standby that your help > > would be appreciated with, shown on latest commitfest page. It's a > > Windows issue and I don't maintain a Windows dev environment. > The patch has been rejected for now, according to the Commitfest page. > Not sure what you want my help on. Well, the patch was rejected long ago, not sure why its in this commitfest. But its an open issue on the Windows port. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
Simon Riggs wrote: What is more, I apparently managed to get the recovery server to lose a WAL file and hang totally by having a bad recovery.conf. Triple ick. Sounds like a bug you should report in the normal way. Correctness is paramount. Or are you confusing the message in the log for file AA with an error? No, it had to do with pg_standby waiting for a WAL file that had already gone, somehow. I will try to reproduce it when I get a spare moment. Second, what is all this about .history files? My understanding is that they are not necessary, so surely we should try to stat them to see if they are present before trying to copy them. These lines are going to confuse a lot of people, I suspect (including me). I try to keep it as simple as possible, since much of this code only gets run when you really need it to work. The request for the .history file and the cp are examples of that. I don't follow. AFAICT no .history file was in fact archived. ISTM that in this case we should only call RestoreWALFileForRecovery if the file in fact exists. Lastly, not quite related to this output, but in the same general area, should we have an option on pg_standby to allow removing the archive file after it has been restored? There already is one, but its more complex than that. (%r) I was using %r. But the WAL files that have been restored (according to the log) are still in the archive dir. So it looks like %r isn't working properly. There is an outstanding Windows issue with pg_standby that your help would be appreciated with, shown on latest commitfest page. It's a Windows issue and I don't maintain a Windows dev environment. The patch has been rejected for now, according to the Commitfest page. Not sure what you want my help on. BTW, none of what I reported was on Windows - it's on Linux. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fairly serious bug induced by latest guc enum changes
Magnus Hagander <[EMAIL PROTECTED]> writes: > I still think going with the older method would be the safest, though, > for the other reasons. You agree? Seems reasonable to me. > (I assume you mean GUC enum here, that seems fairly obvious) Sorry, was writing in too much of a hurry. > In these, the value was previously derived from a string and set in the > hook. It's now set directly by the GUC code, and the hook only updates > "other things" (setting the actual syslog facility, and resetting the > cache, respectively). > I think that means there are no bugs there. Yeah, that's fine. I think though that I may have created a bug inside GUC itself: the new stacking code could conceivably fail (palloc error) between success return from the assign hook and setting up the stack entry that is needed to undo the assignment on abort. In this situation the assign hook would've made its "other thing" changes but there is no GUC state to cause the hook to be called again to undo 'em. I need to fix it so that any palloc'ing needed is done before calling the assign hook. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] odd output in restore mode
On Mon, 2008-05-12 at 16:57 -0400, Andrew Dunstan wrote: > I have just been working on setting up a continuous recovery failover > system, and noticed some odd log lines, shown below. (Using 8.3). Hmmm, well, the first time you use something complex, there are some surprising features, I guess. Most especially the log lines are there to allow production issues to be diagnosed, not to create a beautiful log. Many of the things that look somewhat strange are there for a reason, since a wide range of options and save-your-customers-ass scenarios are covered by the recovery code. Suggestions for improvement are always welcome and you are welcome to suggest doc changes, as many people do. > First note that our parsing of recovery.conf in xlog.c is pretty bad, > and at least we need to document the quirks if it's not going to be > fixed. log_restartpoints is said to be boolean, but when I set it to an > unquoted true I got a fatal error, while a quoted 'on' sets it to false, > as seen. Ick. Yes, some improvements are definitely due there. > What is more, I apparently managed to get the recovery > server to lose a WAL file and hang totally by having a bad > recovery.conf. Triple ick. Sounds like a bug you should report in the normal way. Correctness is paramount. Or are you confusing the message in the log for file AA with an error? > Second, what is all this about .history files? My understanding is that > they are not necessary, so surely we should try to stat them to see if > they are present before trying to copy them. These lines are going to > confuse a lot of people, I suspect (including me). I try to keep it as simple as possible, since much of this code only gets run when you really need it to work. The request for the .history file and the cp are examples of that. > Lastly, not quite related to this output, but in the same general area, > should we have an option on pg_standby to allow removing the archive > file after it has been restored? There already is one, but its more complex than that. (%r) > LOG: database system was interrupted; last known up at 2008-05-12 > 15:18:23 EDT > LOG: starting archive recovery > LOG: log_restartpoints = false > LOG: restore_command = '../bin/pg_standby -t ../common_archive/failover > ../common_archive %f %p %r ' > cp: cannot stat `../common_archive/0001.history': No such file or > directory > cp: cannot stat `../common_archive/0001.history': No such file or > directory > cp: cannot stat `../common_archive/0001.history': No such file or > directory > LOG: restored log file "000100A5.0068.backup" from > archive > LOG: restored log file "000100A5" from archive > LOG: automatic recovery in progress > LOG: redo starts at 0/A5B0 > LOG: restored log file "000100A6" from archive > LOG: restored log file "000100A7" from archive > LOG: restored log file "000100A8" from archive > LOG: restored log file "000100A9" from archive > trigger file found > LOG: could not open file "pg_xlog/000100AA" (log file > 0, segment 170): No such file or directory > LOG: redo done at 0/A968 > LOG: restored log file "000100A9" from archive > cp: cannot stat `../common_archive/0002.history': No such file or > directory > cp: cannot stat `../common_archive/0002.history': No such file or > directory > cp: cannot stat `../common_archive/0002.history': No such file or > directory > LOG: selected new timeline ID: 2 > cp: cannot stat `../common_archive/0001.history': No such file or > directory > cp: cannot stat `../common_archive/0001.history': No such file or > directory > cp: cannot stat `../common_archive/0001.history': No such file or > directory > LOG: archive recovery complete > LOG: database system is ready to accept connections > LOG: autovacuum launcher started There is an outstanding Windows issue with pg_standby that your help would be appreciated with, shown on latest commitfest page. It's a Windows issue and I don't maintain a Windows dev environment. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Alvaro Herrera napsal(a): I try to play with lint now if it gets better results. Ok, good. Hmm, It generates a lot of unnecessary includes in *.c files. I have checked only couple of them and it seems that they are really unnecessary. A attach output. Unfortunately, it does not detect missing heapam.h from bufpage.h. However, I have not tested all magic switches yet :-). There are also several reports about system headers file, but it could be platform specific warning. Zdenek include.out.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] odd output in restore mode
I have just been working on setting up a continuous recovery failover system, and noticed some odd log lines, shown below. (Using 8.3). First note that our parsing of recovery.conf in xlog.c is pretty bad, and at least we need to document the quirks if it's not going to be fixed. log_restartpoints is said to be boolean, but when I set it to an unquoted true I got a fatal error, while a quoted 'on' sets it to false, as seen. Ick. What is more, I apparently managed to get the recovery server to lose a WAL file and hang totally by having a bad recovery.conf. Triple ick. Second, what is all this about .history files? My understanding is that they are not necessary, so surely we should try to stat them to see if they are present before trying to copy them. These lines are going to confuse a lot of people, I suspect (including me). Lastly, not quite related to this output, but in the same general area, should we have an option on pg_standby to allow removing the archive file after it has been restored? cheers andrew LOG: database system was interrupted; last known up at 2008-05-12 15:18:23 EDT LOG: starting archive recovery LOG: log_restartpoints = false LOG: restore_command = '../bin/pg_standby -t ../common_archive/failover ../common_archive %f %p %r ' cp: cannot stat `../common_archive/0001.history': No such file or directory cp: cannot stat `../common_archive/0001.history': No such file or directory cp: cannot stat `../common_archive/0001.history': No such file or directory LOG: restored log file "000100A5.0068.backup" from archive LOG: restored log file "000100A5" from archive LOG: automatic recovery in progress LOG: redo starts at 0/A5B0 LOG: restored log file "000100A6" from archive LOG: restored log file "000100A7" from archive LOG: restored log file "000100A8" from archive LOG: restored log file "000100A9" from archive trigger file found LOG: could not open file "pg_xlog/000100AA" (log file 0, segment 170): No such file or directory LOG: redo done at 0/A968 LOG: restored log file "000100A9" from archive cp: cannot stat `../common_archive/0002.history': No such file or directory cp: cannot stat `../common_archive/0002.history': No such file or directory cp: cannot stat `../common_archive/0002.history': No such file or directory LOG: selected new timeline ID: 2 cp: cannot stat `../common_archive/0001.history': No such file or directory cp: cannot stat `../common_archive/0001.history': No such file or directory cp: cannot stat `../common_archive/0001.history': No such file or directory LOG: archive recovery complete LOG: database system is ready to accept connections LOG: autovacuum launcher started -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fairly serious bug induced by latest guc enum changes
Tom Lane wrote: > I see that assign_xlog_sync_method() is still assigning to > sync_method. This is 100% wrong: an assign hook is chartered to set > derived values, but not to set the GUC variable itself. This will > for example result in set_config_option() stacking the wrong value > (the already-updated one) as the value to roll back to if the > transaction aborts. Hm. I never quite did get my head around how the stacking work, that's probably why :-) > You could just remove the assignment from assign_xlog_sync_method, > although it might look a bit odd to be setting open_sync_bit but > not sync_method. It also bothers me slightly that the derived and > main values wouldn't be set at exactly the same point --- problems > inside guc.c might lead to them getting out of sync. > > Another possibility is to stick with something equivalent to the > former design: what GUC thinks is the variable is just a dummy static > integer in guc.c, and the assign hook is still setting the "real" > value that the rest of the code looks at. A minor advantage of this > second way is that the "real" value could still be declared as enum > rather than int. That value never was an enum, so that part is not an advantage. I still think going with the older method would be the safest, though, for the other reasons. You agree? > Please fix this, and take another look at the prior WAL enum changes > to see if the same problem hasn't been created elsewhere. (I assume you mean GUC enum here, that seems fairly obvious) The only other assign hooks are assign_syslog_facility and assign_session_replication_role. The changes there are: In these, the value was previously derived from a string and set in the hook. It's now set directly by the GUC code, and the hook only updates "other things" (setting the actual syslog facility, and resetting the cache, respectively). I think that means there are no bugs there. //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fairly serious bug induced by latest guc enum changes
I see that assign_xlog_sync_method() is still assigning to sync_method. This is 100% wrong: an assign hook is chartered to set derived values, but not to set the GUC variable itself. This will for example result in set_config_option() stacking the wrong value (the already-updated one) as the value to roll back to if the transaction aborts. You could just remove the assignment from assign_xlog_sync_method, although it might look a bit odd to be setting open_sync_bit but not sync_method. It also bothers me slightly that the derived and main values wouldn't be set at exactly the same point --- problems inside guc.c might lead to them getting out of sync. Another possibility is to stick with something equivalent to the former design: what GUC thinks is the variable is just a dummy static integer in guc.c, and the assign hook is still setting the "real" value that the rest of the code looks at. A minor advantage of this second way is that the "real" value could still be declared as enum rather than int. Please fix this, and take another look at the prior WAL enum changes to see if the same problem hasn't been created elsewhere. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Zdenek Kotala wrote: > Alvaro Herrera napsal(a): >> Well, then it is not working, because transam.h is missing from >> bufpage.h ... > > It tried catch problems with defines not with datatypes. If you mean that > TransactionID is not defined. No, I mean that TransactionIdIsNormal() is used in PageSetPrunable(). > I try to play with lint now if it gets better results. Ok, good. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Alvaro Herrera napsal(a): Zdenek Kotala wrote: Alvaro Herrera napsal(a): Zdenek Kotala wrote: I attached script which should check it. In first step it runs C preprocessor on each header (postgres.h is included as well). The output from first step is processed again with preprocessor and define.h is included. So were you able to detect anything bogus with this technique? No, everything looks OK. Well, then it is not working, because transam.h is missing from bufpage.h ... It tried catch problems with defines not with datatypes. If you mean that TransactionID is not defined. I try to play with lint now if it gets better results. Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > I like this syntax, but I am not if it's good idea add new similar > statement. I don't know - but maybe it's can be better then extending > RAISE - and way to get consensus. I looked a bit more at the SQL spec. It already defines a MESSAGE_TEXT, which arguably is what we should use for the primary message item, but that seems unpleasantly long for something that's going to be used in pretty much every SIGNAL command. Also there's a question of whether it's supposed to mean the *complete* message delivered to a client, which would subsume DETAIL, HINT, etc in our scheme. So I'm a bit tempted to stick with MESSAGE, DETAIL, and HINT as the settable options if we go with SQL/PSM-derived syntax. We'd also want LEVEL or something to be able to specify non-ERROR elog levels. Also, as to the re-throw-an-error capability, SQL/PSM defines a RESIGNAL command that does this. I propose implementing only the parameterless variant of this, at least for the time being. (The spec appears to intend that RESIGNAL can override selected fields of the error being re-thrown, which doesn't strike me as a terribly good idea --- you could end up with a completely nonsensical error report.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Convert wal_sync_method to guc enum.
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: > > I need to leave for a couple of hours, will look again when I get > > back. But so far, I'm quite surprised. Here's my reasoning, please > > poke holes in it :-) > > I think you forgot to handle SYNC_METHOD_OPEN_DSYNC in > issue_xlog_fsync. If you are going to split SYNC_METHOD_OPEN into two > codes, you need to handle both those codes everywhere > SYNC_METHOD_OPEN was formerly referenced ... That was it, fixed. I missed the fact that the same error message occured twice in the file. I blame lack of caffeine comined with having hacked Latex code. It just kills the brain. Thanks for the pointer! //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Zdenek Kotala wrote: > Alvaro Herrera napsal(a): >> Zdenek Kotala wrote: >>> I attached script which should check it. In first step it runs C >>> preprocessor on each header (postgres.h is included as well). The >>> output from first step is processed again with preprocessor and >>> define.h is included. >> So were you able to detect anything bogus with this technique? > > No, everything looks OK. Well, then it is not working, because transam.h is missing from bufpage.h ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: Integrity check
Alvaro Herrera napsal(a): Zdenek Kotala escribió: Regarding to Robert Mach's work during Google SOC on data integrity check. I would like to improve storage module and implement some Robert's code into the core. Did this go anywhere? I did not catch May commit fest :(. I plan to send core improvement to next commit fest. Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Alvaro Herrera napsal(a):
Zdenek Kotala wrote:
Alvaro Herrera wrote:
(Digging further, it seems like bufpage.h should also include transam.h
in order to get TransactionIdIsNormal ... I start to wonder how many
problems of this nature we have on our headers. Without having a way to
detect whether the defined macros are valid, it seems hard to check
programatically, however.)
I attached script which should check it. In first step it runs C
preprocessor on each header (postgres.h is included as well). The output
from first step is processed again with preprocessor and define.h is
included. Define.h contains "all" used macros in following format:
#define SIGABRT "NOT_EXPANDED_SIGABRT"
Main problem is how to generate define.h. I used following magic:
grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1
-d"("
but it generates some noise as well. Maybe some Perl or AWK magic should be
better.
So were you able to detect anything bogus with this technique?
No, everything looks OK.
Zdenek
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>:
> "Kevin Grittner" <[EMAIL PROTECTED]> writes:
>> I'm probably in the minority, but I care more about SQL/PSM
>> compatibility than Oracle compatibility.
>
> Well, a different line of attack would be to leave RAISE as-is and adopt
> the SQL/PSM syntax for a modernized command. What I'm seeing in Part 4
> is
>
> ::=
> SIGNAL
>[ ]
>
> ::=
>
> |
>
> ::=
>
>
> ::=
> SQLSTATE [ VALUE ]
>
> ::=
> SET
>
> ::=
> [ {
> }... ]
>
> ::=
> value specification>
>
> If we're willing to invent Postgres-specific names> for MESSAGE, DETAIL, etc, then this is just about isomorphic to
> the proposed RAISE syntax, except that if you want an elog level other
> than ERROR you'd have to specify it as an item in the SET-list.
>
> BTW, the spec also uses and as above
> in handler declarations, so it looks like both Pavel and I got it wrong
> about how to extend the EXCEPTION syntax: it should be
> SQLSTATE [VALUE] 'x'
>
next step can be extension of GET DIAGNOSTIC statement ...
Pavel
p.s. CASE statement going from SQL/PSM too. so why not?
>regards, tom lane
>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>:
> "Kevin Grittner" <[EMAIL PROTECTED]> writes:
>> I'm probably in the minority, but I care more about SQL/PSM
>> compatibility than Oracle compatibility.
>
> Well, a different line of attack would be to leave RAISE as-is and adopt
> the SQL/PSM syntax for a modernized command. What I'm seeing in Part 4
> is
>
> ::=
> SIGNAL
>[ ]
>
> ::=
>
> |
>
> ::=
>
>
> ::=
> SQLSTATE [ VALUE ]
>
> ::=
> SET
>
> ::=
> [ {
> }... ]
>
> ::=
> value specification>
>
> If we're willing to invent Postgres-specific names> for MESSAGE, DETAIL, etc, then this is just about isomorphic to
> the proposed RAISE syntax, except that if you want an elog level other
> than ERROR you'd have to specify it as an item in the SET-list.
>
> BTW, the spec also uses and as above
> in handler declarations, so it looks like both Pavel and I got it wrong
> about how to extend the EXCEPTION syntax: it should be
> SQLSTATE [VALUE] 'x'
>
>regards, tom lane
>
I like this syntax, but I am not if it's good idea add new similar
statement. I don't know - but maybe it's can be better then extending
RAISE - and way to get consensus.
Regards
Pavel Stehule
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
"Kevin Grittner" <[EMAIL PROTECTED]> writes:
> I'm probably in the minority, but I care more about SQL/PSM
> compatibility than Oracle compatibility.
Well, a different line of attack would be to leave RAISE as-is and adopt
the SQL/PSM syntax for a modernized command. What I'm seeing in Part 4
is
::=
SIGNAL
[ ]
::=
|
::=
::=
SQLSTATE [ VALUE ]
::=
SET
::=
[ {
}... ]
::=
If we're willing to invent Postgres-specific for MESSAGE, DETAIL, etc, then this is just about isomorphic to
the proposed RAISE syntax, except that if you want an elog level other
than ERROR you'd have to specify it as an item in the SET-list.
BTW, the spec also uses and as above
in handler declarations, so it looks like both Pavel and I got it wrong
about how to extend the EXCEPTION syntax: it should be
SQLSTATE [VALUE] 'x'
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> 2008/5/12 Tom Lane <[EMAIL PROTECTED]>: >>> It would get less annoying if we allowed user-declared exception names. > >> Tom, it's exactly like my patch that you rejected two years ago. > > Uh, no, not "exactly like" --- that patch doesn't have anything to do > with the SQL/PSM syntax, and not much with the SQL/PSM semantics. > As I read the spec, a condition name isn't a variable and so you can't > do runtime assignment to it (and unlike Neil, I don't think you should > be able to do so). > In plpgsql I prefer PL/SQL syntax. Mix SQL/PSM and PL/SQL will be mismas. But I like idea, so you can set dynamically SQLSTATE and other params - because you can write own wrapper for RAISE statement. It's can be usable for centralized exception management. I can do it in C, but there are lot of users, that could use only plpgsql. >regards, tom lane > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>: > I've started to look over Pavel's revised RAISE patch > http://archives.postgresql.org/pgsql-patches/2008-05/msg00187.php > and I've got a few quibbles with the syntax choices. > > Pavel proposes extending RAISE like this: > > RAISE level 'format' [, expression [, ...] ] [ USING ( option = value [, ... > ] ) ] > > the part before USING being what we had already. Each "option" keyword > is one of SQLSTATE, CONDITION, DETAIL, or HINT, and each "value" is a > string-valued expression. SQLSTATE takes a value like '22012' while the > (mutually exclusive) CONDITION takes a value like 'DIVISION_BY_ZERO'. > DETAIL and HINT allow those parts of an error report to be filled in. > > I'd like to propose the following changes: > > 1. The parentheses around the USING list seem useless; let's drop 'em. it hasn't any precedent in PostgreSQL. But option list in parenthesesis > > 2. I think the separation between SQLSTATE and CONDITION is just > complication. A SQLSTATE is required to be exactly 5 digits and/or > upper case ASCII letters; I see no realistic prospect that any condition > name would ever look like a SQLSTATE (and we could certainly adjust > condition names to prevent it, if anyone would make such an unhappy > choice). So I think we could unify these options into one. I think > CODE might be a better choice for the option name than SQLSTATE (since > the latter already has a meaning in pl/pgsql, ie the function that > gives you the code for the currently thrown error) --- thoughts? > CODE isn't well name. It's too much general. If you would to drop one identifier I prefer CONDITION or some similar (minim. ERRCODE). In plpgsql SQLSTATE is keyword, and in some implementations it's implicit variables too. Using it, it's more readable - more verbose - it's in spirit of PL/SQL. Maybe: CONDITION = expression returning name | SQLSTATE expression returning SQLSTATE. > 3. I think we should allow the user to specify the error message the > same way as the other options, that is >RAISE level USING MESSAGE = string_expression [ , ... ] > The %-format business has always struck me as a bit weird, and it's > much more so if we aren't handling the other error report components > in the same fashion. So we ought to provide an alternative that's > more uniform. > > Now, the elephant in the room is the issue of Oracle compatibility. > None of this looks anything even a little bit like Oracle's RAISE > command. Oracle allows >RAISE exception_name ; >RAISE ; > where the second case is allowed only in an EXCEPTION handler and > means to re-throw the current error. I think the latter is a very > good idea and we ought to support it. Right now there's no way to > re-throw an error without information loss, and that'll get a lot > worse with these additions to what RAISE can throw. I'm less > excited about the condition-name-only syntax; that seems awfully > impoverished given the lack of any way to supply a specific error > message or data values. Still, we could imagine people wanting > something like >RAISE condition_name USING message = string_expression > where the condition_name would substitute for the CODE option. > I think we could support this as long as the condition name were > given as an exception name rather than a string literal (otherwise > it looks too much like our legacy syntax). Comments? Is anyone > excited about that one way or the other? I agree with this syntax, but I propose using code only with SQLSTATE keyword RAISE 22345 is ugly RAISE SQLSTATE 22345 is better and on this position can be parametrized - now I thing, so SQLSTATE and CONDITION shouldn't be defined in USING. variants: RAISE unique_violation USING message = '', hint = ''; RAISE SQLSTATE USING message ... RAISE variable USING ... RAISE SQLSTATE USING ... > > Lastly: to allow users to catch errors thrown with user-defined > SQLSTATEs, Pavel proposes extending the syntax of EXCEPTION WHEN lists > so that an error code can be specified in either of these styles: >DIVISION_BY_ZERO >SQLSTATE 22012 > I find the second style rather weird, and I think it probably doesn't > even work for cases like 2201F (which isn't going to get lexed as > a single token). I would suggest a quoted literal and drop the noise > word, so that the alternatives are >DIVISION_BY_ZERO >'22012' > Comments? it's true - it's have to quoted literal or other hand, solve it on lexer level. But it's not important on plpgsql - there we can choice the most simple solution. Regards Pavel Stehule > > If we can get some consensus I'll undertake to adjust the patch > accordingly. > >regards, tom lane > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > 2008/5/12 Tom Lane <[EMAIL PROTECTED]>: >> It would get less annoying if we allowed user-declared exception names. > Tom, it's exactly like my patch that you rejected two years ago. Uh, no, not "exactly like" --- that patch doesn't have anything to do with the SQL/PSM syntax, and not much with the SQL/PSM semantics. As I read the spec, a condition name isn't a variable and so you can't do runtime assignment to it (and unlike Neil, I don't think you should be able to do so). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Tom Lane <[EMAIL PROTECTED]>: > "Brendan Jurd" <[EMAIL PROTECTED]> writes: >> I agree that the % formatting in the RAISE message is weird, but it is >> useful. > > Sure, I'm not proposing removing it. > >> What would we do if the user specifies a %-formatted message as well >> as a MESSAGE option? > > Throw an error (just like if they specified the same option type twice). > >> I like "RAISE condition_name", can we support it in conjunction with >> the current syntax? That is: > >> RAISE level [condition] [string literal, [parameter, ...]] [USING >> [option = value, ...]] > > Well, it's sort of a mess because level has to become optional in order > to be Oracle-compatible (or PSM-compliant, if Kevin is correct). We > could get away with it only if the condition were not allowed to be > a string literal, which I guess is tolerable but it's a bit annoying. > It would get less annoying if we allowed user-declared exception names. > I find the Oracle syntax for those to be spectacularly awful: > >DECLARE > deadlock_detected EXCEPTION; > PRAGMA EXCEPTION_INIT(deadlock_detected, -60); > > but it sounds like SQL/PSM's syntax isn't so bad. I could live with > the reported > >DECLARE > condition-name CONDITION FOR SQLSTATE VALUE character-literal > > However, that's a separate feature and I don't want to get into it as > part of the current patch. > >regards, tom lane > Tom, it's exactly like my patch that you rejected two years ago. http://archives.postgresql.org/pgsql-patches/2005-07/msg00176.php Pavel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
2008/5/12 Kevin Grittner <[EMAIL PROTECTED]>: Tom Lane <[EMAIL PROTECTED]> wrote: > >> Now, the elephant in the room is the issue of Oracle compatibility. >> None of this looks anything even a little bit like Oracle's RAISE >> command. Oracle allows >> RAISE exception_name ; >> RAISE ; > > I'm probably in the minority, but I care more about SQL/PSM > compatibility than Oracle compatibility. I would hope that the ISO > standard is at least a gorilla sitting in the corner of the room. > > If it's not too impractical, a nod toward these would be good: > > DECLARE condition-name CONDITION FOR SQLSTATE VALUE character-literal > > SIGNAL condition-name > > -Kevin plpgsql can't be SQL/PSM compatible - it's goal other language plpgpsm, and there is condition declared via standard. Pavel > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
"Brendan Jurd" <[EMAIL PROTECTED]> writes: > I agree that the % formatting in the RAISE message is weird, but it is > useful. Sure, I'm not proposing removing it. > What would we do if the user specifies a %-formatted message as well > as a MESSAGE option? Throw an error (just like if they specified the same option type twice). > I like "RAISE condition_name", can we support it in conjunction with > the current syntax? That is: > RAISE level [condition] [string literal, [parameter, ...]] [USING > [option = value, ...]] Well, it's sort of a mess because level has to become optional in order to be Oracle-compatible (or PSM-compliant, if Kevin is correct). We could get away with it only if the condition were not allowed to be a string literal, which I guess is tolerable but it's a bit annoying. It would get less annoying if we allowed user-declared exception names. I find the Oracle syntax for those to be spectacularly awful: DECLARE deadlock_detected EXCEPTION; PRAGMA EXCEPTION_INIT(deadlock_detected, -60); but it sounds like SQL/PSM's syntax isn't so bad. I could live with the reported DECLARE condition-name CONDITION FOR SQLSTATE VALUE character-literal However, that's a separate feature and I don't want to get into it as part of the current patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XIDs and big boxes again ...
Hans-Juergen Schoenig wrote: i suggest to introduce a --with-long-xids flag which would give me 62 / 64 bit XIDs per vacuum on the entire database. this should be fairly easy to implement. i am not too concerned about the size of the tuple header here - if we waste 500 gb of storage here i am totally fine. As you say later in the thread, you are on 8.1. Alot of work has gone into reducing the effect, impact and frequency of XID wrap around and vacuuming since then. In 8.3 transactions that don't actually update a table no long use a real XID and autovacuum you no longer need a database wide vacuum to solve the XID wraparound problem, so I think the answer is upgrade to 8.3 and see if you still have this problem. Matthew O'Connor -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
On Tue, May 13, 2008 at 2:53 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > 1. The parentheses around the USING list seem useless; let's drop 'em. Yes. > > 2. I think the separation between SQLSTATE and CONDITION is just > complication. A SQLSTATE is required to be exactly 5 digits and/or > upper case ASCII letters; I see no realistic prospect that any condition > name would ever look like a SQLSTATE (and we could certainly adjust > condition names to prevent it, if anyone would make such an unhappy > choice). So I think we could unify these options into one. I think > CODE might be a better choice for the option name than SQLSTATE (since > the latter already has a meaning in pl/pgsql, ie the function that > gives you the code for the currently thrown error) --- thoughts? > Yes. CODE has a nice symmetry with the use of errcode in ereport as well. > 3. I think we should allow the user to specify the error message the > same way as the other options, that is > RAISE level USING MESSAGE = string_expression [ , ... ] > The %-format business has always struck me as a bit weird, and it's > much more so if we aren't handling the other error report components > in the same fashion. So we ought to provide an alternative that's > more uniform. > I agree that the % formatting in the RAISE message is weird, but it is useful. When you're writing an exception message you almost always want to substitute in information about the values (causing|involved in) the exception. With MESSAGE = string you would have to concatenate the pieces together with ||, which is longer and less readable. I support adding the MESSAGE option (again, nice symmetry with ereport), but will probably continue to use the %-formatted message style in my applications. What would we do if the user specifies a %-formatted message as well as a MESSAGE option? I think it would be reasonable to bail out with a message explaining that they should use the formatted message XOR the MESSAGE option. > Now, the elephant in the room is the issue of Oracle compatibility. > None of this looks anything even a little bit like Oracle's RAISE > command. Oracle allows > RAISE exception_name ; > RAISE ; > where the second case is allowed only in an EXCEPTION handler and > means to re-throw the current error. I think the latter is a very > good idea and we ought to support it. Right now there's no way to > re-throw an error without information loss, and that'll get a lot > worse with these additions to what RAISE can throw. Yes! I've wished for a re-throw ability several times in the past. > I'm less > excited about the condition-name-only syntax; that seems awfully > impoverished given the lack of any way to supply a specific error > message or data values. Still, we could imagine people wanting > something like > RAISE condition_name USING message = string_expression > where the condition_name would substitute for the CODE option. > I think we could support this as long as the condition name were > given as an exception name rather than a string literal (otherwise > it looks too much like our legacy syntax). Comments? Is anyone > excited about that one way or the other? I like "RAISE condition_name", can we support it in conjunction with the current syntax? That is: RAISE level [condition] [string literal, [parameter, ...]] [USING [option = value, ...]] Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Convert wal_sync_method to guc enum.
Magnus Hagander <[EMAIL PROTECTED]> writes: > I need to leave for a couple of hours, will look again when I get back. > But so far, I'm quite surprised. Here's my reasoning, please poke holes > in it :-) I think you forgot to handle SYNC_METHOD_OPEN_DSYNC in issue_xlog_fsync. If you are going to split SYNC_METHOD_OPEN into two codes, you need to handle both those codes everywhere SYNC_METHOD_OPEN was formerly referenced ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Syntax decisions for pl/pgsql RAISE extension
>>> Tom Lane <[EMAIL PROTECTED]> wrote: > Now, the elephant in the room is the issue of Oracle compatibility. > None of this looks anything even a little bit like Oracle's RAISE > command. Oracle allows > RAISE exception_name ; > RAISE ; I'm probably in the minority, but I care more about SQL/PSM compatibility than Oracle compatibility. I would hope that the ISO standard is at least a gorilla sitting in the corner of the room. If it's not too impractical, a nod toward these would be good: DECLARE condition-name CONDITION FOR SQLSTATE VALUE character-literal SIGNAL condition-name -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Setting a pre-existing index as a primary key
Tom Lane wrote: BTW, aside from selecting the index the command would have to verify that the indexed columns are all NOT NULL. We could either have it just throw an error if they aren't, or have it silently try to do an ALTER SET NOT NULL, which would require a table scan. I'm going to argue for the "just throw an error" choice. I don't like the idea of a utility command that takes exclusive lock and then is either near-instantaneous or slow depending on factors not immediately obvious. +1 cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: Integrity check
Zdenek Kotala escribió: > Regarding to Robert Mach's work during Google SOC on data integrity > check. I would like to improve storage module and implement some > Robert's code into the core. Did this go anywhere? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Setting a pre-existing index as a primary key
Bruce Momjian <[EMAIL PROTECTED]> writes: > I realize most feel we don't need to add a rename to this, but there are > two more reasons _not_ to do this. One other thought I had about this is that the proposed syntax ALTER TABLE tab ADD PRIMARY KEY (col [, ...]) USING INDEX foo is not well chosen anyway. It forces the user to provide a column name list matching the index, which is just extra typing and extra cognitive burden, and it forces the system to have code checking that this list matches the specified index. So I'm thinking it should look like ALTER TABLE tab ADD PRIMARY KEY USING INDEX foo or maybe just ALTER TABLE tab ADD PRIMARY KEY USING foo This would be a separate grammar production having nothing to do with the ADD CONSTRAINT syntax. It's not ambiguous since the column name list is required in ADD CONSTRAINT. BTW, aside from selecting the index the command would have to verify that the indexed columns are all NOT NULL. We could either have it just throw an error if they aren't, or have it silently try to do an ALTER SET NOT NULL, which would require a table scan. I'm going to argue for the "just throw an error" choice. I don't like the idea of a utility command that takes exclusive lock and then is either near-instantaneous or slow depending on factors not immediately obvious. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Syntax decisions for pl/pgsql RAISE extension
I've started to look over Pavel's revised RAISE patch http://archives.postgresql.org/pgsql-patches/2008-05/msg00187.php and I've got a few quibbles with the syntax choices. Pavel proposes extending RAISE like this: RAISE level 'format' [, expression [, ...] ] [ USING ( option = value [, ... ] ) ] the part before USING being what we had already. Each "option" keyword is one of SQLSTATE, CONDITION, DETAIL, or HINT, and each "value" is a string-valued expression. SQLSTATE takes a value like '22012' while the (mutually exclusive) CONDITION takes a value like 'DIVISION_BY_ZERO'. DETAIL and HINT allow those parts of an error report to be filled in. I'd like to propose the following changes: 1. The parentheses around the USING list seem useless; let's drop 'em. 2. I think the separation between SQLSTATE and CONDITION is just complication. A SQLSTATE is required to be exactly 5 digits and/or upper case ASCII letters; I see no realistic prospect that any condition name would ever look like a SQLSTATE (and we could certainly adjust condition names to prevent it, if anyone would make such an unhappy choice). So I think we could unify these options into one. I think CODE might be a better choice for the option name than SQLSTATE (since the latter already has a meaning in pl/pgsql, ie the function that gives you the code for the currently thrown error) --- thoughts? 3. I think we should allow the user to specify the error message the same way as the other options, that is RAISE level USING MESSAGE = string_expression [ , ... ] The %-format business has always struck me as a bit weird, and it's much more so if we aren't handling the other error report components in the same fashion. So we ought to provide an alternative that's more uniform. Now, the elephant in the room is the issue of Oracle compatibility. None of this looks anything even a little bit like Oracle's RAISE command. Oracle allows RAISE exception_name ; RAISE ; where the second case is allowed only in an EXCEPTION handler and means to re-throw the current error. I think the latter is a very good idea and we ought to support it. Right now there's no way to re-throw an error without information loss, and that'll get a lot worse with these additions to what RAISE can throw. I'm less excited about the condition-name-only syntax; that seems awfully impoverished given the lack of any way to supply a specific error message or data values. Still, we could imagine people wanting something like RAISE condition_name USING message = string_expression where the condition_name would substitute for the CODE option. I think we could support this as long as the condition name were given as an exception name rather than a string literal (otherwise it looks too much like our legacy syntax). Comments? Is anyone excited about that one way or the other? Lastly: to allow users to catch errors thrown with user-defined SQLSTATEs, Pavel proposes extending the syntax of EXCEPTION WHEN lists so that an error code can be specified in either of these styles: DIVISION_BY_ZERO SQLSTATE 22012 I find the second style rather weird, and I think it probably doesn't even work for cases like 2201F (which isn't going to get lexed as a single token). I would suggest a quoted literal and drop the noise word, so that the alternatives are DIVISION_BY_ZERO '22012' Comments? If we can get some consensus I'll undertake to adjust the patch accordingly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] another ecpg crash
On Sun, May 11, 2008 at 01:50:22AM -0300, Euler Taveira de Oliveira wrote: > I found another bug when using 'exec sql include filename'. If you use a > filename that doesn't exist, ecpg crashes while trying to close a null > pointer. The above test case shows it. A possible fix is attached. Thanks again, I just committed the fix. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ecpg crash
On Sat, May 10, 2008 at 12:14:57AM -0300, Euler Taveira de Oliveira wrote: > While i'm working on a ecpg patch I found a bug in ecpg code. The simple > program above could reproduce it. But basically it crashes (segfault) It appeared that the whole error checking code was missing. Fixed now. Thanks for reporting this. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Setting a pre-existing index as a primary key
Joshua D. Drake wrote: > Tom Lane wrote: > > >> Well it should be optional but it would be nice if we had the option to > >> have it renamed per the default... meaning the same output if I were to > >> do this: > > > > If you want that, you can rename the index (either before or afterwards). > > I don't see any reason to clutter the make-constraint-from-index command > > with questions of renaming. > > As a counter point, I don't see any reason to make the DBA's life > harder. Sure it is just one step but it is a human step, prone to error > and taking more time than it should. Why not just make it easy? > Especially when the easy isn't sacrificing data integrity or quality of > product? I realize most feel we don't need to add a rename to this, but there are two more reasons _not_ to do this. First, there is the possibility of name collision with the new name so you would then require the user to use the option not to rename. Plus, if you renamed, the old index name would go away, and some people might think the index was removed and not realize it was renamed, or find it confusing it was renamed. -- 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-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Zdenek Kotala wrote:
> Alvaro Herrera wrote:
>> (Digging further, it seems like bufpage.h should also include transam.h
>> in order to get TransactionIdIsNormal ... I start to wonder how many
>> problems of this nature we have on our headers. Without having a way to
>> detect whether the defined macros are valid, it seems hard to check
>> programatically, however.)
>
> I attached script which should check it. In first step it runs C
> preprocessor on each header (postgres.h is included as well). The output
> from first step is processed again with preprocessor and define.h is
> included. Define.h contains "all" used macros in following format:
>
> #define SIGABRT "NOT_EXPANDED_SIGABRT"
>
> Main problem is how to generate define.h. I used following magic:
>
> grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1
> -d"("
>
> but it generates some noise as well. Maybe some Perl or AWK magic should be
> better.
So were you able to detect anything bogus with this technique?
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bloated heapam.h
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > Oops :-( I just noticed that I removed bufmgr.h from bufpage.h, which > > is a change you had objected to previously :-( > > http://archives.postgresql.org/pgsql-patches/2008-04/msg00149.php > > Hmm. I did notice that the patch seemed to need to add bufmgr.h > inclusions to an awful lot of files, but I'd forgotten the argument > about the bufpage.h macros needing it. Do you want to undo that > aspect of it? Done -- I put back bufmgr.h into bufpage.h. Surely there is a better structure for this, perhaps involving more header files, but I don't have time for that ATM. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Convert wal_sync_method to guc enum.
Magnus Hagander wrote: > Tom Lane wrote: > > [EMAIL PROTECTED] (Magnus Hagander) writes: > > > Convert wal_sync_method to guc enum. > > > > Buildfarm says you broke things for Windows. > > Yeah, working on that with Dave. First part was to unbreak the error > message so we can actually figure out what's broken :-( > I need to leave for a couple of hours, will look again when I get back. But so far, I'm quite surprised. Here's my reasoning, please poke holes in it :-) 1) Win32 always defines O_DSYNC (win32.h) 2) That means we should always define OPEN_DATASYNC_FLAG (xlogdefs.h, line 107) 3) That means that the error should not happen at all, because of xlog.c line 6358. Anybody who can kill this argument before I get back ;-) It's obviously flawed somewhere... //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Hmm. Is that really a good idea, compared to hard-wiring the checks >> into nodeSeqscan and friends? > My eyebrows went up when I read this too. Presumably, if it's hardwired > like you suggest then the planner can't take any account of the filter, > though. Do we want it to? Well, the planner could have hardwired knowledge about the existence of the hardwired filters --- if anything, that'd probably be easier than hacking it to have a similar level of knowledge about generic-looking function calls. But in reality, since we don't know at plan time which userid will execute the constructed plan, I'm not sure we could come up with useful estimates anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] constraint exclusion analysis caching
Stephen Frost wrote: > > And it's completely unnecessary. For example, I have set my majordomo > > preferences for the postgresql.org lists not to send me copies of emails > > where I am also in the To: or Cc: lines. After doing that I get no > > duplicates. > > This doesn't help at all, actually. As I pointed out previously, I > *want* the mail through the list, what I *don't* want is people sending > list mail directly to me. Wouldn't it make sense, then, to filter any email which is Cc'ed to a list, into that list's folder? Add to that a bit of duplicate removal (say, procmail's, or whatever you use) and you're set. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
Tom Lane wrote: KaiGai Kohei <[EMAIL PROTECTED]> writes: Tom Lane wrote: Yeah, I remember those. What needs to be looked at here is *why* the output is changing. For a patch that allegedly does not touch the planner, it's fairly disturbing that you don't get the same results. SE-PostgreSQL does not touch the planner, but it modifies given query to filter violated tuples for the current user. Hmm. Is that really a good idea, compared to hard-wiring the checks into nodeSeqscan and friends? I didn't look at the query-rewriting portion of the patch in any detail, but I'd tend not to trust such a technique very far: getting it right is going to be quite complex and probably bug prone. My eyebrows went up when I read this too. Presumably, if it's hardwired like you suggest then the planner can't take any account of the filter, though. Do we want it to? OTOH, I'm not happy about silently rewriting queries, either - it would make optimising queries a lot harder, I suspect. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
KaiGai Kohei <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Yeah, I remember those. What needs to be looked at here is *why* the
>> output is changing. For a patch that allegedly does not touch the
>> planner, it's fairly disturbing that you don't get the same results.
> SE-PostgreSQL does not touch the planner, but it modifies given query
> to filter violated tuples for the current user.
Hmm. Is that really a good idea, compared to hard-wiring the checks
into nodeSeqscan and friends? I didn't look at the query-rewriting
portion of the patch in any detail, but I'd tend not to trust such
a technique very far: getting it right is going to be quite complex
and probably bug prone.
>> Are you sure that the security_label type should not have an array type?
> Yes, security_label type should not have an array type.
You didn't provide one ounce of justification for making it not obey the
expected behavior, so I'm not accepting this position. It doesn't seem
to me to be all that unlikely that users would want to compute with
arrays of security labels. As an example:
select ... where security_label in ('foo', 'bar')
which will become an = ANY(ARRAY[]) construct under the hood.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] constraint exclusion analysis caching
Stephen Frost wrote: Andrew, * Andrew Dunstan ([EMAIL PROTECTED]) wrote: For example, because it put *my* address in the list for your message above, it caused my MUA quite correctly to add a To: line to myself, which I certainly didn't want to do. Honestly, I suspect thunderbird just doesn't know your addresses if it's adding your address back in. Adding your address isn't for you- it's for other people. The, completely reasonable, assumption is that if your address was included in a To or Cc that you're not on the list and stripping that out would mean you'd be left out. And it's completely unnecessary. For example, I have set my majordomo preferences for the postgresql.org lists not to send me copies of emails where I am also in the To: or Cc: lines. After doing that I get no duplicates. This doesn't help at all, actually. As I pointed out previously, I *want* the mail through the list, what I *don't* want is people sending list mail directly to me. And I don't casue anyone else to have to edit the addresses when they reply to my mail. Are you sure thunderbird recognizes the email address you use for posting as a local identity/account? Mutt has a specific 'alternates' configuration to let it know what addresses are local. If you want to ensure that you reply to a list, use an MUA that has a reply-to-list command - I see you use mutt, which has such a command IIRC. Indeed, and it's exactly what I use when replying to list mail. The issue isn't making sure that *I* reply to a list, it's asking other people to reply through the list rather than to me. a. I don't use Thunderbird. b. Of couse the MUA knows what my address is. c. Yours are pretty much the *only* settings of all the users of this list that cause me issues. Judging by your own words I am not alone in being thus inconvenienced (otherwise, why would "an amazing number" of people ask you about it?). If you don't care about that then there's nothing much I can do. Alvaro used to have a similar setup. When I complained he very kindly fixed it. d. Your "completely reasonable" assumption above is, of course, bogus. Most people when replying to a list reply to all adresses. Assuming that the non-list addresses are for people not on the list is nonsense. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] constraint exclusion analysis caching
Andrew, * Andrew Dunstan ([EMAIL PROTECTED]) wrote: > For example, because it put *my* address in the list for your message > above, it caused my MUA quite correctly to add a To: line to myself, > which I certainly didn't want to do. Honestly, I suspect thunderbird just doesn't know your addresses if it's adding your address back in. Adding your address isn't for you- it's for other people. The, completely reasonable, assumption is that if your address was included in a To or Cc that you're not on the list and stripping that out would mean you'd be left out. > And it's completely unnecessary. For example, I have set my majordomo > preferences for the postgresql.org lists not to send me copies of emails > where I am also in the To: or Cc: lines. After doing that I get no > duplicates. This doesn't help at all, actually. As I pointed out previously, I *want* the mail through the list, what I *don't* want is people sending list mail directly to me. > And I don't casue anyone else to have to edit the addresses when they > reply to my mail. Are you sure thunderbird recognizes the email address you use for posting as a local identity/account? Mutt has a specific 'alternates' configuration to let it know what addresses are local. > If you want to ensure that you reply to a list, use an MUA that has a > reply-to-list command - I see you use mutt, which has such a command > IIRC. Indeed, and it's exactly what I use when replying to list mail. The issue isn't making sure that *I* reply to a list, it's asking other people to reply through the list rather than to me. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Stack depth exceeded error
Hi, The code uses Asynchronous I/O for fetching certain tids. The code works fine if I use only one condition in where condition, but fails if I use multiple condition. -- Suresh Iyengar Gregory Stark <[EMAIL PROTECTED]> wrote: "Suresh" writes: > Hello, > > I have custom postgres code. What kind of code is this? The error below is typical if you create new threads in the server. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! - Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now.
Re: [HACKERS] Stack depth exceeded error
"Suresh" <[EMAIL PROTECTED]> writes: > Hello, > > I have custom postgres code. What kind of code is this? The error below is typical if you create new threads in the server. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Stack depth exceeded error
Hello, I have custom postgres code. I get the error below for the query "select l_orderkey as a from tpcd.orders, tpcd.lineitem where o_orderkey=l_orderkey and l_partkey<100 and l_linestatus='F';" ERROR: stack depth limit exceeded HINT: Increase the configuration parameter "max_stack_depth". However, the same code runs fine with one condition in where clause, but fails with the error above in case of multiple conditions. Whats the cause of this error ? I tried increasing the stack limit; but it doesnt help. -- Suresh Iyengar - Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now.
Re: [HACKERS] bloated heapam.h
Alvaro Herrera wrote:
Alvaro Herrera wrote:
Oops :-( I just noticed that I removed bufmgr.h from bufpage.h, which
is a change you had objected to previously :-(
However, it seems the right fix is to move BufferGetPageSize and
BufferGetPage from bufpage.h to bufmgr.h.
+1 It makes more sense.
(Digging further, it seems like bufpage.h should also include transam.h
in order to get TransactionIdIsNormal ... I start to wonder how many
problems of this nature we have on our headers. Without having a way to
detect whether the defined macros are valid, it seems hard to check
programatically, however.)
I attached script which should check it. In first step it runs C preprocessor on
each header (postgres.h is included as well). The output from first step is
processed again with preprocessor and define.h is included. Define.h contains
"all" used macros in following format:
#define SIGABRT "NOT_EXPANDED_SIGABRT"
Main problem is how to generate define.h. I used following magic:
grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1
-d"("
but it generates some noise as well. Maybe some Perl or AWK magic should be
better.
Zdenek
test.sh
Description: application/shellscript
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XIDs and big boxes again ...
"Hans-Juergen Schoenig" <[EMAIL PROTECTED]> writes: > i forgot to mention - i am on 8.1 here. > so, VACUUM is not so smart yet. So even if we added 64-bit xids it wouldn't be useful to you. You would have to update (at which point you get all the other improvements which make it less useful.) Or at the very least rebuild with the patch and dump and reload which is just as hard. > my changes are pretty much random I/O - so tuple header does not contribute to > a lot more I/O as i have to read entire blocks anway. > this is why i said - it is not that kind of an issue. TPCC experiments show that even on random access you get the same performance hit from bloat. I'm not entirely sure what the mechanism is unless it's simply the cache hit rate being hurt by the wasted memory. > and no, updating is not a 5 min task ... I do hope you mean 8.1.11 btw. Updating your binaries should be a 5 minute job and there are real bugs fixed in those releases. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
Tom Lane wrote:
> KaiGai Kohei <[EMAIL PROTECTED]> writes:
>> Some of the test fails contains minor differences from expected results,
>> like:
>
>> | SELECT '' AS "xxx", *
>> | FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a);
>> |xxx | a | b | c | d
>> | -+---+---+--+---
>> | - | 0 | | zero |
>> || 2 | 3 | two | 2
>> || 4 | 1 | four | 2
>> | + | 0 | | zero |
>> | (3 rows)
>
> Yeah, I remember those. What needs to be looked at here is *why* the
> output is changing. For a patch that allegedly does not touch the
> planner, it's fairly disturbing that you don't get the same results.
SE-PostgreSQL does not touch the planner, but it modifies given query
to filter violated tuples for the current user.
Thus, the above query is dealt as if the following query is inputed.
SELECT '' AS "xxx", *
FROM J1_TBL t1 (a, b, c) NATURAL JOIN J2_TBL t2 (d, a)
ON sepgsql_tuple_perms(t1.security_context, SEPGSQL_PERMS_SELECT, ...)
and sepgsql_tuple_perms(t2.security_context, SEPGSQL_PERMS_SELECT, ...);
sepgsql_tuple_perms() returns true, if the security policy allows user
to access a given tuple. It returns false, if not so.
The result of EXPLAIN shows it more clearly:
| kaigai=# EXPLAIN SELECT '' AS "xxx", * FROM J1_TBL t1 (a, b, c) NATURAL JOIN
J2_TBL t2 (d, a);
| QUERY PLAN
|
---
| Hash Join (cost=29023.54..82599.93 rows=1380 width=44)
|Hash Cond: (t2.a = t1.a)
|-> Seq Scan on j2_tbl t2 (cost=0.00..53526.05 rows=713 width=8)
| Filter: pg_catalog.sepgsql_tuple_perms(tableoid, security_context,
12288, t2.*)
|-> Hash (cost=29018.70..29018.70 rows=387 width=40)
| -> Seq Scan on j1_tbl t1 (cost=0.00..29018.70 rows=387 width=40)
|Filter: pg_catalog.sepgsql_tuple_perms(tableoid,
security_context, 12288, t1.*)
| (7 rows)
>> and, some of them are trivial ones, like:
>
>> | SELECT p1.oid, p1.typname
>> | FROM pg_type as p1
>> | WHERE p1.typtype in ('b','e') AND p1.typname NOT LIKE E'\\_%' AND NOT
>> EXISTS
>> | (SELECT 1 FROM pg_type as p2
>> |WHERE p2.typname = ('_' || p1.typname)::name AND
>> | p2.typelem = p1.oid and p1.typarray = p2.oid);
>> | - oid | typname
>> | --+-
>> | - 210 | smgr
>> | - 705 | unknown
>> | -(2 rows)
>> | + oid |typname
>> | +--+
>> | + 210 | smgr
>> | + 705 | unknown
>> | + 3403 | security_label
>> | +(3 rows)
>
> Are you sure that the security_label type should not have an array type?
> I do not offhand see a good argument for that. If it really shouldn't,
> we can change the expected output here, but you've got to make that
> case first.
Yes, security_label type should not have an array type.
It is defined with typelem=0 and typarray=0.
The purpose of this type is to represent the new system column of
security attribute ("security_context" column).
So, I think it is a case that a new expented output should be modified.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <[EMAIL PROTECTED]>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XIDs and big boxes again ...
Joshua D. Drake wrote: Hans-Juergen Schoenig wrote: regards, tom lane overhead is not an issue here - if i lose 10 or 15% i am totally fine as long as i can reduce vacuum overhead to an absolute minimum. overhead will vary with row sizes anyway - this is not the point. I am not buying this argument. If you have a 5TB database, I am going to assume you put it on enterprise class hardware. Enterprise class hardware can handle the I/O required to appropriately run vacuum. We have a customer that is constantly running 5 autovacuum workers on only 28 spindles. We are in the process of upgrading them to 50 spindles at which point I will likely try 10 autovacuum workers. i forgot to mention - i am on 8.1 here. so, VACUUM is not so smart yet. my changes are pretty much random I/O - so tuple header does not contribute to a lot more I/O as i have to read entire blocks anway. this is why i said - it is not that kind of an issue. and no, updating is not a 5 min task ... hans -- Cybertec Schönig & Schönig GmbH PostgreSQL Solutions and Support Gröhrmühlgasse 26, A-2700 Wiener Neustadt Tel: +43/1/205 10 35 / 340 www.postgresql-support.de, www.postgresql-support.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
