Re: [HACKERS] PATCH: optimized DROP of multiple tables within a transaction
On Sun, Dec 9, 2012 at 1:07 AM, Tomas Vondra wrote:
> * If you're dropping a single table, it really does not matter - the
> difference will be like 100 ms vs. 200 ms or something like that.
Did you try dropping 10,000 tables in 100 batches, as same as previous post?
If so the overhead introduced by the patch is only 1.52ms per table.
It seems acceptable to me.
> So I'd vote to ditch that special case optimization.
+1. It seems very difficult to determine reasonable threshold of such
kind of optimization. As described above, the overhead seems enough
little, so using bsearch in any case seems fine.
Besides, v3.2 patch needs some more fixes, for minor issues.
* Opening curly bracket of if/for/while/etc. should be in the next
line, like this:
if (foo == bar) /* { <= not here */
{
...
}
* Use hard-tab instead of white-spaces to indent variable name in declarations.
For these two issues, please see the page below:
http://www.postgresql.org/docs/9.2/static/source-format.html
* PostgreSQL allows C89 features, so we can't use variable length array.
* Contains unintentional blank lines?
Please see attached patch for details of fixes above.
--
Shigeru HANADA
drop-in-tx.diff
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] [PATCH] PL/Python: Add spidata to all spiexceptions
Hi, I don't feel particularly qualified to comment, but in the interest of (hopefully) helping with the patch review process I'm going to comment anyway. (Having written this, I feel decidedly unqualified to critique so please keep this in mind when considering my comments.) On 10/31/2012 04:33:17 AM, Jan Urbański wrote: > You're right, I never thought of the possibility of user code > explicitly > throwing SPIError exceptions. > > The root issue is that PLy_elog will only set errcode if it finds the > "spidata" attribute, but I think passing error details through that > attribute is a kludge more than something more code should rely on. > > Here's an alternative patch that takes advantage of the alreadu > present > (and documented) "sqlstate" variable to set the error code when > handling > SPIError exceptions. It does seem to me that using sqlstate is the appropriate approach. If you're going to have user code throw the SPIError exception then shouldn't you allow them to also set detail and hints? Setting sqlstate seems a step in the right direction. I don't feel well enough qualified to say whether it goes far enough to be useful. I do note that pl/pgsql users are allowed to raise any 5 character error code regardless of whether it's listed in the documented set of error codes. This is a lot less useful if the code can't supply anything more than an error code. Extending the Python exception class to add attributes to SPIError for message, detail, and hint seems called for in the long run. I also wonder whether the if (sqlstate == NULL) test really belongs in the PLy_get_spi_sqlerrcode() code. Seems to me that different callers might need to do different things in this case, and that instead of PLy_get_spi_sqlerrcode you might instead want a function PLy_sqlstate_to_errcode(PyObject *sqlstate, int *sqlerrcode) and do the if (sqlstate == NULL) test (and the surrounding infrastructure) in the calling code. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein P.S. For reasons I don't understand I can't seem to download your patch directly from the mailing list archive at: http://archives.postgresql.org/pgsql-hackers/2012-10/msg01590.php -- 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]Tablesample Submission
On Sun, Nov 4, 2012 at 10:22 PM, Qi Huang wrote: > Dear hackers > Sorry for not replying the patch review. I didn't see the review until > recently as my mail box is full of Postgres mails and I didn't notice the > one for me, my mail box configuration problem. > I am still kind of busy with my university final year project. I shall > not have time to work on updating the patch until this semester finishes > which is December. I will work on then. While we are still in the middle of a commitfest, i'm curious... should we still wait for an update of this patch? i know, any update on this should go to the next commitfest but i wanted to ask before i forget about it -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] CommitFest #3 and upcoming schedule
On Sunday, December 09, 2012 9:27 PM Simon Riggs > On 16 November 2012 07:20, Greg Smith wrote: > > > Let's bring balance to the situation through our own actions. Please > review one patch now for every one you submitted. In CF-3, I am Author of 5 and Reviewer of 5 3 of my patches as Author have been moved from CF-2 4 of the patches where I am reviewer have been moved from CF-2 One of my Patch : Patch for option in pg_resetxlog for restore from WAL files is dependent on another patch XLogReader, so I am expecting to get it done only after XLogReader. I wanted to know if I should attach myself as reviewer to more patches as per initial policy of CF? In anycase as soon as I get time I shall review more patches. With Regards, Amit Kapila. -- 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] Submission Review: User control over psql error stream
On 12/09/2012 03:58:26 PM, Karl O. Pinc wrote: > Hi Alastair, > > On 12/09/2012 02:13:32 PM, Alastair Turner wrote: > > - It's closed ended - there are three things about error output > which > > affect where it's written to: does it go to query output, does it > go > > somewhere else (a file or pipe), does it get displayed as well as > > going to the other destination. The patch addresses the first and > > third questions without making allowance for asking or dealing with > > second one. Internally I think this should be two bools rather than > a > > custom tri-state, mainly because this would allow the addition of > the > > error file option (in another patch, when the time came) with > minimum > > intrusion. > > I was thinking along the same lines, that case 2) stderr to a file > or pipe needs addressing. I think it's necessary to address the > issue now. Otherwise we risk cluttering up the syntax in > inhospitable ways. It occurs to me that my reply does not address the issue of case 3, displaying or not) errors to the terminal in addition to wherever else errors are sent. I think you're right, whether or not errors continue to be sent to stdout when they are directed elsewhere should be a separate flag. My inclination is to have another special psql variable to control this but that would break backwards compatibility. Instead, let's work out a syntax for the rest of the functionality and try to fit this in. One nice thing about special psql variables is that they self-report their state. I mention this since we're adding more state. It would be nice if the chosen syntax does not preclude some additional tweaking to report on the state involving the output streams. (Although I don't think that needs to be a part of this patch.) And oh yes, \e won't work as a command since it's already taken. Since this is only an issue if \o is not overloaded I await your review. Thanks for the help. Regards, Karl Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein -- 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?] lag of minRecoveryPont in archive recovery
Hello, > I think that minRecoveryPoint should be updated before the data-file > changes, so XLogFlush() should be called before smgrtruncate(). No? Mmm.. As far as I saw in xact_redo_commit_internal, XLogFlush seems should be done AFTER redo substantial operation. Is it wrong? If so, I suppose xact_redo_commit_internal could shold be fixed in the same way. At Mon, 10 Dec 2012 00:41:34 +0900, Fujii Masao wrote in > On Thu, Dec 6, 2012 at 1:04 PM, Kyotaro HORIGUCHI > wrote: > > diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c > > index 993bc49..d34ab65 100644 > > --- a/src/backend/catalog/storage.c > > +++ b/src/backend/catalog/storage.c > > @@ -519,6 +519,12 @@ smgr_redo(XLogRecPtr lsn, XLogRecord *record) > > visibilitymap_truncate(rel, xlrec->blkno); > > > > FreeFakeRelcacheEntry(rel); > > + > > + /* > > +* Xlogs before this record is unrepeatable, so winding > > +* minRecoveryPoint to here. > > +*/ > > + XLogFlush(lsn); > > } > > else > > elog(PANIC, "smgr_redo: unknown op code %u", info); > > I think that minRecoveryPoint should be updated before the data-file > changes, so XLogFlush() should be called before smgrtruncate(). No? -- Kyotaro Horiguchi NTT Open Source Software Center -- 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?] lag of minRecoveryPont in archive recovery
Thank you. > I think moving CheckRecoveryConsistency() after redo apply loop might cause > a problem. > As currently it is done before recoveryStopsHere() function, which can allow > connections > on HOTSTANDY. But now if due to some reason recovery pauses or stops due to > above function, > connections might not be allowed as CheckRecoveryConsistency() is not > called. It depends on the precise meaning of minRecoveryPoint. I've not found the explicit explanation for it. Currently minRecoveryPoint is updated only in XLogFlush. Other updates of it seems to reset to InvalidXLogRecPtr. XLogFlush seems to be called AFTER the redo core operation has been done, at least in xact_redo_commit_internal. I shows me that the meaning of minRecoveryPoint is that "Just AFTER applying the XLog at current LSN, the database files will be assumed to be consistent." At Mon, 10 Dec 2012 00:36:31 +0900, Fujii Masao wrote in > Yes, so we should just add the CheckRecoveryConsistency() call after > rm_redo rather than moving it? This issue is related to the old discussion: > http://archives.postgresql.org/pgsql-bugs/2012-09/msg00101.php Since I've not cleary understood the problem of missing it before redo, and it also seems to have no harm on performance, I have no objection to place it both before and after of redo. -- Kyotaro Horiguchi NTT Open Source Software Center -- 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] Proof of concept: auto updatable views [Review of Patch]
Simon Riggs writes: > On 9 December 2012 22:00, Tom Lane wrote: >> But in any case, those functions are expensive enough that I can't see >> running them against every view in the DB anytime somebody hits tab. >> I think just allowing tab-completion to include all views is probably >> the best compromise. > I'm surprised to see that "updateable" and "trigger updateable" states > aren't recorded in the catalog somewhere. ISTM a useful thing to be > able to know about a view and not something we should be calculating > on the fly. That has nothing much to do with tab completion, it just > seems like a generally useful thing. No, I don't find that a useful idea. These things are not that expensive to check given that you have an open relcache entry to look at, which would be the case anywhere in the backend that we wanted to know them. The reason that running the functions in a tab-completion query looks unpleasant is that it'd imply opening (and probably locking) a large number of views. If we did put an "updatable" flag into the catalogs then (1) we'd be giving up the ability to change the updatability conditions without an initdb, and (2) we'd have a problem with updating the flag for referencing views when a referenced view changed its state. 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] CommitFest #3 and upcoming schedule
On 9.12.2012 22:41, Jeff Janes wrote: > On Sun, Dec 9, 2012 at 10:47 AM, Tomas Vondra wrote: >> >> IMHO many of the patches that are currently marked as "needs review" and >> have no reviewers, were actually reviewed or are being discussed >> thoroughly on the list, but this information was not propagated to the >> CF page. > > Should active discussion on the hackers list prevent someone from > doing a review? I know I am reluctant to review a patch when it seems > it is still being actively redesigned/debated by others. > > Maybe a new status of "needs design consensus" would be useful. IMHO introducing new statuses won't improve the state. Moreover reaching a design consensus is a natural part of the review process. I see those discussions as a part of the review process, so it's not that an active discussion means 'no review' (although the CF page states "needs review" or "no reviewer" for such patches). There's nothing wrong with doing yet another review for a patch, but in most cases I tend to agree with the points already raised in the discussion so it's not really productive. Thus I share the same reluctance to do more reviews for those actively discussed patches. My point is that some of the "idle patches" are actually quite active in the background, no one just updated the CF page. And I see many such patches moved forward over the last few days. Tomas -- 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] Proof of concept: auto updatable views [Review of Patch]
On 9 December 2012 22:00, Tom Lane wrote: > Dean Rasheed writes: >> It's a shame though that pg_view_is_updatable() and >> pg_view_is_insertable() are not really useful for identifying >> potentially updatable views (e.g., consider an auto-updatable view on >> top of a trigger-updatable view). I'm left wondering if I >> misinterpreted the SQL standard's intentions when separating out the >> concepts of "updatable" and "trigger updatable". It seems like it >> would have been more useful to have "trigger updatable" imply >> "updatable". > > I wondered about that too, but concluded that they were separate after > noticing that the standard frequently writes things like "updatable or > trigger updatable". They wouldn't need to write that if the latter > implied the former. > > But in any case, those functions are expensive enough that I can't see > running them against every view in the DB anytime somebody hits tab. > I think just allowing tab-completion to include all views is probably > the best compromise. I'm surprised to see that "updateable" and "trigger updateable" states aren't recorded in the catalog somewhere. ISTM a useful thing to be able to know about a view and not something we should be calculating on the fly. That has nothing much to do with tab completion, it just seems like a generally useful thing. -- 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] Proof of concept: auto updatable views [Review of Patch]
On 9 December 2012 22:00, Tom Lane wrote: > Dean Rasheed writes: >> It's a shame though that pg_view_is_updatable() and >> pg_view_is_insertable() are not really useful for identifying >> potentially updatable views (e.g., consider an auto-updatable view on >> top of a trigger-updatable view). I'm left wondering if I >> misinterpreted the SQL standard's intentions when separating out the >> concepts of "updatable" and "trigger updatable". It seems like it >> would have been more useful to have "trigger updatable" imply >> "updatable". > > I wondered about that too, but concluded that they were separate after > noticing that the standard frequently writes things like "updatable or > trigger updatable". They wouldn't need to write that if the latter > implied the former. > Yeah, that was my reasoning too. > But in any case, those functions are expensive enough that I can't see > running them against every view in the DB anytime somebody hits tab. > I think just allowing tab-completion to include all views is probably > the best compromise. > Agreed. Regards, Dean -- 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] Proof of concept: auto updatable views [Review of Patch]
Dean Rasheed writes: > It's a shame though that pg_view_is_updatable() and > pg_view_is_insertable() are not really useful for identifying > potentially updatable views (e.g., consider an auto-updatable view on > top of a trigger-updatable view). I'm left wondering if I > misinterpreted the SQL standard's intentions when separating out the > concepts of "updatable" and "trigger updatable". It seems like it > would have been more useful to have "trigger updatable" imply > "updatable". I wondered about that too, but concluded that they were separate after noticing that the standard frequently writes things like "updatable or trigger updatable". They wouldn't need to write that if the latter implied the former. But in any case, those functions are expensive enough that I can't see running them against every view in the DB anytime somebody hits tab. I think just allowing tab-completion to include all views is probably the best compromise. 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] Submission Review: User control over psql error stream
Hi Alastair, On 12/09/2012 02:13:32 PM, Alastair Turner wrote: > Hi Karl, > > I have given the patch a quick review and read the related mails > following its initial submission. Thank you very much. > - It's closed ended - there are three things about error output which > affect where it's written to: does it go to query output, does it go > somewhere else (a file or pipe), does it get displayed as well as > going to the other destination. The patch addresses the first and > third questions without making allowance for asking or dealing with > second one. Internally I think this should be two bools rather than a > custom tri-state, mainly because this would allow the addition of the > error file option (in another patch, when the time came) with minimum > intrusion. I was thinking along the same lines, that case 2) stderr to a file or pipe needs addressing. I think it's necessary to address the issue now. Otherwise we risk cluttering up the syntax in inhospitable ways. > - \pset is not the right place for this switch - all the > documentation > (including \?) indicate that psets are for controlling the display of > result tables and this doesn't fit with the other options to \pset. > Since this affects what goes into the output file I would think that > this should be an option to \o. Maybe \o& ? Before talking about syntax let's get semantics out of the way. I think we're agreed that we want to be able to mix stderr in with stdout. It makes sense to me to be able to say, as in shell, send stderr to wherever stdout is going, without having to re-specify where stdout has actually gone. This is especially needed in the case of pipes. The big question is whether, after redirecting stderr to stdout, redirecting stdout to someplace else also then changes where stderr goes. My inclination is that it does not, that it work like shell (without the godawful syntax). In shell, cat foo 2>&1 >/dev/null sends stderr to stdout and discards stdout. (In psudocode: stderr=stdout; stdout=/dev/null) Likewise, cat foo >/dev/null 2>&1 discards both stderr and stdout. (stdout=/dev/null; stderr=stdout) Regards syntax: It seems cleanest to make the syntax involved in redirection of stderr look (almost) like redirection of stdout. And likewise redirecting stdout should look like redirecting stderr. The latter would then allow the semantics of redirecting stdout to wherever stderr is going, without having to re-specify exactly where stderr has been sent. I don't have a good sense of what you're proposing regards using a '\o&' type of syntax. (But see below.) Please elaborate if you'd like me to pursue this direction. My thoughts are having a \e command that would look like \o. So: \o file stdout to file \o |prog stdout to pipe \oe stdout to wherever stderr is going \ostdout to /dev/stdout \e file stderr to file \e |prog stderr to pipe \eo stderr to wherever stdout is going \estderr to /dev/stderr The above seem to be the semantic permutations the syntax needs to handle. It would be nice to be able to not have to introduce a new \ command, but I can't think of a clear way to shoehorn stderr into \o. The best I can come up with, based on your & idea, is something like: (Text enclosed in [] is optional.) \o[o][&] file stdout to file \o[o][&] |prog stdout to pipe \o[o]&e stdout to wherever stderr is going \o[&] stdout to /dev/stdout \o[&o] stdout to wherever stdout is going (noop) \oe[&] file stderr to file \oe[&] |progstderr to pipe \oe&o stderr to wherever stdout is going \oe[&] stderr to /dev/stderr \oe[&e] stderr to wherever stderr is going (noop) It seems cluttered. And the & seems redundant. Without & it looks like: \o[o] file stdout to file \o[o] |prog stdout to pipe \o[o]e stdout to wherever stderr is going \o stdout to /dev/stdout \oe file stderr to file \oe |progstderr to pipe \oeo stderr to wherever stdout is going \oe stderr to /dev/stderr I suppose it's also possible to do as shell does and (probably with & as a syntax token) be able to use file descriptor numbers directly. It's not clear to me where the win is in doing that. Would you be thinking & introduces additional syntax?: \o[&[o[>]]] file stdout to file \o[&[o[>]]] |prog stdout to pipe \o&[o]>e stdout to wherever stderr is going \o[&[o[>]]]stdout to /dev/stdout \o&e[>] file stderr to file \o&e[>] |prog stderr to pipe \o&e>o stderr to wherever stdout is going \o&e[>]stderr to /dev/stderr What sort of future extensibility do we get by adding syntax to \o as opposed to adding a new command? I'm unclear on how extending \o would fit in with --output. (Adding a new \e command would seemingly involve adding a new command line option.) Thanks for the help. Karl Free Software: "You don't pay back, you pay forward."
Re: [HACKERS] CommitFest #3 and upcoming schedule
On Sun, Dec 9, 2012 at 10:47 AM, Tomas Vondra wrote: > > IMHO many of the patches that are currently marked as "needs review" and > have no reviewers, were actually reviewed or are being discussed > thoroughly on the list, but this information was not propagated to the > CF page. Should active discussion on the hackers list prevent someone from doing a review? I know I am reluctant to review a patch when it seems it is still being actively redesigned/debated by others. Maybe a new status of "needs design consensus" would be useful. Cheers, Jeff -- 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] Proof of concept: auto updatable views [Review of Patch]
On 9 December 2012 16:53, Tom Lane wrote: > Thom Brown writes: >> One observation: There doesn't appear to be any tab-completion for view >> names after DML statement keywords in psql. Might we want to add this? > > Well, there is, but it only knows about INSTEAD OF trigger cases. > I'm tempted to suggest that Query_for_list_of_insertables and friends > be simplified to just include all views. > Yeah, that's probably OK for tab-completion. It's a shame though that pg_view_is_updatable() and pg_view_is_insertable() are not really useful for identifying potentially updatable views (e.g., consider an auto-updatable view on top of a trigger-updatable view). I'm left wondering if I misinterpreted the SQL standard's intentions when separating out the concepts of "updatable" and "trigger updatable". It seems like it would have been more useful to have "trigger updatable" imply "updatable". Regards, Dean -- 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] Review of Row Level Security
2012/12/9 Simon Riggs : > On 9 December 2012 06:08, Kohei KaiGai wrote: >> 2012/12/7 Simon Riggs : >>> On 5 December 2012 11:16, Kohei KaiGai wrote: >>> > * TRUNCATE works, and allows you to remove all rows of a table, even > ones you can't see to run a DELETE on. Er... > It was my oversight. My preference is to rewrite TRUNCATE command with DELETE statement in case when row-security policy is active on the target table. In this case, a NOTICE message may be helpful for users not to assume the table is always empty after the command. >>> >>> I think the default must be to throw an ERROR, since part of the >>> contract with TRUNCATE is that it is fast and removes storage. >>> >> OK. Does the default imply you are suggesting configurable >> behavior using GUC or something? >> I think both of the behaviors are reasonable from security point >> of view, as long as user cannot remove unprivileged rows. > > Hmm, its difficult one that. I guess this raises the question as to > whether users know they are accessing a table with RLS enabled. If > they don't and we want to keep it that way, then changing TRUNCATE > into DELETE makes sense. > > To issue TRUNCATE you need the correct privilege, which is separate from > DELETE. > > If they have TRUNCATE privilege they should be allowed to remove all > rows, bypassing the row level security. > > If that behavious isn't wanted, then the table owner can create an > INSTEAD OF TRUNCATE trigger that turns the action into a DELETE, which > is then subject to RLS rules. > It seems to me make sense, also. Even though selinux does not define separated permissions for TRUNCATE, the later option will work well for me in case of row-level label based security is configured in the future version. So, I don't implement something special around TRUNCATE, except for paying mention at the documentation. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Submission Review: User control over psql error stream
Hi Karl, I have given the patch a quick review and read the related mails following its initial submission. I agree with that functionality along these lines is desirable. The ability to manage output from within psql at least as richly as is possible with shell redirection - and change it between commands because it is accessible from with psql - would be great. The basics of the review were fine: the patch applied with minimal fuzz and compiled cleanly. The implementation of the functionality - as built for your specific requirement - concerns me in a few ways though: - It's closed ended - there are three things about error output which affect where it's written to: does it go to query output, does it go somewhere else (a file or pipe), does it get displayed as well as going to the other destination. The patch addresses the first and third questions without making allowance for asking or dealing with second one. Internally I think this should be two bools rather than a custom tri-state, mainly because this would allow the addition of the error file option (in another patch, when the time came) with minimum intrusion. - \pset is not the right place for this switch - all the documentation (including \?) indicate that psets are for controlling the display of result tables and this doesn't fit with the other options to \pset. Since this affects what goes into the output file I would think that this should be an option to \o. Maybe \o& ? Marked "Returned to Author". Regards, Alastair. -- 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] Commits 8de72b and 5457a1 (COPY FREEZE)
On Fri, Dec 07, 2012 at 06:51:18PM -0500, Stephen Frost wrote: > * Jeff Davis (pg...@j-davis.com) wrote: > > Most of your concerns seem to be related to freezing, and I'm mostly > > interested in HEAP_XMIN_COMMITTED optimizations. So I think we're > > talking past each other. > > My concern is about this patch/capability as a whole. I agree that the > hint bit setting may be fine by itself, I'm not terribly concerned with > that. Now, if you (and others) aren't concerned about the overall > behavior that is being introduced here, that's fine, but it was my > understanding from previous discussions that making tuples visible to > all transactions, even those started before the committing transaction > which are set more strictly than 'read-committed', wasn't 'ok'. > > Now, what I've honestly been hoping for on this thread was for someone > to speak up and point out why I'm wrong about this concern and explain > how this patch addresses that issue. If that's been done, I've missed > it.. I favor[1] unconditionally letting older snapshots see the new rows after the CREATE+COPY transaction commits. To recap, making affected scans see an empty table is as wrong as making them see those rows. Robert also listed[2] that as a credible option, and I don't recall anyone opining against it in previous discussions. I did perceive an undercurrent preference, all other things being equal, for an optimization free from semantic side-effects. I shared that preference, but investigations showed that we must compromise something. Though I like the idea of making new relations appear nonexistent to older snapshots, let's keep that as an independent patch. Otherwise, the chance of having none of the above in PostgreSQL 9.3 escalates significantly. Thanks, nm [1] http://archives.postgresql.org/message-id/20120302205834.gc29...@tornado.leadboat.com [2] http://archives.postgresql.org/message-id/CA+TgmoYh_KXErp9eOejMV6EJJaczeZZcSj3kRtq=yg1sjim...@mail.gmail.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] autovacuum truncate exclusive lock round two
Jan Wieck wrote: > Based on the discussion and what I feel is a consensus I have > created an updated patch that has no GUC at all. The hard coded > parameters in include/postmaster/autovacuum.h are > > AUTOVACUUM_TRUNCATE_LOCK_CHECK_INTERVAL 20 /* ms */ > AUTOVACUUM_TRUNCATE_LOCK_WAIT_INTERVAL 50 /* ms */ > AUTOVACUUM_TRUNCATE_LOCK_TIMEOUT 5000 /* ms */ Since these really aren't part of the external API and are only referenced in vacuumlazy.c, it seems more appropriate to define them there. > I gave that the worst workload I can think of. A pgbench (style) > application that throws about 10 transactions per second at it, > so that there is constantly the need to give up the lock due to > conflicting lock requests and then reacquiring it again. A > "cleanup" process is periodically moving old tuples from the > history table to an archive table, making history a rolling > window table. And a third job that 2-3 times per minute produces > a 10 second lasting transaction, forcing autovacuum to give up on > the lock reacquisition. > > Even with that workload autovacuum slow but steady is chopping > away at the table. Applies with minor offsets, builds without warning, and passes `make check-world`. My tests based on your earlier posted test script confirm the benefit. There are some minor white-space issues; for example git diff --color shows some trailing spaces in comments. There are no docs, but since there are no user-visible changes in behavior other than better performance and more prompt and reliable trunction of tables where we were already doing so, it doesn't seem like any new docs are needed. Due to the nature of the problem, tests are tricky to run correctly and take a long time to run, so I don't see how any regressions tests would be appropriate, either. This patch seems ready for committer, and I would be comfortable with making the minor changes I mention above and committing it. I'll wait a day or two to allow any other comments or objections. To summarize, there has been pathalogical behavior in an infrequently-encountered corner case of autovacuum, wasting a lot of resources indefinitely when it is encountered; this patch gives a major performance improvement in in this situation without any other user-visible change and without requiring any new GUCs. It adds a new public function in the locking area to allow a process to check whether a particular lock it is holding is blocking any other process, and another to wrap that to make it easy to check whether the lock held on a particular table is blocking another process. It uses this new capability to be smarter about scheduling autovacuum's truncation work, and to avoid throwing away incremental progress in doing so. As such, I don't think it would be crazy to back-patch this, but I think it would be wise to allow it to be proven on master/9.3 for a while before considering that. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH 02/14] Add support for a generic wal reading facility dubbed XLogReader
On 2012-11-15 16:22:56 +0200, Heikki Linnakangas wrote: > On 15.11.2012 03:17, Andres Freund wrote: > > > >Features: > >- streaming reading/writing > >- filtering > >- reassembly of records > > > >Reusing the ReadRecord infrastructure in situations where the code that wants > >to do so is not tightly integrated into xlog.c is rather hard and would > >require > >changes to rather integral parts of the recovery code which doesn't seem to > >be > >a good idea. > > > >Missing: > >- "compressing" the stream when removing uninteresting records > >- writing out correct CRCs > >- separating reader/writer > > I'm disappointed to see that there has been no progress on this patch since > last commitfest. I thought we agreed on the approach I championed for here: > http://archives.postgresql.org/pgsql-hackers/2012-09/msg00636.php. There > wasn't much work left to finish that, I believe. > > Are you going to continue working on this? Patch (git repo) is now based ontop of my vesion of your xlogreader version... Greetings, Andres Freund -- Andres Freund 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] logical changeset generation v3 - git repository
On 2012-11-15 02:26:53 +0100, Andres Freund wrote: > On 2012-11-15 01:27:46 +0100, Andres Freund wrote: > > In response to this you will soon find the 14 patches that currently > > implement $subject. > > As its not very wieldly to send around that many/big patches all the > time, until the next "major" version I will just update the git tree at: > > Web: > http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlog-decoding-rebasing-cf3 > > Git: > git clone git://git.postgresql.org/git/users/andresfreund/postgres.git > xlog-decoding-rebasing-cf3 I pushed a new version which - is rebased ontop of master - is based ontop of the new xlogreader (biggest part) - is base ontop of the new binaryheap.h - some fixes - some more comments Greetings, Andres Freund -- Andres Freund 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] Unresolved error 0xC0000409 on Windows Server
All, I have successfully isolated this error and created a simple SQL script to reproduce it. Just to recap - this script will cause a server crash with exception 0xC409 as described in previous emails. The crux of the problem seems to be my creation / use of the function st_transform_null. My intent with this function is to wrap the st_transform function provided by PostGIS, but account for the situation where the argument to be transformed is NULL. In this situation, st_transform throws an internal_error, which my function catches and returns NULL for. The error / crash is not caused by a NULL argument; rather, it is caused by the final value in the attached script's INSERT statement, which contains a lat/lon pair that is beyond PostGIS's range. I'm not questioning whether this value is actually outside the legal range, but I do not think such an input should cause the server to crash completely. Here are the steps to reproduce the crash: 1) Create a new instance of a 9.2 server (Windows 64-bit), and a new database (call it test) with the PostGIS extension. 2) Run the script: psql -U postgres -d test -f C:\server_crash.sql You should see the following: psql:C:/server_crash.sql:31: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. psql:C:/server_crash.sql:31: connection to server was lost 3) Check your log for the error. I hope this helps. It took me quite a while to track down the problem so I hope someone can figure out what is going on under the hood. It seems to be a pretty significant problem. Cheers, Matt On Sun, Nov 11, 2012 at 9:45 PM, Matthew Gerber wrote: > > > On Sun, Nov 11, 2012 at 8:27 PM, Tom Lane wrote: > >> Noah Misch writes: >> > So, I can reproduce the lower threshold, but the exception type does >> not agree >> > with the one Matthew observed. >> >> I finally got around to looking at the link you provided about error >> 0xC409, and realized that I'd been completely confusing it with >> stack overflow --- but actually, it's a report that something scribbled >> past the end of a finite-size local-variable array. So I now think that >> Matthew's stumbled across two completely independent bugs, and we've >> fixed only one of them. The 0xC409 error is something else, and >> possibly a lot worse since it could conceivably be a security issue. >> >> It still seems likely that the actual location of the bug is either >> in PostGIS or in the GIST index code, but without the ability to >> reproduce the failure it's awfully hard to find it. Matthew, could >> you try a bit harder to find a self-contained test case that produces >> that error? >> >> regards, tom lane >> > > Sure, it might take me a while to find time but I'll keep it on my list. > > Matt > > server_crash.sql 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] xlogreader v3/xlogdump v2
Hi, On 2012-12-04 18:52:13 +0100, Andres Freund wrote: > At > http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlogreader_v3 > git://git.postgresql.org/git/users/andresfreund/postgres.git > you can find my attempt trying to bring the xlogreader from Heikki, as > modified by Alvaro, into a state where it has the capabilities to be > usable for BDR. > > This is *preliminary* work, to see whether people roughly agree with the > API, there is some smoothing of edges left. > > Changes I made: > * Add XLogFindNextRecord, to find the next valid xlog record >= an recptr > * Move the page validation handling into xlogreader > * Add support for reading pages which are only partially valid > * Add callback as a replacement for emode_for_corrupt_record > > I don't like the last part, it seems ugly to me, but moving the full > error processing/formatting to a callback seems to involve more work. I > am willing to do that work, but would like some input first. > > The xlogdump utility itself is in a mostly good state, some parts of > support infrastructure (ereport wrapper, realpathbackend, > timestamptz_to_str, pfree) need some work. I pushed a new version of the patch with some fixes, more comments and a slightly changed read_page callback API. Greetings, Andres Freund -- Andres Freund 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] CommitFest #3 and upcoming schedule
On 9.12.2012 16:56, Simon Riggs wrote: > On 16 November 2012 07:20, Greg Smith wrote: > >> Project guidelines now ask each patch submitter to review patches of the >> same number and approximate complexity as they submit. If you've submitted >> some items to the CommitFest, please look at the open list and try to find >> something you can review. > > The deadline for 9.3 is looming and many patches have not yet been reviewed. > > I'm sending a public reminder to all patch authors that they should > review other people's patches if they expect their own to be reviewed. > > Simply put, if you don't help each other by reviewing other patches > then the inevitable result will be your patch will not be neither > reviewed nor committed. IMHO many of the patches that are currently marked as "needs review" and have no reviewers, were actually reviewed or are being discussed thoroughly on the list, but this information was not propagated to the CF page. Not sure how to fix this except for updating patches that I've reviewed and urging the others to do the same. Tomas -- 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] Commits 8de72b and 5457a1 (COPY FREEZE)
On Wed, Dec 05, 2012 at 07:43:08PM -0500, Tom Lane wrote: > Robert Haas writes: > > On Tue, Dec 4, 2012 at 3:38 PM, Jeff Davis wrote: > >> After reading that thread, I still don't understand why it's unsafe to > >> set HEAP_XMIN_COMMITTED in those conditions. Even if it is, I would > >> think that a sufficiently narrow case -- such as CTAS outside of a > >> transaction block -- would be safe, along with some slightly broader > >> cases (like BEGIN; CREATE TABLE; INSERT/COPY). > > > I haven't looked at the committed patch - which seemed a bit > > precipitous to me given the stage the discussion was at - but I > > believe the general issue with HEAP_XMIN_COMMITTED is that there might > > be other snapshots in the same transaction, for example from open > > cursors. > > From memory, the tqual.c code assumes that any tuple with XMIN_COMMITTED > couldn't possibly be from its own transaction, and thus it doesn't make > the tests that would be appropriate for a tuple that is from the current > transaction. Maybe it's all right anyway (i.e. if we should always treat > such a tuple as good) but I don't recall exactly what's tested in those > paths. I don't see semantics preservable by freezing, yet omitting HEAP_XMIN_COMMITTED. The "HeapTupleHeaderGetCmin(tuple) >= snapshot->curcid" test is the one at risk. HeapTupleSatisfiesMVCC() does skip that test for HEAP_XMIN_COMMITTED tuples, but seeing xmin==FrozenTransactionId hampers it all the more. What if one of the preconditions for the optimization were the equivalent of CheckTableNotInUse()? I cannot immediately think of a older-cmin-scan source not caught thereby. Unmodified HeapTupleSatisfiesMVCC() will then suffice. Happily, it's not a restriction users will regularly encounter. Thanks, nm -- 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] New statistics for WAL buffer dirty writes
On 29.10.2012 04:58, Satoshi Nagayasu wrote:
> 2012/10/24 1:12, Alvaro Herrera wrote:
>> Satoshi Nagayasu escribi�:
>>
>>> With this patch, walwriter process and each backend process
>>> would sum up dirty writes, and send it to the stat collector.
>>> So, the value could be saved in the stat file, and could be
>>> kept on restarting.
>>>
>>> The statistics could be retreive with using
>>> pg_stat_get_xlog_dirty_writes() function, and could be reset
>>> with calling pg_stat_reset_shared('walwriter').
>>>
>>> Now, I have one concern.
>>>
>>> The reset time could be captured in globalStats.stat_reset_timestamp,
>>> but this value is the same with the bgwriter one.
>>>
>>> So, once pg_stat_reset_shared('walwriter') is called,
>>> stats_reset column in pg_stat_bgwriter does represent
>>> the reset time for walwriter, not for bgwriter.
>>>
>>> How should we handle this? Should we split this value?
>>> And should we have new system view for walwriter?
>>
>> I think the answer to the two last questions is yes. It doesn't seem to
>> make sense, to me, to have a single reset timings for what are
>> effectively two separate things.
>>
>> Please submit an updated patch to next CF. I'm marking this one
>> returned with feedback. Thanks.
>>
>
> I attached the latest one, which splits the reset_time
> for bgwriter and walwriter, and provides new system view,
> called pg_stat_walwriter, to show the dirty write counter
> and the reset time.
I've done a quick review of the v4 patch:
1) applies fine on HEAD, compiles fine
2) "make installcheck" fails because of a difference in the 'rules'
test suite (there's a new view "pg_stat_walwriter" - see the
attached patch for a fixed version or expected/rules.out)
3) I do agree with Alvaro that using the same struct for two separate
components (bgwriter and walwriter) seems a bit awkward. For example
you need to have two separate stat_reset fields, the reset code
becomes much more verbose (because you need to list individual
fields) etc.
So I'd vote to either split this into two structures or keeping it
as a single structure (although with two views on top of it).
4) Are there any other fields that might be interesting? Right now
there's just "dirty_writes" but I guess there are other values. E.g.
how much data was actually written etc.?
Tomas
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index e376452..334ce4c 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -135,6 +135,11 @@ intunlogged_tables = 0;
double sample_rate = 0.0;
/*
+ * logging steps (seconds between log messages)
+ */
+intlog_step_seconds = 5;
+
+/*
* tablespace selection
*/
char *tablespace = NULL;
@@ -1362,6 +1367,11 @@ init(bool is_no_vacuum)
charsql[256];
int i;
+ /* used to track elapsed time and estimate of the remaining time */
+ instr_time start, diff;
+ double elapsed_sec, remaining_sec;
+ int log_interval = 1;
+
if ((con = doConnect()) == NULL)
exit(1);
@@ -1430,6 +1440,8 @@ init(bool is_no_vacuum)
}
PQclear(res);
+ INSTR_TIME_SET_CURRENT(start);
+
for (i = 0; i < naccounts * scale; i++)
{
int j = i + 1;
@@ -1441,10 +1453,27 @@ init(bool is_no_vacuum)
exit(1);
}
- if (j % 10 == 0)
- fprintf(stderr, "%d of %d tuples (%d%%) done.\n",
- j, naccounts * scale,
- (int) (((int64) j * 100) / (naccounts *
scale)));
+ /* let's not call the timing for each row, but only each 100
rows */
+ if (j % 100 == 0 || j == scale * naccounts)
+ {
+ INSTR_TIME_SET_CURRENT(diff);
+ INSTR_TIME_SUBTRACT(diff, start);
+
+ elapsed_sec = INSTR_TIME_GET_DOUBLE(diff);
+ remaining_sec = (scale * naccounts - j) * elapsed_sec /
j;
+
+ /* have we reached the next interval? */
+ if (elapsed_sec >= log_interval * log_step_seconds) {
+
+ fprintf(stderr, "%d of %d tuples (%d%%) done
(elapsed %.2f s, remaining %.2f s).\n",
+ j, naccounts * scale,
+ (int) (((int64) j * 100) /
(naccounts * scale)), elapsed_sec, remaining_sec);
+
+ /* skip to the next interval */
+ log_interval =
(int)ceil(elapsed_sec/log_step_seconds);
+ }
+ }
+
}
if (PQputline(con, "\\.\n"))
{
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgm
Re: [HACKERS] Review of Row Level Security
On 9 December 2012 06:08, Kohei KaiGai wrote: > 2012/12/7 Simon Riggs : >> On 5 December 2012 11:16, Kohei KaiGai wrote: >> * TRUNCATE works, and allows you to remove all rows of a table, even ones you can't see to run a DELETE on. Er... >>> It was my oversight. My preference is to rewrite TRUNCATE command >>> with DELETE statement in case when row-security policy is active on >>> the target table. >>> In this case, a NOTICE message may be helpful for users not to assume >>> the table is always empty after the command. >> >> I think the default must be to throw an ERROR, since part of the >> contract with TRUNCATE is that it is fast and removes storage. >> > OK. Does the default imply you are suggesting configurable > behavior using GUC or something? > I think both of the behaviors are reasonable from security point > of view, as long as user cannot remove unprivileged rows. Hmm, its difficult one that. I guess this raises the question as to whether users know they are accessing a table with RLS enabled. If they don't and we want to keep it that way, then changing TRUNCATE into DELETE makes sense. To issue TRUNCATE you need the correct privilege, which is separate from DELETE. If they have TRUNCATE privilege they should be allowed to remove all rows, bypassing the row level security. If that behavious isn't wanted, then the table owner can create an INSTEAD OF TRUNCATE trigger that turns the action into a DELETE, which is then subject to RLS rules. -- 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] Support for REINDEX CONCURRENTLY
Simon Riggs writes: > On 8 December 2012 15:14, Tom Lane wrote: >> Or we could wait for MVCC catalog access ... > If there was a published design for that, it would help believe in it more. > Do you think one exists? Well, there have been discussion threads about it in the past. I don't recall whether any insoluble issues were raised. I think the concerns were mostly about performance, if we start taking many more snapshots than we have in the past. The basic idea isn't hard: anytime a catalog scan is requested with SnapshotNow, replace that with a freshly taken MVCC snapshot. I think we'd agreed that this could safely be optimized to "only take a new snapshot if any new heavyweight lock has been acquired since the last one". But that'll still be a lot of snapshots, and we know the snapshot-getting code is a bottleneck already. I think the discussions mostly veered off at this point into how to make snapshots cheaper. 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] Review of Row Level Security
On 9 December 2012 06:21, Kohei KaiGai wrote: > 2012/12/7 Simon Riggs : >> On 5 December 2012 11:16, Kohei KaiGai wrote: >> Oracle defaults to putting VPD on all event types: INSERT, UPDATE, DELETE, SELECT. ISTM we should be doing the same, not just say "we can add an INSERT trigger if you want". Adding a trigger just begs the question as to why we are bothering in the first place, since this functionality could already be added by INSERT, UPDATE or DELETE triggers, if they are a full replacement for this feature. The only answer is "ease of use" We can easily add syntax like this [ROW SECURITY CHECK ( ) [ON [ ALL | INSERT, UPDATE, DELETE, SELECT [.., with the default being "ALL" >>> I think it is flaw of Oracle. :-) >> >> Agreed >> >>> In case when user can define leakable function, it enables to leak contents >>> of invisible rows at the timing when executor fetch the rows, prior to >>> modification >>> stage, even if we allows to configure individual row-security policies >>> for SELECT >>> and DELETE or UPDATE commands. >>> My preference is one policy on a particular table for all the commands. >> >> Yes, only one security policy allowed. >> >> Question is, should we offer the option to enforce it on a subset of >> command types. >> >> That isn't anything I can see a need for myself. >> > It is not hard to support a feature not to apply security policy on > particular command types, from implementation perspective. > So, my preference is to support only the behavior corresponding > to above "ALL" option, then support per commands basis when > we got strong demands. > How about your thought? Very much agree that ALL should be the default, and only option for first commit of this feature. -- 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] MySQL search query is not executing in Postgres DB
I am aware that in the case at hand, the call to make_fn_arguments() is with the only possible candidate function, so changing COERCE_IMPLICIT to COERCE_ASSIGNMENT inside of make_fn_arguments() is correct. But I wonder if this may have any unwanted side effects for other code paths to make_fn_arguments(), like from optimizer/util/clauses.c or from parser/parse_oper.c. I'm not saying it does, just asking if that could be the case. Jan -- Anyone who trades liberty for security deserves neither liberty nor security. -- Benjamin Franklin -- 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] Support for REINDEX CONCURRENTLY
On 8 December 2012 15:14, Tom Lane wrote: > Maybe the best way is to admit that we need a short-term exclusive lock > for the swapping step. Which wouldn't be so bad if this is just for the toast index, since in many cases the index itself is completely empty anyway, which must offer opportunities for optimization. > Or we could wait for MVCC catalog access ... If there was a published design for that, it would help believe in it more. Do you think one exists? -- 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] Proof of concept: auto updatable views [Review of Patch]
Thom Brown writes: > One observation: There doesn't appear to be any tab-completion for view > names after DML statement keywords in psql. Might we want to add this? Well, there is, but it only knows about INSTEAD OF trigger cases. I'm tempted to suggest that Query_for_list_of_insertables and friends be simplified to just include all views. 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] Commits 8de72b and 5457a1 (COPY FREEZE)
On 8 December 2012 22:18, Stephen Frost wrote: >> So the committed feature does address the visibility issue. > > Not hardly. It lets a user completely violate the basic rules of the > overall database. The *correct* solution to this problem is to actually > *fix* it, by setting it up such that tables created after a particular > transaction starts aren't visible and similar. Agreed, but that is also be a silent change of current behaviour. But the above will only work for CREATE TABLE, not for TRUNCATE. I've invested a lot of time in trying to improve the situation and investigated many routes forwards, none of them very satisfying. Until someone comes up with a better plan, FREEZE is a pragmatic way forwards that improves things now rather than waiting for the perfect solution. And if we want checksums anytime soon we need ways to ameliorate the effect of hints on checksums, which this does, soemwhat. Better plans, with code, welcome. -- 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] CommitFest #3 and upcoming schedule
On 16 November 2012 07:20, Greg Smith wrote: > Project guidelines now ask each patch submitter to review patches of the > same number and approximate complexity as they submit. If you've submitted > some items to the CommitFest, please look at the open list and try to find > something you can review. The deadline for 9.3 is looming and many patches have not yet been reviewed. I'm sending a public reminder to all patch authors that they should review other people's patches if they expect their own to be reviewed. Simply put, if you don't help each other by reviewing other patches then the inevitable result will be your patch will not be neither reviewed nor committed. PostgreSQL has always maintained high standards and the QA process for all code is for it to be reviewed/discussed prior to commit, which is known as "peer review". The PostgreSQL project is fortunate to have so many keen developers, though for some time now there has been an imbalance between the amount of code to review and the amount of time available to do those reviews. I suggested that we encourage peer review by developers, on the basis of "one patch, one review" as a way of solving the problem. Since many/most people are submitting patches as part of their professional job, this message needs to be passed on to your bosses so they are able to allocate sufficient time for you to do both development *and* peer review. Future planning needs to take into account the time/cost of both of those tasks. Let's bring balance to the situation through our own actions. Please review one patch now for every one you submitted. -- 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] [BUG?] lag of minRecoveryPont in archive recovery
On Thu, Dec 6, 2012 at 1:04 PM, Kyotaro HORIGUCHI wrote: > diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c > index 993bc49..d34ab65 100644 > --- a/src/backend/catalog/storage.c > +++ b/src/backend/catalog/storage.c > @@ -519,6 +519,12 @@ smgr_redo(XLogRecPtr lsn, XLogRecord *record) > visibilitymap_truncate(rel, xlrec->blkno); > > FreeFakeRelcacheEntry(rel); > + > + /* > +* Xlogs before this record is unrepeatable, so winding > +* minRecoveryPoint to here. > +*/ > + XLogFlush(lsn); > } > else > elog(PANIC, "smgr_redo: unknown op code %u", info); I think that minRecoveryPoint should be updated before the data-file changes, so XLogFlush() should be called before smgrtruncate(). No? 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?] lag of minRecoveryPont in archive recovery
On Thu, Dec 6, 2012 at 8:39 PM, Amit Kapila wrote: > On Thursday, December 06, 2012 9:35 AM Kyotaro HORIGUCHI wrote: >> Hello, I have a problem with PostgreSQL 9.2 with Pacemaker. >> >> HA standby sometime failes to start under normal operation. >> >> Testing with a bare replication pair showed that the standby failes >> startup recovery under the operation sequence shown below. 9.3dev too, >> but 9.1 does not have this problem. This problem became apparent by the >> invalid-page check of xlog, but >> 9.1 also has same glitch potentially. >> >> After the investigation, the lag of minRecoveryPoint behind EndRecPtr in >> redo loop seems to be the cause. The lag brings about repetitive redoing >> of unrepeatable xlog sequences such as XLOG_HEAP2_VISIBLE -> >> SMGR_TRUNCATE on the same page. So I did the same aid work as >> xact_redo_commit_internal for smgr_redo. While doing this, I noticed >> that >> CheckRecoveryConsistency() in redo apply loop should be after redoing >> the record, so moved it. > > I think moving CheckRecoveryConsistency() after redo apply loop might cause > a problem. > As currently it is done before recoveryStopsHere() function, which can allow > connections > on HOTSTANDY. But now if due to some reason recovery pauses or stops due to > above function, > connections might not be allowed as CheckRecoveryConsistency() is not > called. Yes, so we should just add the CheckRecoveryConsistency() call after rm_redo rather than moving it? This issue is related to the old discussion: http://archives.postgresql.org/pgsql-bugs/2012-09/msg00101.php 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] pg_basebackup is taking backup of extra files inside a tablespace directory
On Fri, Nov 30, 2012 at 8:30 PM, Robert Haas wrote: > On Wed, Nov 28, 2012 at 1:55 AM, Hari Babu wrote: >> I think backup should be done only files and folders present inside >> '/opt/tblspc/PG_*' directory (TABLESPACE_VERSION_DIRECTORY). >> Not all the files and folders in side '/opt/tblspc.' directory. > > I think I agree. The current behavior would have made sense in older > releases of PG where we plopped down our stuff right inside the > tablespace directory, but now that everything is being shoved in a > subdirectory I don't see a reason to copy anything other than that > subdirectory. Of course it's probably bad style to have anything > that's not related to PG in there, but given that the whole point of > this change was to allow different PG versions to create tablespaces > on the same directory at the same time, we can hardly say that this is > a case that should never arise in real life. Makes sense, yeah. Of course, anything stuffed *inside* said subdir will still be included, but that's violating that principle even more :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Proof of concept: auto updatable views [Review of Patch]
On 8 December 2012 23:29, Tom Lane wrote: > Dean Rasheed writes: > > Attached is a rebased patch using new OIDs. > > Applied after a fair amount of hacking. > One observation: There doesn't appear to be any tab-completion for view names after DML statement keywords in psql. Might we want to add this? -- Thom
Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]
On 8 December 2012 23:29, Tom Lane wrote: > Dean Rasheed writes: >> Attached is a rebased patch using new OIDs. > > Applied after a fair amount of hacking. > Awesome! Thank you very much. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
