Re: [HACKERS] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Mon, Oct 20, 2014 at 3:26 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 10:37 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao masao.fu...@gmail.com wrote: In this case, the patch seems to make the restartpoint recycle even WAL files which have .ready files and will have to be archived later. Thought? The real problem currently is that it is possible to have a segment file not marked as .done during recovery when stream connection is abruptly cut when this segment is switched, marking it as .ready in archive_status and simply letting this segment in pg_xlog because it will neither be recycled nor removed. I have not been able to look much at this code these days, so I am not sure how invasive it would be in back-branches, but perhaps we should try to improve code such as when a segment file is switched and connection to the is cut, we guarantee that this file is completed and marked as .done. I have spent more time on that, with a bit more of underground work... First, the problem can be reproduced most of the time by running this simple command: psql -c 'select pg_switch_xlog()'; pg_ctl restart -m immediate What about fixing this problem directly? That is, we can make walreceiver check whether the end of last received WAL data is the end of current WAL file or not, and then close the WAL file and create .done file if the test is true. This is not a perfect solution. If the standby crashes during very short interval (i.e., after closing the WAL file and before creating .done file), the problem would happen again. But it can really rarely happen, so I don't think that it's worth fixing the corner case at least in back-branches. Of course, we can find out the perfect solution for the master, though. Regards, -- Fujii Masao -- 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] [PATCH] add ssl_protocols configuration option
On Wed, Oct 22, 2014 at 09:36:59PM +0200, Dag-Erling Smørgrav wrote: Martijn van Oosterhout klep...@svana.org writes: Dag-Erling Smørgrav d...@des.no writes: If I understand correctly, imaps has been shown to be vulnerable as well, so I wouldn't be so sure. Reference? Sorry, no reference. I was told that Thunderbird was vulnerable to POODLE when talking imaps. Ugh, found it. It does the same connection fallback stuff as firefox. https://securityblog.redhat.com/2014/10/20/can-ssl-3-0-be-fixed-an-analysis-of-the-poodle-attack/ Since you can already specify the cipher list, couldn't you just add -SSLv3 to the cipher list and be done? I didn't want to change the existing behavior; all I wanted was to give users a way to do so if they wish. I think we should just disable SSL3.0 altogether. The only way this could cause problems is if people are using PostgreSQL with an OpenSSL library from last century. As for client libraries, even Windows XP supports TLS1.0. Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] Reducing lock strength of adding foreign keys
On 10/22/2014 04:13 PM, Tom Lane wrote: Andreas Karlsson andr...@proxel.se writes: I have attached a proof of concept patch which reduces the lock strength to ShareLock. You're kidding, right? ShareLock isn't even self-exclusive. Why would it have to be self-exclusive? As far as I know we only need to ensure that nobody changes the rows while we add the trigger. Adding multiple triggers concurrently should not pose a problem unless I am missing something (which I probably am). Andreas -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On 10/23/2014 08:59 AM, Fujii Masao wrote: On Mon, Oct 20, 2014 at 3:26 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 10:37 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao masao.fu...@gmail.com wrote: In this case, the patch seems to make the restartpoint recycle even WAL files which have .ready files and will have to be archived later. Thought? The real problem currently is that it is possible to have a segment file not marked as .done during recovery when stream connection is abruptly cut when this segment is switched, marking it as .ready in archive_status and simply letting this segment in pg_xlog because it will neither be recycled nor removed. I have not been able to look much at this code these days, so I am not sure how invasive it would be in back-branches, but perhaps we should try to improve code such as when a segment file is switched and connection to the is cut, we guarantee that this file is completed and marked as .done. I have spent more time on that, with a bit more of underground work... First, the problem can be reproduced most of the time by running this simple command: psql -c 'select pg_switch_xlog()'; pg_ctl restart -m immediate What about fixing this problem directly? That is, we can make walreceiver check whether the end of last received WAL data is the end of current WAL file or not, and then close the WAL file and create .done file if the test is true. This is not a perfect solution. If the standby crashes during very short interval (i.e., after closing the WAL file and before creating .done file), the problem would happen again. But it can really rarely happen, so I don't think that it's worth fixing the corner case at least in back-branches. Of course, we can find out the perfect solution for the master, though. Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. At least for master, we should consider changing the way the archiving works so that we only archive WAL that was generated in the same server. I.e. we should never try to archive WAL files belonging to another timeline. I just remembered that we discussed a different problem related to this some time ago, at http://www.postgresql.org/message-id/20131212.110002.204892575.horiguchi.kyot...@lab.ntt.co.jp. The conclusion of that was that at promotion, we should not archive the last, partial, segment from the old timeline. Also, we could make the archive recovery more precise in what WAL files it tries to restore. For example, if we're doing recovery with target timeline 4, but the timeline history says that segment AB came from timeline 3 (i.e. the switch to timeline 4 happened within a later segment), we should not try to restore segment AB from timeline 4. It shouldn't exist, so trying to restore it is a waste of cycles, but more importantly, if there's some sort of confusion with the timelines, and the file exists anyway, we should not restore it. Also, if the timeline history says e.g. that we switched from TLI 3 to 4 at WAL position 0/AD123456, and we restore segment AD from timeline 3, we should stop recovering that segment at position 0/AD123456 and try to find the AD segment from timeline 4. We currently only do that when streaming, but we should also do that when restoring from archive. In summary, let's do something small for back-branches, like what you suggested. But for master, let's do bigger changes to the timeline handling. Would you like to write a patch for the back-branches, if I start working on master? - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Incorrect comment in tablecmds.c
I don't think that the lock level mentioned in the following comment in MergeAttributes() in tablecmds.c is right, since that that function has opened the relation with ShareUpdateExclusiveLock, not with AccessShareLock. Patch attached. 1749 /* 1750 * Close the parent rel, but keep our AccessShareLock on it until xact 1751 * commit. That will prevent someone else from deleting or ALTERing 1752 * the parent before the child is committed. 1753 */ 1754 heap_close(relation, NoLock); Thanks, Best regards, Etsuro Fujita *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *** *** 1747,1755 MergeAttributes(List *schema, List *supers, char relpersistence, pfree(newattno); /* ! * Close the parent rel, but keep our AccessShareLock on it until xact ! * commit. That will prevent someone else from deleting or ALTERing ! * the parent before the child is committed. */ heap_close(relation, NoLock); } --- 1747,1755 pfree(newattno); /* ! * Close the parent rel, but keep our ShareUpdateExclusiveLock on it ! * until xact commit. That will prevent someone else from deleting or ! * ALTERing the parent before the child is committed. */ heap_close(relation, NoLock); } -- 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] idea: allow AS label inside ROW constructor
Hi here is a prototype postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) postgres=# select row_to_json(row(10, row(30, 20))); row_to_json -- {f1:10,f2:{f1:30,f2:20}} (1 row) Regards Pavel 2014-10-22 19:09 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2014-10-22 18:35 GMT+02:00 Merlin Moncure mmonc...@gmail.com: On Wed, Oct 22, 2014 at 11:21 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hi with new functions row_to_json(b), there is more often usage of ROW constructor. Using names in fields is relative difficult. Because ROW has special clause in parser, I am thinking so we can enable labeling inside ROW constructor so instead currently supported: select row_to_json(r) from (select 10 as a, 20 as b) r; users can to write: select row_to_json(row(10 as a,20 as b)); labeling will be enabled only inside ROW constructor. I don't propose enable it everywhere. What do you think about it? It's a neat idea -- maybe a better alternative to what I was thinking here: http://postgresql.1045698.n5.nabble.com/Support-UPDATE-table-SET-tp5823073p5823944.html Some questions: *) What would the parser transformation resolve to row:ROW '(' expr_list ')' { $$ = $3; } | ROW '(' ')' { $$ = NIL; } | '(' expr_list ',' a_expr ')' { $$ = lappend($2, $4); } ; we can replace a expr_list by target_list. I know only so it doesn't enforce a problems with gramatic - bison doesn't raise any warning. *) Are we ok with SQL standard SQL standard doesn't think named attributes in row - so it is out of range ANSI. But it is not in conflict with standard. AS name is used more in SQL/MM, SQL/XML -- and function named parameters has different syntax parameter_name = value - I checked it against SQL99. *) Do you think this (or some similar variant) would work? select row_to_json(row(foo.*)) from foo; It looks like independent feature and can work too - it is more natural for user. merlin diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y new file mode 100644 index 0de9584..682506d *** a/src/backend/parser/gram.y --- b/src/backend/parser/gram.y *** static void processCASbits(int cas_bits, *** 174,179 --- 174,180 bool *deferrable, bool *initdeferred, bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner); static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); + static List *extractArgs(List *target_list); %} *** static Node *makeRecursiveViewSelect(cha *** 431,437 %type list ExclusionConstraintList ExclusionConstraintElem %type list func_arg_list %type node func_arg_expr ! %type list row type_list array_expr_list %type node case_expr case_arg when_clause case_default %type list when_clause_list %type ival sub_type --- 432,439 %type list ExclusionConstraintList ExclusionConstraintElem %type list func_arg_list %type node func_arg_expr ! %type list row type_list array_expr_list row_field_list ! %type target row_field_el %type node case_expr case_arg when_clause case_default %type list when_clause_list %type ival sub_type *** a_expr: c_expr { $$ = $1; } *** 11162,11179 } | row OVERLAPS row { ! if (list_length($1) != 2) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg(wrong number of parameters on left side of OVERLAPS expression), parser_errposition(@1))); ! if (list_length($3) != 2) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg(wrong number of parameters on right side of OVERLAPS expression), parser_errposition(@3))); $$ = (Node *) makeFuncCall(SystemFuncName(overlaps), ! list_concat($1, $3), @2); } | a_expr IS TRUE_P %prec IS --- 11164,11184 } | row OVERLAPS row { ! List *l1 = extractArgs($1); ! List *l2 = extractArgs($3); ! ! if (list_length(l1) != 2) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg(wrong number of parameters on left side of OVERLAPS expression), parser_errposition(@1))); ! if (list_length(l2) != 2) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg(wrong number of parameters on right side of OVERLAPS expression), parser_errposition(@3))); $$ = (Node *) makeFuncCall(SystemFuncName(overlaps), ! list_concat(l1, l2), @2); } | a_expr IS TRUE_P %prec IS *** frame_bound: *** 12302,12310
Re: [HACKERS] [Windows,PATCH] Use faster, higher precision timer API
On Thu, Oct 23, 2014 at 1:27 AM, Craig Ringer cr...@2ndquadrant.com wrote: Here's an updated patch addressing David's points. I haven't had a chance to test it yet, on win2k8 or win2k12 due to pgconf.eu . Hi Craig, thanks for the fast turnaround. I've just had a look over the patch again: + DWORD errcode = GetLastError(); + Assert(errcode == ERROR_PROC_NOT_FOUND); I'm not a big fan of this. It seems quite strange to be using Assert in this way. I'd rather see any error just silently fall back on GetSystemTimeAsFileTime() instead of this. I had originally assumed that you stuck the debug log in there so that people would have some sort of way of finding out if their system is using GetSystemTimePreciseAsFileTime() or GetSystemTimeAsFileTime(), the assert's not really doing this. I'd vote for, either removing this assert or sticking some elog DEBUG1 sometime after the logger has started. Perhaps just a test like: if (pg_get_system_time == GetSystemTimeAsFileTime) elog(DEBUG1, gettimeofday is using GetSystemTimeAsFileTime()); else elog(DEBUG1, gettimeofday is using GetSystemTimePreciseAsFileTime()); But perhaps it's not worth the trouble. Also if you decide to get rid of the elog, probably should also remove the include of elog.h that you've added. Or if you disagree with my comment on the Assert() you'll need to include the proper header for that. The compiler is currently giving a warning about that. Regards David Rowley
Re: [HACKERS] [PATCH] add ssl_protocols configuration option
Dag-Erling Smørgrav wrote: Martijn van Oosterhout klep...@svana.org writes: Dag-Erling Smørgrav d...@des.no writes: Martijn van Oosterhout klep...@svana.org writes: Since you can already specify the cipher list, couldn't you just add -SSLv3 to the cipher list and be done? I didn't want to change the existing behavior; all I wanted was to give users a way to do so if they wish. I think we should just disable SSL3.0 altogether. The only way this could cause problems is if people are using PostgreSQL with an OpenSSL library from last century. As for client libraries, even Windows XP supports TLS1.0. As far as I'm concerned (i.e. as far as FreeBSD and the University of Oslo are concerned), I couldn't care less about anything older than 0.9.8, which is what FreeBSD 8 and RHEL5 have, but I don't feel comfortable making that decision for other people. On the gripping hand, no currently supported version of libpq uses anything older than TLS; 9.0 through 9.3 use TLS 1.0 only while 9.4 uses TLS 1.0 or higher. OpenSSL just announced a week or two ago that they're abandoning support for 0.9.8 by the end of next year[1], which means its replacements have been around for a really long time. I think it's fine to drop 0.9.7 support --- we already dropped support for 0.9.6 with the renegotiation rework[2] in the 9.4 timeframe. OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of security issues, so anyone *is* using SSL but not at least the 0.9.8 branch, they are in trouble. [1] http://openssl.6102.n7.nabble.com/OpenSSL-0-9-8-End-Of-Life-Announcement-td54155.html [2] http://www.postgresql.org/message-id/20130712203252.gh29...@eldon.alvh.no-ip.org -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? There is still a small window between XLogWalRcvFlush and XLogArchiveForceDone in XLogWalRcvWrite if the standby crashes exactly between them. -- Michael diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index c2d4ed3..b367cb7 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -136,7 +136,8 @@ static void WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last); static void WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI); static void WalRcvDie(int code, Datum arg); static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len); -static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr); +static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr, + XLogRecPtr walEnd); static void XLogWalRcvFlush(bool dying); static void XLogWalRcvSendReply(bool force, bool requestReply); static void XLogWalRcvSendHSFeedback(bool immed); @@ -831,7 +832,7 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len) buf += hdrlen; len -= hdrlen; -XLogWalRcvWrite(buf, len, dataStart); +XLogWalRcvWrite(buf, len, dataStart, walEnd); break; } case 'k':/* Keepalive */ @@ -869,7 +870,7 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len) * Write XLOG data to disk. */ static void -XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) +XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr, XLogRecPtr walEnd) { int startoff; int byteswritten; @@ -878,7 +879,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) { int segbytes; - if (recvFile 0 || !XLByteInSeg(recptr, recvSegNo)) + if (recvFile 0 || !XLByteInSeg(walEnd, recvSegNo)) { bool use_existent; -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On 10/23/2014 01:25 PM, Michael Paquier wrote: On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. There is still a small window between XLogWalRcvFlush and XLogArchiveForceDone in XLogWalRcvWrite if the standby crashes exactly between them. Yeah. I think we can live with that. - Heikki diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index c2d4ed3..ca24acc 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -138,6 +138,7 @@ static void WalRcvDie(int code, Datum arg); static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len); static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr); static void XLogWalRcvFlush(bool dying); +static void XLogWalRcvFileClose(void); static void XLogWalRcvSendReply(bool force, bool requestReply); static void XLogWalRcvSendHSFeedback(bool immed); static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime); @@ -887,30 +888,7 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) * would otherwise have to reopen this file to fsync it later */ if (recvFile = 0) - { -char xlogfname[MAXFNAMELEN]; - -XLogWalRcvFlush(false); - -/* - * XLOG segment files will be re-read by recovery in startup - * process soon, so we don't advise the OS to release cache - * pages associated with the file like XLogFileClose() does. - */ -if (close(recvFile) != 0) - ereport(PANIC, - (errcode_for_file_access(), - errmsg(could not close log segment %s: %m, - XLogFileNameP(recvFileTLI, recvSegNo; - -/* - * Create .done file forcibly to prevent the streamed segment - * from being archived later. - */ -XLogFileName(xlogfname, recvFileTLI, recvSegNo); -XLogArchiveForceDone(xlogfname); - } - recvFile = -1; +XLogWalRcvFileClose(); /* Create/use new log file */ XLByteToSeg(recptr, recvSegNo); @@ -966,6 +944,10 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr) LogstreamResult.Write = recptr; } + + /* If we just finished writing to the current file, fsync and close it */ + if (recvFile = 0 recvOff == XLOG_SEG_SIZE) + XLogWalRcvFileClose(); } /* @@ -1022,6 +1004,38 @@ XLogWalRcvFlush(bool dying) } /* + * Close the currently open WAL file. + */ +static void +XLogWalRcvFileClose(void) +{ + char xlogfname[MAXFNAMELEN]; + + Assert(recvFile = 0); + + XLogWalRcvFlush(false); + + /* + * XLOG segment files will be re-read by recovery in startup process soon, + * so we don't advise the OS to release cache pages associated with the + * file like XLogFileClose() does. + */ + if (close(recvFile) != 0) + ereport(PANIC, +(errcode_for_file_access(), + errmsg(could not close log segment %s: %m, + XLogFileNameP(recvFileTLI, recvSegNo; + recvFile = -1; + + /* + * Create .done file forcibly to prevent the streamed segment from being + * archived later. + */ + XLogFileName(xlogfname, recvFileTLI, recvSegNo); + XLogArchiveForceDone(xlogfname); +} + +/* * Send reply message to primary, indicating our current XLOG positions, oldest * xmin and the current time. * -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 5:09 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: On Mon, Oct 20, 2014 at 3:26 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 10:37 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Oct 17, 2014 at 9:23 PM, Fujii Masao masao.fu...@gmail.com wrote: In this case, the patch seems to make the restartpoint recycle even WAL files which have .ready files and will have to be archived later. Thought? The real problem currently is that it is possible to have a segment file not marked as .done during recovery when stream connection is abruptly cut when this segment is switched, marking it as .ready in archive_status and simply letting this segment in pg_xlog because it will neither be recycled nor removed. I have not been able to look much at this code these days, so I am not sure how invasive it would be in back-branches, but perhaps we should try to improve code such as when a segment file is switched and connection to the is cut, we guarantee that this file is completed and marked as .done. I have spent more time on that, with a bit more of underground work... First, the problem can be reproduced most of the time by running this simple command: psql -c 'select pg_switch_xlog()'; pg_ctl restart -m immediate What about fixing this problem directly? That is, we can make walreceiver check whether the end of last received WAL data is the end of current WAL file or not, and then close the WAL file and create .done file if the test is true. This is not a perfect solution. If the standby crashes during very short interval (i.e., after closing the WAL file and before creating .done file), the problem would happen again. But it can really rarely happen, so I don't think that it's worth fixing the corner case at least in back-branches. Of course, we can find out the perfect solution for the master, though. Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. +1 At least for master, we should consider changing the way the archiving works so that we only archive WAL that was generated in the same server. I.e. we should never try to archive WAL files belonging to another timeline. I just remembered that we discussed a different problem related to this some time ago, at http://www.postgresql.org/message-id/20131212.110002.204892575.horiguchi.kyot...@lab.ntt.co.jp. The conclusion of that was that at promotion, we should not archive the last, partial, segment from the old timeline. So, the last, partial, segment of the old timeline is never archived? If yes, I'm afraid that the PITR to the old timeline cannot replay the last segment. No? Or you're thinking to change the code so that the segment of new timeline is replayed in that case? In summary, let's do something small for back-branches, like what you suggested. But for master, let's do bigger changes to the timeline handling. Yep. Regards, -- Fujii Masao -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 8:45 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 01:25 PM, Michael Paquier wrote: On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. Looks good to me. WalReceiverMain has almost the same code as what XLogWalRcvFileClose does. So we can refactor that. There is still a small window between XLogWalRcvFlush and XLogArchiveForceDone in XLogWalRcvWrite if the standby crashes exactly between them. Yeah. I think we can live with that. Yes. Regards, -- Fujii Masao -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 9:23 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Oct 23, 2014 at 8:45 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 01:25 PM, Michael Paquier wrote: On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. Looks good to me. WalReceiverMain has almost the same code as what XLogWalRcvFileClose does. So we can refactor that. While looking at the code of WAL archiving and recovery, I found another small issue. The standby always creates .ready file for the timeline history file even when WAL archiving is not enabled. Since WAL archiving is off, that .ready file will remain infinitely. Probably this is almost harmless but confusing, so I'd like to fix that. Patch attached. Thought? Regards, -- Fujii Masao *** a/src/backend/access/transam/timeline.c --- b/src/backend/access/transam/timeline.c *** *** 36,41 --- 36,42 #include unistd.h #include access/timeline.h + #include access/xlog.h #include access/xlog_internal.h #include access/xlogdefs.h #include storage/fd.h *** *** 437,444 writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, #endif /* The history file can be archived immediately. */ ! TLHistoryFileName(histfname, newTLI); ! XLogArchiveNotify(histfname); } /* --- 438,448 #endif /* The history file can be archived immediately. */ ! if (XLogArchivingActive()) ! { ! TLHistoryFileName(histfname, newTLI); ! XLogArchiveNotify(histfname); ! } } /* -- 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] [Windows,PATCH] Use faster, higher precision timer API
On 10/23/2014 11:41 AM, David Rowley wrote: I'm not a big fan of this. It seems quite strange to be using Assert in this way. I'd rather see any error just silently fall back on GetSystemTimeAsFileTime() instead of this. That's fair. I'd like some visibility into it, but I don't think it's vital. I had originally assumed that you stuck the debug log in there so that people would have some sort of way of finding out if their system is using GetSystemTimePreciseAsFileTime() or GetSystemTimeAsFileTime() No, that was never the goal. The previous code using elog only logged if the system couldn't load GetSystemTimePreciseAsFileTime() because of an error other than the expected one when the symbol can't be found. In other words, if you're on win2k8 nothing happens, it just silently uses GetSystemTimeAsFileTime(). We expect failure to load the proc address, that's ok, we just assume it's an older windows. If the load fails for some _other_ reason though, that's a weird issue that's worth complaining about, but we don't know anything more than something isn't right here. if (pg_get_system_time == GetSystemTimeAsFileTime) elog(DEBUG1, gettimeofday is using GetSystemTimeAsFileTime()); else elog(DEBUG1, gettimeofday is using GetSystemTimePreciseAsFileTime()); But perhaps it's not worth the trouble. That's probably not really worth it; it's completey different to what the prior code was doing anyway. Also if you decide to get rid of the elog, probably should also remove the include of elog.h that you've added. Rather. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 2:34 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Oct 23, 2014 at 9:23 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Oct 23, 2014 at 8:45 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 01:25 PM, Michael Paquier wrote: On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. Looks good to me. WalReceiverMain has almost the same code as what XLogWalRcvFileClose does. So we can refactor that. While looking at the code of WAL archiving and recovery, I found another small issue. The standby always creates .ready file for the timeline history file even when WAL archiving is not enabled. Since WAL archiving is off, that .ready file will remain infinitely. Probably this is almost harmless but confusing, so I'd like to fix that. Patch attached. Thought? Good catch once again. We could as well put the check of XLogArchivingActive directly in XLogArchiveNotify... -- Michael
Re: [HACKERS] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 1:45 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 01:25 PM, Michael Paquier wrote: On Thu, Oct 23, 2014 at 10:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 10/23/2014 08:59 AM, Fujii Masao wrote: Sounds reasonable, for back-branches. Although I'm still worried we might miss some corner-case unless we go with a more wholesale solution. Don't really want to be the intruder here, but isn't that the simple patch attached? That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. There is still a small window between XLogWalRcvFlush and XLogArchiveForceDone in XLogWalRcvWrite if the standby crashes exactly between them. Yeah. I think we can live with that. Just tested this patch with the combo pg_switch_xlog() + stop/immediate and the apparition of the .ready files is not fixed. Regards, -- Michael
Re: [HACKERS] idea: allow AS label inside ROW constructor
On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hi here is a prototype postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) postgres=# select row_to_json(row(10, row(30, 20))); row_to_json -- {f1:10,f2:{f1:30,f2:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. 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] idea: allow AS label inside ROW constructor
On Oct23, 2014, at 15:39 , Andrew Dunstan and...@dunslane.net wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule pavel.steh...@gmail.com wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Typo fixes for pg_recvlogical documentation
Hi all, pg_recvlogical is missing some = signs for a couple of option names where double-dash is used, like this one: --username user should be that: --username=user Attached is a patch correcting that. Regards, -- Michael diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml index f30b484..eaa5b5d 100644 --- a/doc/src/sgml/ref/pg_recvlogical.sgml +++ b/doc/src/sgml/ref/pg_recvlogical.sgml @@ -105,7 +105,7 @@ PostgreSQL documentation varlistentry termoption-U replaceableuser/replaceable/option/term - termoption--username replaceableuser/replaceable/option/term + termoption--username=replaceableuser/replaceable/option/term listitem para Username to connect as. Must have a suitable literalpg_hba.conf/literal @@ -117,7 +117,7 @@ PostgreSQL documentation varlistentry termoption-d replaceabledatabase/replaceable/option/term - termoption--dbname replaceabledatabase/replaceable/option/term + termoption--dbname=replaceabledatabase/replaceable/option/term listitem para The database to connect to in literalreplication/literal mode; see @@ -130,7 +130,7 @@ PostgreSQL documentation varlistentry termoption-h replaceablehostname-or-ip/replaceable/option/term - termoption--host replaceablehostname-or-ip/replaceable/option/term + termoption--host=replaceablehostname-or-ip/replaceable/option/term listitem para Host or socket to connect @@ -143,7 +143,7 @@ PostgreSQL documentation varlistentry termoption-p replaceableport/replaceable/option/term - termoption--port replaceableport/replaceable/option/term + termoption--port=replaceableport/replaceable/option/term listitem para Port number to connect to. See -- 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] BUG: *FF WALs under 9.2 (WAS: .ready files appearing on slaves)
On Thu, Oct 23, 2014 at 10:16 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Oct 23, 2014 at 1:45 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: That's not right. Should check *after* the write if the segment was completed, and close it if so. Like the attached. Just tested this patch with the combo pg_switch_xlog() + stop/immediate and the apparition of the .ready files is not fixed. Btw, I think that we should pass walEnd to XLogWalRcvWrite and add an additional check based on that after the write() loop calls to enforce the segment to be switched to .done. -- Michael
Re: [HACKERS] idea: allow AS label inside ROW constructor
On 10/23/2014 09:57 AM, Florian Pflug wrote: On Oct23, 2014, at 15:39 , Andrew Dunstan and...@dunslane.net wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule pavel.steh...@gmail.com wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. Well, I think we need to see those other use cases. The only use case I recall seeing involves the already provided case of constructing JSON. 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
[HACKERS] KEY UPDATE / UPDATE / NO KEY UPDATE distinction vs. README.tuplock
Hi, It seems that README.tuplock never got updated when the KEY SHARE patch's lock level were changed from being KEY UPDATE / UPDATE / SHARE / KEY SHARE to UPDATE / NO KEY UPDATE / SHARE / KEY SHARE. Thus, as it stands, that file implies that SELECT FOR UPDATE obtains a weaker lock than an actual UPDATE might take (if that update modifies key columns) -- according to README.tuplock, the former doesn't conflict with KEY SHARE while the actual UPDATE would. But this isn't actually the case in the committed version of the patch - one now needs to explicitly request that weaker lock level with SELECT FOR NO KEY UPDATE. The attached patch updated README.tuplock accordingly. best regards, Florian Pflug README.tuplock.patch Description: Binary data -- 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] pg_receivexlog --status-interval add fsync feedback
On Wed, Oct 22, 2014 at 10:47 PM, Simon Riggs si...@2ndquadrant.com wrote: On 22 October 2014 14:26, Heikki Linnakangas hlinnakan...@vmware.com wrote: We seem to be going in circles. You suggested having two options, --feedback, and --fsync, which is almost exactly what Furuya posted originally. I objected to that, because I think that user interface is too complicated. Instead, I suggested having just a single option called --synchronous I'm OK with this. But the option name synchronous seems confusing because pg_receivexlog with sync option can work as async standby. So the better name is needed. or even better, have no option at all and have the server tell the client if it's participating in synchronous replication, and have pg_receivexlog automatically fsync when it is, and not otherwise [1]. That way you don't need to expose any new options to the user. What did you think of that idea? Sorry, if we're going in circles. Yes, I like the idea. The master setting of synchronous_standby_names defines which standbys/rep clients will have their feedback used to release waiting COMMITs. That uses application_name, which is set at the replication client connection. (That has a default value, but lets assume the user sets this). So when a replication client connects, the WALSender knows its priority, as defined in sync_standby_names. I guess we can have WALSender send a protocol message to the client every time the sync priority changes (as a result of a changed value of sync_standby_names). Which is the function SyncRepInitConfig() Sorry, I'm going around in the circle. But I'd like to say again, I don't think this is good idea. It prevents asynchronous pg_receivexlog from fsyncing WAL data and sending feedbacks more frequently at all. They are useful, for example, when we want to monitor the write location of asynchronous pg_receivexlog in almost realtime. But if we adopt the idea, since feedback cannot be sent soon in async mode, pg_stat_replication always returns the not-up-to-date location. Regards, -- Fujii Masao -- 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] Simplify calls of pg_class_aclcheck when multiple modes are used
On Thu, Oct 23, 2014 at 10:45 AM, Peter Eisentraut pete...@gmx.net wrote: Committed your patch and tests. Thanks! -- Michael
Re: [HACKERS] pg_receivexlog --status-interval add fsync feedback
On 23 October 2014 15:39, Fujii Masao masao.fu...@gmail.com wrote: Sorry, I'm going around in the circle. But I'd like to say again, I don't think this is good idea. It prevents asynchronous pg_receivexlog from fsyncing WAL data and sending feedbacks more frequently at all. They are useful, for example, when we want to monitor the write location of asynchronous pg_receivexlog in almost realtime. But if we adopt the idea, since feedback cannot be sent soon in async mode, pg_stat_replication always returns the not-up-to-date location. Why not send a message every 10 seconds when its not sync rep? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] pg_receivexlog --status-interval add fsync feedback
On 10/23/2014 06:01 PM, Simon Riggs wrote: On 23 October 2014 15:39, Fujii Masao masao.fu...@gmail.com wrote: Sorry, I'm going around in the circle. But I'd like to say again, I don't think this is good idea. It prevents asynchronous pg_receivexlog from fsyncing WAL data and sending feedbacks more frequently at all. They are useful, for example, when we want to monitor the write location of asynchronous pg_receivexlog in almost realtime. But if we adopt the idea, since feedback cannot be sent soon in async mode, pg_stat_replication always returns the not-up-to-date location. Why not send a message every 10 seconds when its not sync rep? Or even after every write(). It's a tiny amount of network traffic anyway. - Heikki -- 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] delta relations in AFTER triggers
On Wed, Oct 22, 2014 at 5:29 PM, Tom Lane t...@sss.pgh.pa.us wrote: I was thinking that the hook would return a RelationParam. When parse analysis sees the returned RelationParam, it adds an entry for that to the range table, and creates the RangeTblRef for it. The way you describe it works too, but manipulating the range table directly in the hook seems a bit too low-level. The problem with that idea is that then the API for the hook has to cover every possible sort of RTE that hooks might wish to create; I see no reason to restrict them to creating just one kind. I agree that the hook should avoid *physically* manipulating the rangetable, but it seems reasonable to expect that it can call one of the addRangeTableEntryXXX functions provided by parse_relation.c, and then return a RangeTblEntry* gotten that way. So hooks would have an API more or less equivalent to, eg, transformRangeFunction(). Right, that reasoning makes sense to me. Unlike regular parameters, where the existence of the parameter is known at parse time but the value isn't available until bind time, we would be creating a RelationParam node and then, literally immediately, turning it into a range-table entry. That seems like unnecessary complexity, and it's also something we can invent later if a more compelling use case emerges. So what I'm imagining now is: 1. During parse analysis, p_tableref_hook gets control and calls addRangeTableEntryForTuplestore(), creating an RTE of type RTE_TUPLESTORE. The RTE stores an integer parameter-index. 2. Path generation doesn't need to do anything very exciting; it just generates a Path node of type T_TuplestoreScan. The RTE is still available, so the path itself doesn't need to know which tuplestore we're referencing, because that information is present in the RTE. 3. At plan generation time, we look up the RTE for the path and extract the parameter index, which is what gets stored in the TuplestoreScan node. 4. At executor initialization time, we use the parameter index in the TuplestoreScan to index into the EState's es_param_list_info and retrieve the tuplestore. This means that Kevin's notion of a Tsrcache goes away completely, which means a lot of the function-signature changes in his last version of the patch can be reverted. The EState doesn't need a es_tsrcache either. The mapping from name (OLD/NEW) to parameter index happens inside the p_paramref_hook and after that we use integer indices throughout. All that sees good. One thing that's not too clear to me is how we're imagining that the TuplestoreScan will get it's tupledesc. Right now the Tsrcache stores essentially (tupledesc, tuplestore), but I understood the suggestions above to imply that the ParamListInfo should point only to the tuplestore, not to the tupledesc. I *think* the information we need to reconstruct the TupleDesc is mostly present in the RTE; Kevin reused the ctecoltypes, ctecoltypmods, and ctecolcollations fields to store that information, which (a) probably requires some thought about renaming those fields but (b) seems like it ought to be enough to construct a viable TupleDesc. It seems that for CTEs, we somehow engineer things so that the RecursiveUnion's target-list is such that we can apply ExecAssignResultTypeFromTL() to it and get the tupledesc that matches its Tuplestorestate, but I'm kinda unclear about what makes that work and whether we can use a similar trick here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] idea: allow AS label inside ROW constructor
Andrew Dunstan wrote On 10/23/2014 09:57 AM, Florian Pflug wrote: On Oct23, 2014, at 15:39 , Andrew Dunstan lt; andrew@ gt; wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule lt; pavel.stehule@ gt; wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. Well, I think we need to see those other use cases. The only use case I recall seeing involves the already provided case of constructing JSON. Even if it simply allows CTE and sibqueries to form anonymous record types which can then be re-expanded in the outer layer for table-like final output this feature would be useful. When working with wide tables and using multiple aggregates and joins being able to avoid specifying individual columns repeatedly is quite desirable. It would be especially nice to not have to use as though, if the source fields are already so named. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/idea-allow-AS-label-inside-ROW-constructor-tp5823954p5824045.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Inefficient barriers on solaris with sun cc
06.10.2014, 17:42, Andres Freund kirjoitti: I think we can pretty much apply Oskari's patch after replacing acquire/release with read/write intrinsics. Attached a patch rebased to current master using read write barriers. / Oskari From a994c0f4feff74050ade183ec26d726397fa14a7 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa o...@ohmu.fi Date: Thu, 23 Oct 2014 18:36:31 +0300 Subject: [PATCH] =?UTF-8?q?=C2=A0atomics:=20add=20compiler=20and=20memory?= =?UTF-8?q?=20barriers=20for=20solaris=20studio?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- configure | 2 +- configure.in | 2 +- src/include/pg_config.h.in| 3 +++ src/include/port/atomics/generic-sunpro.h | 17 + 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/configure b/configure index b403a04..1248b06 100755 --- a/configure +++ b/configure @@ -9164,7 +9164,7 @@ fi done -for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h +for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h do : as_ac_Header=`$as_echo ac_cv_header_$ac_header | $as_tr_sh` ac_fn_c_check_header_mongrel $LINENO $ac_header $as_ac_Header $ac_includes_default diff --git a/configure.in b/configure.in index df86882..0a3725f 100644 --- a/configure.in +++ b/configure.in @@ -1016,7 +1016,7 @@ AC_SUBST(UUID_LIBS) ## dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES -AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) +AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) # On BSD, test for net/if.h will fail unless sys/socket.h # is included first. diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index ddcf4b0..3e78d65 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -340,6 +340,9 @@ /* Define to 1 if `long long int' works and is 64 bits. */ #undef HAVE_LONG_LONG_INT_64 +/* Define to 1 if you have the mbarrier.h header file. */ +#undef HAVE_MBARRIER_H + /* Define to 1 if you have the `mbstowcs_l' function. */ #undef HAVE_MBSTOWCS_L diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h index 77d3ebe..cd84107 100644 --- a/src/include/port/atomics/generic-sunpro.h +++ b/src/include/port/atomics/generic-sunpro.h @@ -19,6 +19,23 @@ #if defined(HAVE_ATOMICS) +#ifdef HAVE_MBARRIER_H +#include mbarrier.h + +#define pg_compiler_barrier_impl() __compiler_barrier() + +#ifndef pg_memory_barrier_impl +# define pg_memory_barrier_impl() __machine_rw_barrier() +#endif +#ifndef pg_read_barrier_impl +# define pg_read_barrier_impl() __machine_r_barrier() +#endif +#ifndef pg_write_barrier_impl +# define pg_write_barrier_impl() __machine_w_barrier() +#endif + +#endif /* HAVE_MBARRIER_H */ + /* Older versions of the compiler don't have atomic.h... */ #ifdef HAVE_ATOMIC_H -- 1.8.4.1 -- 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] Question about RI checks
Kevin Grittner kgri...@ymail.com wrote: Florian Pflug f...@phlo.org wrote: Also, note that after the DELETE FROM parent, further SELECTS in the same transaction will use the original snapshot again, und thus will see the conflicting child rows again that were ignored by the RI trigger. But they won't, of course, see the parent row. IOW, transaction A will, after the delete, see a state of the database in which the PK constraint is broken. I don't think that's acceptable in any isolation level. Good point. Based on that observation, I agree that our RI is broken at both the REPEATABLE READ and SERIALIZABLE isolation levels. I think that READ COMMITTED is OK, because it will see the child row as deleted in time to prevent problems. Every way I look at it, inside a REPEATABLE READ or SERIALIZABLE transaction a check for child rows when validating a parent DELETE should consider both rows which exist according to the transaction snapshot and according to a current snapshot. Interestingly, the run of the query passes both snapshots through to the executor, but for this query the estate-es_crosscheck_snapshot field (which contains the transaction snapshot) doesn't seem to be consulted. It makes me wonder whether we were at some point doing this right and it later got broken. Before I write a patch to fix this, does anyone feel that we should not use that -- in other words, does anyone consider that it is OK for a REPEATABLE READ or SERIALIZABLE transaction to delete a referenced row if that transaction can see referencing rows but a concurrent transaction has deleted them? (This currently allows subsequent queries in the transaction to see orphaned child rows when they can no longer see the parent.) -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] idea: allow AS label inside ROW constructor
2014-10-23 17:36 GMT+02:00 David G Johnston david.g.johns...@gmail.com: Andrew Dunstan wrote On 10/23/2014 09:57 AM, Florian Pflug wrote: On Oct23, 2014, at 15:39 , Andrew Dunstan lt; andrew@ gt; wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule lt; pavel.stehule@ gt; wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. Well, I think we need to see those other use cases. The only use case I recall seeing involves the already provided case of constructing JSON. Even if it simply allows CTE and sibqueries to form anonymous record types which can then be re-expanded in the outer layer for table-like final output this feature would be useful. When working with wide tables and using multiple aggregates and joins being able to avoid specifying individual columns repeatedly is quite desirable. Expanding anonymous record is harder task, but it is possible probably Pavel It would be especially nice to not have to use as though, if the source fields are already so named. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/idea-allow-AS-label-inside-ROW-constructor-tp5823954p5824045.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] idea: allow AS label inside ROW constructor
On 10/23/2014 11:36 AM, David G Johnston wrote: Andrew Dunstan wrote On 10/23/2014 09:57 AM, Florian Pflug wrote: On Oct23, 2014, at 15:39 , Andrew Dunstan lt; andrew@ gt; wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule lt; pavel.stehule@ gt; wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. Well, I think we need to see those other use cases. The only use case I recall seeing involves the already provided case of constructing JSON. Even if it simply allows CTE and sibqueries to form anonymous record types which can then be re-expanded in the outer layer for table-like final output this feature would be useful. When working with wide tables and using multiple aggregates and joins being able to avoid specifying individual columns repeatedly is quite desirable. It would be especially nice to not have to use as though, if the source fields are already so named. You can already name the output of CTEs and in many cases subqueries, too. Maybe if you or someone gave a concrete example of something you can't do that this would enable I'd be more convinced. 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] superuser() shortcuts
Alvaro, I noticed something strange while perusing this patch, but the issue predates the patch. Some messages say must be superuser or replication role to foo, but our longstanding practice is to say permission denied to foo. Why do we have this inconsistency? Should we remove it? If we do want to keep the old wording this patch should use permission denied to in the places that it touches. If we were to make it consistent and use the old wording, what do you think about providing an errhint as well? Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot: errmsg - permission denied to create physical replication slot errhint - You must be superuser or replication role to use replication slots. -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.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] idea: allow AS label inside ROW constructor
On Thu, Oct 23, 2014 at 8:51 AM, Andrew Dunstan and...@dunslane.net wrote: On 10/23/2014 11:36 AM, David G Johnston wrote: Andrew Dunstan wrote On 10/23/2014 09:57 AM, Florian Pflug wrote: On Oct23, 2014, at 15:39 , Andrew Dunstan lt; andrew@ gt; wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule lt; pavel.stehule@ gt; wrote: postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. I've wanted to name the field of rows created with ROW() on more than one occasion, quite independent from whether the resulting row is converted to JSON or not. And quite apart from usefulness, this is a matter of orthogonality. If we have named fields in anonymous record types, we should provide a convenient way of specifying the field names. So to summarize, I think this is an excellent idea, json_build_object non-withstanding. Well, I think we need to see those other use cases. The only use case I recall seeing involves the already provided case of constructing JSON. Even if it simply allows CTE and sibqueries to form anonymous record types which can then be re-expanded in the outer layer for table-like final output this feature would be useful. When working with wide tables and using multiple aggregates and joins being able to avoid specifying individual columns repeatedly is quite desirable. It would be especially nice to not have to use as though, if the source fields are already so named. You can already name the output of CTEs and in many cases subqueries, too. Maybe if you or someone gave a concrete example of something you can't do that this would enable I'd be more convinced. cheers andrew Mechanically I've wanted to do the following without creating an actual type: {query form} WITH invoiceinfo (invoiceid, invoicedetailentry) AS ( SELECT invoiceid, ROW(itemid, itemdescription, itemcost, itemsale, itemquantity) FROM invoicelines ) [... other CTE joins and stuff here...can carry around the 5 info fields in a single composite until they are needed] SELECT invoiceid, (invoicedetailentry).* FROM invoiceinfo ; {working example} WITH invoiceinfo (invoiceid, invoicedetailentry) AS ( SELECT invoiceid, ROW(itemid, itemdescription, itemcost, itemsale, itemquantity) FROM (VALUES ('1',1,'1',0,1,1)) invoicelines (invoiceid, itemid, itemdescription, itemcost, itemsale, itemquantity) ) SELECT invoiceid, (invoicedetailentry).* FROM invoiceinfo ; This is made up but not dissimilar to what I have worked with. That said I can and do usually either just join in the details one time or I need to do more with the details than just carry them around and so providing a named type usually ends up being the way to go. Regardless the form is representative. My most recent need for this ended up being best handled with named types and support functions operating on those types so its hard to say I have a strong desire for this but anyway. David J.
Re: [HACKERS] Deferring some AtStart* allocations?
On Tue, Oct 21, 2014 at 12:00 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-10-09 15:01:19 -0400, Robert Haas wrote: /* @@ -960,18 +966,38 @@ AtEOXact_Inval(bool isCommit) ... + /* + * We create invalidation stack entries lazily, so the parent might + * not have one. Instead of creating one, moving all the data over, + * and then freeing our own, we can just adjust the level of our own + * entry. + */ + if (myInfo-parent == NULL || myInfo-parent-my_level my_level - 1) + { + myInfo-my_level--; + return; + } + I think this bit might not be correct. What if the subxact one level up aborts? Then it'll miss dealing with these invalidation entries. Or am I misunderstanding something? One of us is. I think you're asking about a situation where we have a transaction, and a subtransaction, and within that another subtransaction. Only the innermost subtransaction has invalidation messages. At the innermost level, we commit; the above code makes those messages the responsibility of the outer subtransaction. If that subtransaction abouts, AtEOSubXact_Inval() gets called again, sees that it has messages (that it inherited from the innermost subtransaction), and takes the exact same code-path that it would have pre-patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [PATCH] add ssl_protocols configuration option
Alvaro Herrera alvhe...@2ndquadrant.com writes: OpenSSL just announced a week or two ago that they're abandoning support for 0.9.8 by the end of next year[1], which means its replacements have been around for a really long time. RHEL5 still has 0.9.8e with backported patches and will be supported until 2017-03-31. FreeBSD 8.4, 9.1, 9.2 and 9.3 all have 0.9.8y with backported patches. 8.4, 9.1 and 9.2 all expire before OpenSSL 0.9.8, but 9.3 will be supported until 2016-12-31. 0.9.8 and 1.0.1 are not binary compatible, so upgrading is *not* an option. We (as in FreeBSD) will have to make do - either develop our own patches or adapt RedHat's. OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of security issues, so anyone *is* using SSL but not at least the 0.9.8 branch, they are in trouble. The latest 0.9.8 still only has TLS 1.0, unless they're planning to backport 1.1 and 1.2 (which I seriously doubt). DES -- Dag-Erling Smørgrav - d...@des.no -- 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] idea: allow AS label inside ROW constructor
On Thu, Oct 23, 2014 at 8:39 AM, Andrew Dunstan and...@dunslane.net wrote: On 10/23/2014 09:27 AM, Merlin Moncure wrote: On Thu, Oct 23, 2014 at 4:34 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hi here is a prototype postgres=# select row_to_json(row(10 as A, row(30 as c, 20 AS B) as x)); row_to_json -- {a:10,x:{c:30,b:20}} (1 row) postgres=# select row_to_json(row(10, row(30, 20))); row_to_json -- {f1:10,f2:{f1:30,f2:20}} (1 row) wow -- this is great. I'll take a a look. Already in 9.4: andrew=# select json_build_object('a',10,'x',json_build_object('c',30,'b',20)); json_build_object {a : 10, x : {c : 30, b : 20}} (1 row) So I'm not sure why we want another mechanism unless it's needed in some other context. json_build_object is super useful for sure, but what about performance? Application communication of data via json has been steadily increasing in terms of overall percentage in all the work that I do and performance is very important. I tested at one million rows and: A. select to_json(array(select json_build_object('a',a,'b',b) from foo f)); takes about twice as long as either: B. select to_json(array(select row(a,b) from foo f)); or C. select to_json(array(select f from foo f)); Note the results aren't quite the same, B anonymizes the columns to 'f1' etc and 'A' adds 5 extra spaces per array element (aside: the json serialization functions are not consistently spaced -- shouldn't they generally be as spartan as possible?). Maybe the performance differences are a reflection if that spurious space consumption though...looking a the code json_build_object just does basic StringInfo processing so I don't see any reason for it to be greatly slower. With a nested construction (json_build_object('a',a,'b',json_build_object('a', a, 'b', b)) vs row(a,b,row(a,b))) the results are closer; about 1.5x the time taken for json_build_object. Not close enough to call it a wash, but not damning either, at least for this one case. In terms of row() construction, there aren't many cases today because row() is used precisely because it destroys column names unless you have a composite type handy to cast (and it's cpu cycle sucking overhead) so I've learned to code around it. In some cases a row() type that preserved names would remove the need for the composite. It doesn't happen *that* often -- usually it comes up when stashing aggregated rows through a CTE. At least some of *those* cases are to work around the lack of LATERAL; my production systems are still on 9.2. All that being said, row() seems to me to have a lot of style points and I don't think nested row constructions should have a dependency on json/jsonb. It's just something you do, and json processing is deferred to the last stage of processing before the data goes out the door..that's where we would presumably apply formatting decisions on top of that. merlin -- 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] [PATCH] add ssl_protocols configuration option
=?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= d...@des.no writes: Alvaro Herrera alvhe...@2ndquadrant.com writes: OpenSSL 0.9.7 has already not gotten fixes for all the latest flurry of security issues, so anyone *is* using SSL but not at least the 0.9.8 branch, they are in trouble. The latest 0.9.8 still only has TLS 1.0, unless they're planning to backport 1.1 and 1.2 (which I seriously doubt). The upshot of this conversation still seems to be that we don't need to do anything. Unless I'm misunderstanding something: (1) No currently supported (or even recently supported) version of either the backend or libpq will select protocol less than TLS 1.0 unless forced to via (poorly chosen) configuration settings. (2) Anyone who is feeling paranoid about shutting off SSLv3 despite (1) can do so via the existing ssl_ciphers GUC parameter. Seems to me that's sufficient, not only for now but for the future; existing OpenSSL practice is that the ciphers string includes categories corresponding to protocol versions, so you can shut off an old protocol version there if you need to. 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] Proposal : REINDEX SCHEMA
On Wed, Oct 22, 2014 at 9:21 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Sawada Masahiko wrote: Thank you for reviewing. I agree 2) - 5). Attached patch is latest version patch I modified above. Also, I noticed I had forgotten to add the patch regarding document of reindexdb. Please don't use pg_catalog in the regression test. That way we will need to update the expected file whenever a new catalog is added, which seems pointless. Maybe create a schema with a couple of tables specifically for this, instead. Attached new regression test. Isn't better join the two patches in just one? Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index cabae19..e47604c 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation refsynopsisdiv synopsis -REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ] +REINDEX { INDEX | TABLE | ALL IN SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ] /synopsis /refsynopsisdiv @@ -101,6 +101,19 @@ REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERnam /varlistentry varlistentry +termliteralALL IN SCHEMA/literal/term +listitem + para + Recreate all indexes of the specified schema. If the table has a + secondary quoteTOAST/ table, that is reindexed as well. + Indexes on shared system catalogs are also processed. + This form of commandREINDEX/command cannot be executed inside a + transaction block. + /para +/listitem + /varlistentry + + varlistentry termliteralDATABASE/literal/term listitem para diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3c1e90e..7c25e02 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1770,34 +1770,52 @@ ReindexTable(RangeVar *relation) } /* - * ReindexDatabase - * Recreate indexes of a database. + * ReindexDatabaseOrSchema + * Recreate indexes of a database or schema. * + * kind means the type of object, database or schema. * To reduce the probability of deadlocks, each table is reindexed in a * separate transaction, so we can release the lock on it right away. * That means this must not be called within a user transaction block! */ Oid -ReindexDatabase(const char *databaseName, bool do_system, bool do_user) +ReindexDatabaseOrSchema(const char *objectName, bool do_system, bool do_user, ObjectType kind) { + Oid objectOid; Relation relationRelation; HeapScanDesc scan; + ScanKeyData *key = NULL; HeapTuple tuple; MemoryContext private_context; MemoryContext old; List *relids = NIL; ListCell *l; + bool do_database = (kind == OBJECT_DATABASE); + int nkeys; - AssertArg(databaseName); + AssertArg(objectName); + Assert(kind == OBJECT_DATABASE || kind == OBJECT_SCHEMA); - if (strcmp(databaseName, get_database_name(MyDatabaseId)) != 0) + /* Get OID of object for result */ + if (do_database) + objectOid = MyDatabaseId; + else + objectOid = get_namespace_oid(objectName, false); + + if (!do_database) + { + do_system = IsSystemNamespace(objectOid); + do_user = !do_system; + } + + if (do_database strcmp(objectName, get_database_name(MyDatabaseId)) != 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg(can only reindex the currently open database))); - if (!pg_database_ownercheck(MyDatabaseId, GetUserId())) + if (do_database !pg_database_ownercheck(MyDatabaseId, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE, - databaseName); + objectName); /* * Create a memory context that will survive forced transaction commits we @@ -1806,7 +1824,8 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user) * abort cleanup logic. */ private_context = AllocSetContextCreate(PortalContext, - ReindexDatabase, + (do_database) ? + ReindexDatabase : ReindexSchema, ALLOCSET_DEFAULT_MINSIZE, ALLOCSET_DEFAULT_INITSIZE, ALLOCSET_DEFAULT_MAXSIZE); @@ -1824,6 +1843,23 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user) MemoryContextSwitchTo(old); } + /* For schema, we search target relations by relnamespce and relkind */ + if (!do_database) + { + key = palloc(sizeof(ScanKeyData) * 2); + ScanKeyInit(key[0], + Anum_pg_class_relnamespace, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(objectOid)); + ScanKeyInit(key[1], + Anum_pg_class_relkind, + BTEqualStrategyNumber, F_CHAREQ, + 'r'); +
Re: [HACKERS] Question about RI checks
On Oct23, 2014, at 17:45 , Kevin Grittner kgri...@ymail.com wrote: Every way I look at it, inside a REPEATABLE READ or SERIALIZABLE transaction a check for child rows when validating a parent DELETE should consider both rows which exist according to the transaction snapshot and according to a current snapshot. Interestingly, the run of the query passes both snapshots through to the executor, but for this query the estate-es_crosscheck_snapshot field (which contains the transaction snapshot) doesn't seem to be consulted. It makes me wonder whether we were at some point doing this right and it later got broken. I've been pondering a completely different way to fix this. Many years ago I tried to get rid of the crosscheck snapshot completely by changing the way locking conflicts are treated for REPEATABLE READ transactions above. The basic idea is that taking a share lock on a row implies that you're going to apply further changes whose correctness depends on existence of the row you lock. That, in particular, applies to the locks taken by RI triggers -- we lock the parent row before we add children, because the children's existence necessitates the existence of the parent. If you take an exclusive lock, OTOH, that implies a modification of the row itself (we never explicitly take that lock during an UPDATE or DELETE, but we do so implicitly, because UPDATEs and DELETEs conflict with SHARE locks). So after obtaining such a lock, its the lock holder's responsibility to check that the desired update doesn't break anything, i.e. in the case of RI that it doesn't create any orphaned children. The only reason we need the crosscheck snapshot to do that is because children may have been added (and the change committed) *after* the transaction which removed the parent has taken its snapshot, but *before* that transaction locks the parent row. My proposal is to instead extend the locking protocol to prevent that. Essentially, we have to raise a serialization error whenever 1) We attempt to exclusively lock a row (this includes updating or deleting it), and 2) somebody else did hold a share lock on that row recently, and 3) That somebody is invisible to us according to our snapshot. My initial attempt to do that failed, because we used to have very little means of storing the locking history - the only source of information was the xmax field, so any update of a tuple removed information about previous lock holders - even if that update was later aborted. I pondered using multi-xids for this, but at the time I deemed that too risky - back at the time, they had a few wraparound issues and the like which were OK for share locks, but not for what I intended. But now that we have KEY SHARE locks, the situation changes. We now rely on multi-xids to a much greater extent, and AFAIK multi-xid wraparound is now correctly dealt with. We also already ensure that the information contained in multi-xids are preserved across tuple upgrades (otherwise, updating a row on which someone holds a KEY SHARE lock would be broken). So all that is missing, I think, is 1) To make sure we only remove a multi-xid if none of the xids are invisible to any snapshot (i.e. lie before GlobalXmin or something like that). 2) When we acquire a lock (either explicitly or implicitly by doing an UPDATE or DELETE) check if all previous committed lock holders are visible according to our snapshot, and raise a serialization error if not. The big advantage of doing that over fixing the crosscheck logic would be that it'd make it possible to write concurrency-safe FK triggers in any procedural language. best regards, Florian Pflug -- 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] [PATCH] add ssl_protocols configuration option
Tom Lane t...@sss.pgh.pa.us writes: Anyone who is feeling paranoid about shutting off SSLv3 despite (1) can do so via the existing ssl_ciphers GUC parameter [...] the ciphers string includes categories corresponding to protocol versions, so you can shut off an old protocol version there if you need to. The overlap between SSL 3.0 and TLS 1.0 is 100%: % openssl ciphers SSLv2 | md5 fe5ff23432f119364a1126ca0776c5db % openssl ciphers SSLv3 | md5 bde4e4a10b9c3f323c0632ad067e293a % openssl ciphers TLSv1 | md5 bde4e4a10b9c3f323c0632ad067e293a % openssl ciphers TLSv1.2 | md5 26c375b6bdefb018b9dd7df463658320 Thus, if you disable all SSL 3.0 ciphers, you also disable TLS 1.0. DES -- Dag-Erling Smørgrav - d...@des.no -- 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] [Windows,PATCH] Use faster, higher precision timer API
On Thu, Oct 23, 2014 at 5:41 AM, David Rowley dgrowle...@gmail.com wrote: On Thu, Oct 23, 2014 at 1:27 AM, Craig Ringer cr...@2ndquadrant.com wrote: Here's an updated patch addressing David's points. I haven't had a chance to test it yet, on win2k8 or win2k12 due to pgconf.eu . Hi Craig, thanks for the fast turnaround. I've just had a look over the patch again: + DWORD errcode = GetLastError(); + Assert(errcode == ERROR_PROC_NOT_FOUND); I'm not a big fan of this. It seems quite strange to be using Assert in this way. Agreed - I think if you want an error check here it should use elog() or ereport(), not Assert(). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [BUGS] ltree::text not immutable?
Joe Van Dyk j...@tanga.com writes: Seems like casting ltree to text and the subtree function should be immutable? Hm, yeah, I can see no reason why ltree_in and ltree_out shouldn't be immutable. They surely ought not be volatile, which is the way they are marked (by default) right now. The other types in the ltree module have the same disease. More generally, it seems like we ought to have a test in the type_sanity regression script that checks that type I/O functions aren't volatile, because there are various embedded assumptions that this is so, cf commits aab353a60b95aadc00f81da0c6d99bde696c4b75 and 3db6524fe63f0598dcb2b307bb422bc126f2b15d. That wouldn't have directly caught this problem because we don't apply type_sanity or opr_sanity to contrib modules, only to core functions. I have done that manually in the past and think I'll go do it again to see what else crawls out ... but I wonder if anyone can think of a way to automate that? Meanwhile, as far as fixing the immediate issue goes, I propose just fixing the misdeclarations in ltree--1.0.sql without going through all the pushups of making an ltree--1.1.sql. In the past we've fixed similar errors without a catversion bump on the grounds that the implications weren't large enough to justify a catversion bump. I think the equivalent conclusion here is we don't need an extension version bump. 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] superuser() shortcuts
I noticed something strange while perusing this patch, but the issue predates the patch. Some messages say must be superuser or replication role to foo, but our longstanding practice is to say permission denied to foo. Why do we have this inconsistency? Should we remove it? If we do want to keep the old wording this patch should use permission denied to in the places that it touches. If we were to make it consistent and use the old wording, what do you think about providing an errhint as well? Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot: errmsg - permission denied to create physical replication slot errhint - You must be superuser or replication role to use replication slots. As I started looking at this, there are multiple other places where these types of error messages occur (opclasscmds.c, user.c, postinit.c, miscinit.c are just a few), not just around the changes in this patch. If we change them in one place, wouldn't it be best to change them in the rest? If that is the case, I'm afraid that might distract from the purpose of this patch. Perhaps, if we want to change them, then that should be submitted as a separate patch? -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.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] [BUGS] ltree::text not immutable?
I wrote: More generally, it seems like we ought to have a test in the type_sanity regression script that checks that type I/O functions aren't volatile, because there are various embedded assumptions that this is so, cf commits aab353a60b95aadc00f81da0c6d99bde696c4b75 and 3db6524fe63f0598dcb2b307bb422bc126f2b15d. That wouldn't have directly caught this problem because we don't apply type_sanity or opr_sanity to contrib modules, only to core functions. I have done that manually in the past and think I'll go do it again to see what else crawls out ... but I wonder if anyone can think of a way to automate that? Actually, the right thing to do if we want to enforce this is for CREATE TYPE to check the marking. We'd still need a type_sanity test to check erroneous declarations of built-in types, but a complaint in CREATE TYPE would cover all the contrib modules as well as third-party code. I wouldn't propose back-patching such an error check, but it seems reasonable to add it for 9.5. Any objections? 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] superuser() shortcuts
Brightwell, Adam wrote: If we were to make it consistent and use the old wording, what do you think about providing an errhint as well? Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot: errmsg - permission denied to create physical replication slot errhint - You must be superuser or replication role to use replication slots. Sure. As I started looking at this, there are multiple other places where these types of error messages occur (opclasscmds.c, user.c, postinit.c, miscinit.c are just a few), not just around the changes in this patch. If we change them in one place, wouldn't it be best to change them in the rest? If that is the case, I'm afraid that might distract from the purpose of this patch. Perhaps, if we want to change them, then that should be submitted as a separate patch? Yeah. I'm just saying that maybe this patch should adopt whatever wording we agree to, not that we need to change other places. On the other hand, since so many other places have adopted the different wording, maybe there's a reason for it and if so, does anybody know what it is. But I have to say that it does look inconsistent to me. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [PATCH] add ssl_protocols configuration option
On Fri, Oct 17, 2014 at 1:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: This looks to me like re-fighting the last war. Such a GUC has zero value *unless* some situation exactly like the POODLE bug comes up again, and the odds of that are not high. I think it's pretty common for flaws to be discovered in particular protocols - usually, but not always, the oldest ones. The cost of adding a new GUC seems pretty small tom me compared to the appealing potential for users to secure their installation by changing a configuration setting rather than, say, having to wait for new packages to be available to install. Moreover, the GUC could easily be misused to decrease rather than increase one's security, if it's carelessly set. Remember that we only recently fixed bugs that prevented use of the latest TLS version. If we have a setting like this, I fully anticipate that people will set it to only use TLS 1.2 and then forget that they ever did that; years from now they'll still be using 1.2 when it's deprecated. checkpoint_segments can easily be misused to decrease rather than increase performance, and autovacuum_naptime can easily be misused to destroy rather than improve autovacuum behavior. If we only added GUCs that couldn't be used to make things worse, we'd have no GUCs. The bottom line for me is that OpenSSL has (a) a seemingly never-ending supply of serious security flaws and (b) an exposed knob intended to help users of OpenSSL mitigate the effects of those flaws. Exposing that knob to our users seems like a good plan; supporting alternate SSL implementations might be an even better one, not that the two are mutually exclusive. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] KEY UPDATE / UPDATE / NO KEY UPDATE distinction vs. README.tuplock
Florian Pflug wrote: It seems that README.tuplock never got updated when the KEY SHARE patch's lock level were changed from being KEY UPDATE / UPDATE / SHARE / KEY SHARE to UPDATE / NO KEY UPDATE / SHARE / KEY SHARE. You're right. We changed the tuple lock modes at the last minute and I forgot to update this README. I just pushed it; thanks for the patch and for improving the wording of that paragraph also. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [PATCH] add ssl_protocols configuration option
Dag-Erling Smørgrav wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: OpenSSL just announced a week or two ago that they're abandoning support for 0.9.8 by the end of next year[1], which means its replacements have been around for a really long time. RHEL5 still has 0.9.8e with backported patches and will be supported until 2017-03-31. FreeBSD 8.4, 9.1, 9.2 and 9.3 all have 0.9.8y with backported patches. 8.4, 9.1 and 9.2 all expire before OpenSSL 0.9.8, but 9.3 will be supported until 2016-12-31. 0.9.8 and 1.0.1 are not binary compatible, so upgrading is *not* an option. We (as in FreeBSD) will have to make do - either develop our own patches or adapt RedHat's. I think you misread me. I was saying to desupport 0.9.7, not 0.9.8. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] INSERT ... ON CONFLICT {UPDATE | IGNORE}
On Thu, Oct 23, 2014 at 6:43 PM, Peter Geoghegan p...@heroku.com wrote: Documentation === The documentation has been updated, incorporating feedback. I also made the cardinality violation error a lot clearer than before, since Craig said that was unclear. For the convenience of those that want to read the documentation quickly to figure out one particular aspect of user-visible behavior, the latest revision is uploaded here: http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs Of particular interest: http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/transaction-iso.html#XACT-READ-COMMITTED http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/trigger-definition.html http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-createview.html#SQL-CREATEVIEW-UPDATABLE-VIEWS http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/ddl-inherit.html -- Peter Geoghegan -- 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] Getting rid of accept incoming network connections prompts on OS X
On 10/21/14 1:16 PM, Tom Lane wrote: If you do any Postgres development on OS X, you've probably gotten seriously annoyed by the way that, every single time you reinstall the postmaster executable, you get a dialog box asking whether you'd like to allow it to accept incoming network connections. I used to, but somehow I don't see this anymore. Just to be sure, I made sure the firewall is on, checked that postgres is not in the exception list, rebooted, built postgresql from scratch, ran make check, but no pop-up. I'm on Yosemite. Maybe this was changed. -- 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] [Windows,PATCH] Use faster, higher precision timer API
On 10/23/2014 09:21 PM, Robert Haas wrote: Agreed - I think if you want an error check here it should use elog() or ereport(), not Assert(). That's what I originally did, but it's too early for elog. I'm reluctant to just fprintf(...) to stderr, as there's no way for the user to suppress that, and it'll be emitted for each backend start. Though on the other hand it really is a shouldn't happen case. So the options seem to be ignoring the error silently or printing to stderr. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers