Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION
On Mon, Nov 30, 2009 at 12:14 PM, Greg Smith g...@2ndquadrant.com wrote:
Jeff Davis wrote:
COPY target FROM FUNCTION foo() WITH RECORDS;
In what format would the records be?
As a not-quite aside, I think I have a better syntax suggestion. I
have recently been digging around in the grammar with the changes made
in the following commit:
commit a6833fbb85cb5212a9d8085849e7281807f732a6
Author: Tom Lane t...@sss.pgh.pa.us
Date: Mon Sep 21 20:10:21 2009 +
Define a new, more extensible syntax for COPY options.
This is intentionally similar to the recently revised syntax for EXPLAIN
options, ie, (name value, ...). The old syntax is still supported for
backwards compatibility, but we intend that any options added in future
will be provided only in the new syntax.
Robert Haas, Emmanuel Cecchet
As it turns out, the following syntax may work pretty well:
COPY y TO FUNCTION (setup_function the_setup_function('some arg', 3, 7, 42))
Basically the func_expr reduction fits very neatly into the
copy_generic_opt_elem reduction:
copy_generic_opt_elem:
ColLabel copy_generic_opt_arg
{
$$ = (Node *) makeDefElem($1, $2);
}
| ColLabel func_expr
{
$$ = (Node *) $2;
}
;
Now we can use more or less any reasonable number of symbol names and
function calls we desire. This makes life considerably easier, I
think...
We can also try to refactor COPY's internals to take advantage of
these features (and potentially reduce the number of mechanisms. For
example, the legacy COPY ... TO '/place' WITH CSV perhaps can be
more verbosely/generically expressed as:
COPY ... TO FUNCTION (setup_function to_file('/place'),
record_converter csv_formatter,
stream_function fwrite
end_function fclose);
We can also add auxiliary symbols for error handling behavior. For
example, were the COPY to fail for some reason maybe it would make
sense on_error to call unlink to clean up the partially finished
file.
I also have what I think is a somewhat interesting hack. Consider
some of the functions up there without arguments (presumably they'd be
called with a somewhat fixed contract the mechanics of COPY itself):
how does one disambiguate them? Ideally, one could sometimes use
literal arguments (when the return value of that function is desired
to be threaded through the other specified functions) and other times
it'd be nice to disambiguate functions via type names. That would
look something like the following:
COPY ... TO FUNCTION (setup_function to_file('/place'),
record_converter csv_formatter(record),
stream_function fwrite(bytea),
end_function fclose(text));
I think this is possible to implement without much ambiguity, drawing
on the observation that the COPY statement does not have -- and
probably will never have -- references via Var(iable) node, unlike
normal SQL statements such as SELECT, INSERT, et al. That means we
might be able disambiguate using the following rules when scanning the
funcexpr's arguments during the semantic analysis phase to figure out
what to do:
* Only literal list items found: it's a function call with the types
of those literals. Ex: my_setup_function('foo'::text, 3)
* Only non-literal list items found: it's type specifiers. Ex:
csv_formatter(record).
* Both literal and non-literal values found: report an error.
This only works because no cases where a non-literal quantity could be
confused with a type name come to mind. If one could name a type 3
and being forced to double-quote 3 to get your type disambiguated
was just too ugly, then we are at an impasse. But otherwise I think
this may work quite well.
Common constellations of functions could perhaps be bound together
into a DDL to reduce the amount of symbol soup going on here, but that
seems like a pretty clean transition strategy at some later time.
Most of the functionality could still be captured with this simple
approach for now...
Also note that factoring out high-performance implementations of
things like csv_formatter (and friends: pg_binary_formatter) will
probably take some time, but ultimately I think all the existing
functionality could be realized as a layer of syntactic sugar over
this mechanism.
fdr
--
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] add more frame types in window functions (ROWS)
2009/12/5 Andrew Gierth and...@tao11.riddles.org.uk: 1) Memory context handling for aggregate calls aggcontext = AggGetMemoryContext(fcinfo-context); if (!aggcontext) ereport(...); For completeness, there should be two other functions: one to simply return whether we are in fact being called as an aggregate, and another one to return whether it's safe to scribble on the first argument (while it's currently the case that these two are equivalent, it would be good not to assume that). Comments? This is the most significant issue as I see it. Yep, I agree. The most essential point on this is that external functions refer to the struct members directly; we should provide kinds of API. (Also, a function in contrib/tsearch2 that accesses wincontext wasn't updated by the patch.) Thanks for noticing. I didn't know it. 2) Keywords The discussion is: http://archives.postgresql.org/message-id/e08cc0400911241703u4bf53ek1c3910605a3d8...@mail.gmail.com and ideas are extracted from Tom's mail in the last year just before committing the first window function patch, except for changing to COL_NAME_KEYWORD rather than RESERVED_KEYWORD as suggested. The reason I picked it up was only that it works. I cannot tell the side effect of COL_NAME_KEYWORD but I guess we tend to avoid RESERVED_KEYWORD as far as possible. And the reason BETWEEN cannot work as function name any more is based on bison's output shift/reduce conflict when SCONST follows BETWEEN in frame_extent. I don't have clear example that makes this happen, though. 3) Regression tests Testing that views work is OK as far as it goes, but I think that view definition should be left in place rather than dropped (possibly with even more variants) so that the deparse code gets properly tested too. (see the rules test) OK, 4) Deparse output The code is forcing explicit casting on the offset expressions, i.e. the deparsed code looks like ... ROWS BETWEEN 1::bigint PRECEDING AND 1::bigint FOLLOWING ... This looks a bit ugly; is it avoidable? At least for ROWS it should be, I think, since the type is known; even for RANGE, the type would be determined by the sort column. Hmm, I'll change it as LIMIT clause does. Pass false as showimplicit to get_rule_expr() maybe? 5) Documentation issues The entry for BETWEEN in the keywords appendix isn't updated. (Wouldn't it make more sense for this to be generated from the keyword list somehow?) I heard that you don't need to send keywords appendix in the patch because it is auto-generated, if I remember correctly. Spelling: - current row. In literalROWS/ mode this value means phisical row + current row. In literalROWS/ mode this value means physical row (this error appears twice) Oops, thanks. I'm now reworking as reviewed. The last issue is whether we accept extension of frame types without RANGE support. Regards, -- Hitoshi Harada -- 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] Summary and Plan for Hot Standby
On Sun, 2009-11-15 at 16:07 +0200, Heikki Linnakangas wrote: - The assumption that b-tree vacuum records don't need conflict resolution because we did that with the additional cleanup-info record works ATM, but it hinges on the fact that we don't delete any tuples marked as killed while we do the vacuum. That seems like a low-hanging fruit that I'd actually like to do now that I spotted it, but will then need to fix b-tree vacuum records accordingly. You didn't make a change, so I wonder whether you realised no change was required or that you still think change is necessary, but have left it to me? Not sure. I've investigated this but can't see any problem or need for change. btvacuumpage() is very specific about deleting *only* index tuples that have been collected during the VACUUM's heap scan, as identified by the heap callback function, lazy_tid_reaped(). There is no reliance at all on the state of the index tuple. If you ain't on the list, you ain't cleaned. I accept your observation that some additional index tuples may be marked as killed by backends accessing the table that is being vacuumed, since those backends could have a RecentGlobalXmin later than the OldestXmin used by the VACUUM as a result of the change that means GetSnapshotData() ignores lazy vacuums. But those tuples will not be identified by the callback function and so the additionally killed index tuples will not be removed. It is a possible future optimisation of b-tree vacuum that it cleans these additional killed tuples while it executes, but it doesn't do it now and so we need not worry about that for HS. I think its important that we note this assumption though. Comment? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add more frame types in window functions (ROWS)
2009/12/5 Hitoshi Harada umi.tan...@gmail.com: I'm now reworking as reviewed. The last issue is whether we accept extension of frame types without RANGE support. Attached is updated version. I added AggGetMemoryContext() in executor/nodeAgg.h (though I'm not sure where to go...) and its second argument iswindowagg is output parameter to know whether the call context is Agg or WindowAgg. Your proposal of APIs to know whether the function is called as Aggregate or not is also a candidate to be, but it seems out of this patch scope, so it doesn't touch anything. Fix tsearch function is also included, as well as typo phisical - physical. Pass false to get_rule_expr() of value in PRECEDING/FOLLOWING. One thing for rule test, I checked existing regression test cases and concluded DROP VIEW is necessary, or even VIEW test for a specific feature is not needed. I remember your aggregate ORDER BY patch contains rules test changes. However, since processing order of regression tests is not predictable and may change AFAIK, I guess it shouldn't add those changes in rules.out. Regards. -- Hitoshi Harada rows_frame_types.20091205.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add more frame types in window functions (ROWS)
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes: Hitoshi One thing for rule test, I checked existing regression test Hitoshi cases and concluded DROP VIEW is necessary, or even VIEW Hitoshi test for a specific feature is not needed. I remember your Hitoshi aggregate ORDER BY patch contains rules test Hitoshi changes. However, since processing order of regression tests Hitoshi is not predictable and may change AFAIK, I guess it Hitoshi shouldn't add those changes in rules.out. Actually, looking more closely, the way you have it currently works only by chance - rules and window are running in parallel, therefore the view creation in window can break the output of rules. The order of regression tests is set in parallel_schedule and serial_schedule; it's unpredictable only for tests within the same parallel group. I think a modification of the schedule is needed here; the only other option would be to move the view creation into a different test. -- 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] Adding support for SE-Linux security
On Sat, Dec 5, 2009 at 12:14 AM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: Actually, we tried that already, in a previous iteration of this discussion. Someone actually materialized and commented on a few things. The problem, as I remember it, was that they didn't know much about PostgreSQL, so we didn't get very far with it. Unfortunately, I can't find the relevant email thread at the moment. In fact, we've tried about everything with these patches. Tom reviewed them, Bruce reviewed them, Peter reviewed them, I reviewed them, Stephen Frost reviewed them, Heikki took at least a brief look at them, and I think there were a few other people, too. The first person who I can recall being relatively happy with any version of this patch was Stephen Frost, commenting on the access control framework that we suggested KaiGai try to separate from the main body of the patch to break it into more managable chunks. That patch was summarily rejected by Tom for what I believe were valid reasons. In other words, in 18 months of trying we've yet to see something that is close to being committable. Contrast that with Hot Standby, which Heikki made a real shot at committing during the first CommitFest to which it was submitted. I think David Fetter summarized it pretty well here - the rest of the thread is worth reading, too. http://archives.postgresql.org/pgsql-hackers/2009-07/msg01159.php I think the only chance of this ever getting committed is if a committer volunteers to take ownership of it, similar to what Heikki has done for Hot Standby and Streaming Replication. Right now, we don't have any volunteers, and even if Tom or Heikki were interested, I suspect it would occupy their entire attention for several CommitFests just as HS and SR have done for Heikki. I suspect the amount of work for SE-PostgreSQL might even be larger than for HS. If we DON'T have a committer who is willing to own this, then I don't think there's a choice other than giving up. I offered to review it. I was going to mostly review the parts that impacted our existing code, and I wasn't going to be able to do a thorough job of the SE-Linux-specific files. Review it and commit it, after making whatever modifications are necessary? Or review it in part, leaving the final review and commit to someone else? I just read through the latest version of this patch and it does appear to be in significantly better shape than the versions I read back in July. So it might not require a Herculean feat of strength to get this in, but I still think it's going to be a big job. There's a lot of code here that needs to be verified and in some cases probably cleaned up or restructured. If you're prepared to take it on, I'm not going to speak against that, other than to say that I think you have your work cut out for you. ...Robert -- 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] Adding support for SE-Linux security
Robert Haas wrote: I offered to review it. ?I was going to mostly review the parts that impacted our existing code, and I wasn't going to be able to do a thorough job of the SE-Linux-specific files. Review it and commit it, after making whatever modifications are necessary? Or review it in part, leaving the final review and commit to someone else? I just read through the latest version of this patch and it does appear to be in significantly better shape than the versions I read back in July. So it might not require a Herculean feat of strength to get this in, but I still think it's going to be a big job. There's a lot of code here that needs to be verified and in some cases probably cleaned up or restructured. If you're prepared to take it on, I'm not going to speak against that, other than to say that I think you have your work cut out for you. This is no harder than many of the other seemingly crazy things I have done, e.g. Win32 port, client library threading. If this is a feature we should have, I will get it done or get others to help me complete the task. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] First feature patch for plperl - draft [PATCH]
On Sat, Dec 05, 2009 at 01:21:22AM -0500, Tom Lane wrote: David E. Wheeler da...@kineticode.com writes: Tom, what's your objection to Shlib load time being user-visible? It's not really designed to be user-visible. Let me give you just two examples: * We call a plperl function for the first time in a session, causing plperl.so to be loaded. Later the transaction fails and is rolled back. If loading plperl.so caused some user-visible things to happen, should those be rolled back? No. Establishing initial state, no matter how that's triggered, is not part of a transaction. * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) I'll modify the patch to disable the SPI functions during initialization (both on_perl_init and on_(un)trusted_init). Would that address your concerns? That doesn't even begin to cover the problems with allowing any of this to happen inside the postmaster. Recall that the postmaster does not have any database access. Furthermore, it is a very long established reliability principle around here that the postmaster process should do as little as possible, because every thing that it does creates another opportunity to have a nonrecoverable failure. The postmaster can recover if a child crashes, but the other way round, not so much. I hope the combination of disabling the SPI functions during initialization, and documenting the risks of combining on_perl_init and shared_preload_libraries, is sufficient. Tim. -- 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] [GENERAL] pg_attribute.attnum - wrong column ordinal?
On tor, 2009-12-03 at 10:09 -0500, Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Should we recast the attributes and columns views in information_schema? I notice they still use attnum. I'd vote against it, at least until we have something better than a row_number solution. That would create another huge performance penalty on views that are already ungodly slow. Should be easy to test the performance impact of this, since the limit for columns per table is 1600. -- 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] PostgreSQL Release Support Policy
On Fri, 2009-12-04 at 16:36 +, Dave Page wrote: End Of Life (EOL) dates: Version EOL Date PostgreSQL 7.4July 2010 (extended) PostgreSQL 8.0July 2010 (extended) PostgreSQL 8.1November 2010 PostgreSQL 8.2December 2011 PostgreSQL 8.3February 2013 PostgreSQL 8.4July 2014 Could I request we change these dates slightly to PostgreSQL 8.1 February 2011 PostgreSQL 8.2 February 2012 with absolutely no intention to extend the support window any worthwhile amount. That way we have * A regular pattern of de-release, so everybody is clearer, i.e. PostgreSQL 8.1 February 2011 PostgreSQL 8.2 February 2012 PostgreSQL 8.3 February 2013 * We don't have de-support right at Thanksgiving or Christmas, since people may do panic-stricken upgrades. I much prefer February as a time for panic, if that choice is available, which I realise it may not be. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] First feature patch for plperl - draft [PATCH]
Tim Bunce tim.bu...@pobox.com writes: I'll modify the patch to disable the SPI functions during initialization (both on_perl_init and on_(un)trusted_init). Yeah, in the shower this morning I was thinking that not loading SPI till after the on_init code runs would alleviate the concerns about transactionality and permissions --- that would ensure that whatever on_init does affects only the Perl world and not the database world. However, we're not out of the woods yet. In a trusted interpreter (plperl not plperlu), is the on_init code executed before we lock down the interpreter with Safe? I would think it has to be since the main point AFAICS is to let you preload code via use. But then what is left of the security guarantees of plperl? I can hardly imagine DBAs wanting to vet a few thousand lines of random Perl code to see if it contains anything that could be subverted. For example, the ability to scribble on database files (like say pg_hba.conf) would almost surely be easy to come by. If you're willing to also confine the feature to plperlu, then maybe the risk level could be decreased from insane to merely unreasonable. 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] First feature patch for plperl - draft [PATCH]
Tim Bunce wrote: That doesn't even begin to cover the problems with allowing any of this to happen inside the postmaster. Recall that the postmaster does not have any database access. Furthermore, it is a very long established reliability principle around here that the postmaster process should do as little as possible, because every thing that it does creates another opportunity to have a nonrecoverable failure. The postmaster can recover if a child crashes, but the other way round, not so much. I hope the combination of disabling the SPI functions during initialization, and documenting the risks of combining on_perl_init and shared_preload_libraries, is sufficient. We already do a lot during library load - plperl's _PG_init() calls plperl_init_interp() which sets up an interpreter, runs the boot code, loads the Dynaloader and bootstraps the SPI module. Pre-loading perl libraries in forking servers has well known benefits, as Robert Haas noted. We're not talking about touching the database at all. If we turn Tim's proposal down, I suspect someone will create a fork of plperl that allows it anyway - it's not like it needs anything changed elsewhere in the backend - it would be a drop-in replacement, pretty much. Here's a concrete example of something I was working on just yesterday, where it would be useful. One of my clients has a Postgres based application that needs to talk to a number of foreign databases, mostly SQLServer. In some cases it pulls data from them, in this new case we are pushing lots of data at arbitrary times into SQLServer, using plperlu with DBI/DBD::Sybase. We would probably get a significant performance gain if we could have DBI and DBD::Sybase preloaded. The application does use connection pooling, but every so often a function call will take significantly longer because it occurs in a new backend that is having to reload the libraries. I think if we do this the on_perl_init setting should probably be PGC_POSTMASTER, which would remove any issue about it changing underneath us. 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] First feature patch for plperl - draft [PATCH]
Andrew Dunstan and...@dunslane.net writes: If we turn Tim's proposal down, I suspect someone will create a fork of plperl that allows it anyway - it's not like it needs anything changed elsewhere in the backend - it would be a drop-in replacement, pretty much. The question is not about whether we think it's useful; the question is about whether it's safe. I think if we do this the on_perl_init setting should probably be PGC_POSTMASTER, which would remove any issue about it changing underneath us. Yes, if the main intended usage is in combination with preloading perl at postmaster start, it would be pointless to imagine that PGC_SIGHUP is useful anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, misc issues
On Fri, 2009-12-04 at 10:23 +0200, Heikki Linnakangas wrote: @Heikki: Why is error checking in KnownAssignedXidsRemove() #ifdef'd out?? It's explained in the comment: /* XXX: This can still happen: If a transaction with a subtransaction * that haven't been reported yet aborts, and no WAL records have been * written using the subxid, the abort record will contain that subxid * and we haven't seen it before. */ Just realised that this occurs again because the call to RecordKnownAssignedTransactionIds() was removed from xact_commit_abort(). I'm guessing you didn't like the call in that place for some reason, since I smile while I remember it has been removed twice(!) even though I put do not remove comments on it to describe this corner case. Not going to put it back a third time. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Reading recovery.conf earlier
I'm planning to read recovery.conf earlier in the startup process, so we can make a few things more recovery aware. It's a nice-to-have only. This won't be part of the HS patch though. Proposal is to split out the couple of lines in readRecoveryCommandFile() that set important state and make it read in an option block that can be used by caller. It would then be called by both postmaster (earlier in startup) and again later by startup process, as happens now. I want to do it that way so I can read file before we create shared memory, so I don't have to worry about passing details via shared memory itself. It will allow some tidy up in HS patch but isn't going to be intrusive. Not thinking about lexers and stuff though at this stage, even if it is on the todo list. Any vetos? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] First feature patch for plperl - draft [PATCH]
On Sat, Dec 05, 2009 at 11:41:36AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I'll modify the patch to disable the SPI functions during initialization (both on_perl_init and on_(un)trusted_init). Yeah, in the shower this morning I was thinking that not loading SPI till after the on_init code runs would alleviate the concerns about transactionality and permissions --- that would ensure that whatever on_init does affects only the Perl world and not the database world. However, we're not out of the woods yet. In a trusted interpreter (plperl not plperlu), is the on_init code executed before we lock down the interpreter with Safe? The on_perl_init code (PGC_SUSET) is run before Safe is loaded. The on_trusted_init code (PGC_USERSET) is run inside Safe. I would think it has to be since the main point AFAICS is to let you preload code via use. The main use case being targeted at the moment for on_trusted_init is setting values in %_SHARED, perhaps to enable debugging. Inside Safe you'll only be able to 'use' modules that have already been loaded inside Safe. In my draft patch that's currently just strict and warnings. (I am also adding an interface to enable DBAs to configure what gets loaded into the Safe compartment and what gets shared with it. That'll be the way extra modules can be used by plperl. It'll be used via on_perl_init so be controlled via the DBA.) I can hardly imagine DBAs wanting to vet a few thousand lines of random Perl code to see if it contains anything that could be subverted. For example, the ability to scribble on database files (like say pg_hba.conf) would almost surely be easy to come by. It's surely better to give the DBA that option than to remove the choice entirely. If you're willing to also confine the feature to plperlu, then maybe the risk level could be decreased from insane to merely unreasonable. I believe I can arrange for the SPI functions to be disabled during on_*_init for both plperl and plperlu. Hopefully then the default risk level will be better than unreasonable :) Tim. -- 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] Summary and Plan for Hot Standby
Simon Riggs wrote: On Sun, 2009-11-15 at 16:07 +0200, Heikki Linnakangas wrote: - The assumption that b-tree vacuum records don't need conflict resolution because we did that with the additional cleanup-info record works ATM, but it hinges on the fact that we don't delete any tuples marked as killed while we do the vacuum. That seems like a low-hanging fruit that I'd actually like to do now that I spotted it, but will then need to fix b-tree vacuum records accordingly. You didn't make a change, so I wonder whether you realised no change was required or that you still think change is necessary, but have left it to me? Not sure. I've investigated this but can't see any problem or need for change. Sorry if I was unclear: it works as it is. But *if* we change the b-tree vacuum to also delete index tuples marked with LP_DEAD, we have a problem. I think its important that we note this assumption though. Yeah, a comment is in order. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] First feature patch for plperl - draft [PATCH]
Tom Lane escribió: David E. Wheeler da...@kineticode.com writes: Tom, what's your objection to Shlib load time being user-visible? It's not really designed to be user-visible. Let me give you just two examples: * We call a plperl function for the first time in a session, causing plperl.so to be loaded. Later the transaction fails and is rolled back. I don't think there's any way for this to work sanely unless the library has been loaded previously. What about allowing those settings only if plperl is specified in shared_preload_libraries? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New VACUUM FULL
On Fri, 2009-12-04 at 19:49 -0500, Michael Glaesemann wrote: I tested a variety of situations during my review, and everything worked as I expected. Would there be a way for you to package the scenarios you tested into a suite? Except for the simplest tests, they aren't easily moved to pg_regress. For instance, how do you tell if a user table's relfilenode actually changed? Easy to do manually, but tough to do with pg_regress. Regards, Jeff Davis -- 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] Cancelling idle in transaction state
I'd be very grateful to any hackers out there who are looking for a project before close of 8.5 to consider working on this. It's dang useful, both for Hot Standby and normal processing. (You'll have the added bonus of proving you're smarter than me!) On Wed, 2009-01-21 at 15:22 -0500, Bruce Momjian wrote: Added to TODO: Allow administrators to cancel multi-statement idle transactions This allows locks to be released, but it is complex to report the cancellation back to the client. * http://archives.postgresql.org/pgsql-hackers/2008-12/msg01340.php --- Simon Riggs wrote: Currently SIGINT is ignored during IDLE in transaction, but we have recently agreed to allow this to cancel the transaction. We said we would do this in all cases, so this is a separate feature/patch (though Hot Standby requires it). A simple change allows the transaction to be cancelled, but there are some loose ends that I wish to discuss. If we are running a statement and a cancel is received, then we return the ERROR to the client, who is expecting it. If we cancel a transaction while the connection is idle, we have no way of signalling to the client program this has occurred. So the client finds out about this much later, not in fact until the next message is sent. Is there a mechanism for communicating the state back to the client? Will this be handled correctly with existing code? psql appears to be confused by a cancelled backend. I'm not familiar with these aspects of the code, so some clear suggestions are needed to allow me to work this out. I'm worried that this will delay things further otherwise. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Adding support for SE-Linux security
Robert Haas wrote: On Thu, Dec 3, 2009 at 5:23 PM, Josh Berkus j...@agliodbs.com wrote: Kaigai, you've said that you could get SELinux folks involved in the patch review. I think it's past time that they were; please solicit them. Actually, we tried that already, in a previous iteration of this discussion. Someone actually materialized and commented on a few things. The problem, as I remember it, was that they didn't know much about PostgreSQL, so we didn't get very far with it. Unfortunately, I can't find the relevant email thread at the moment. IIRC, at least a couple of the guys mentioned on the NSA's SE-Linux page[1] participated - Joshua Brindle[2] and Chad Sellers[3] (in addition to Kaigai/NEC who's credited on the NSA site as well). Perhaps one or two others too - but with common names it's hard to guess. Links to the threads with Chad and Joshua below. [1] http://www.nsa.gov/research/selinux/contrib.shtml [2] http://www.google.com/search?q=site%3Aarchives.postgresql.org+brindle [3] http://www.google.com/search?q=site%3Aarchives.postgresql.org+chad+sellers -- 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] YAML Was: CommitFest status/management
Josh Berkus wrote: ... YAML for easier readability ... almost as good ... I agree with Kevin that it's more readable. Again, if there were a sensible way to do YAML as a contrib module, I'd go for that, but there isn't. Perhaps that's be a direction this could take? What would it take for this feature to be a demo/example for some future modules system. It seems like there have been a few recent features related to decorating output (UTF8 tables, YAML explain, \d... updates). While there's no great way to make this a contrib module today, would it make sense to add such hooks for an eventual module system? -- 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] PostgreSQL Release Support Policy
Simon Riggs si...@2ndquadrant.com writes: Could I request we change these dates slightly to The intent of the policy is that the last formal minor release for a branch will happen no earlier than the specified times. It might well be later, since we're not going to schedule updates specially for this --- it'd be whenever the next set of updates occur, and that would more likely be driven by bugs in newer branches, not the oldest one. In any case there's no need for someone to move off a version instantly the day after the last release for it. So I really don't see why you think there would be panic updates. There's so much fuzz in when a version would be effectively dead that a few months either way in the nominal date wouldn't make any difference anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reading recovery.conf earlier
Simon Riggs si...@2ndquadrant.com writes: I'm planning to read recovery.conf earlier in the startup process, so we can make a few things more recovery aware. It's a nice-to-have only. Say what? It's read at the beginning already. 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] Hot standby, misc issues
Simon Riggs wrote: On Fri, 2009-12-04 at 10:23 +0200, Heikki Linnakangas wrote: @Heikki: Why is error checking in KnownAssignedXidsRemove() #ifdef'd out?? It's explained in the comment: /* XXX: This can still happen: If a transaction with a subtransaction * that haven't been reported yet aborts, and no WAL records have been * written using the subxid, the abort record will contain that subxid * and we haven't seen it before. */ Just realised that this occurs again because the call to RecordKnownAssignedTransactionIds() was removed from xact_commit_abort(). I'm guessing you didn't like the call in that place for some reason, since I smile while I remember it has been removed twice(!) even though I put do not remove comments on it to describe this corner case. Not going to put it back a third time. :-). Well, it does seem pointless to add entries to the hash table, just to remove them at the very next line. But you're right, we should still advance latestObservedXid, and if we do that, we need to memorize any not-yet-seen XIDs in the known-assigned xids array. So that RecordKnownAssignedTransactionIds() call needs to be put back. BTW, if you want to resurrect the check in KnownAssignedXidsRemove(), you also need to not complain before you reach the running-xacts record and open up for read-only connections. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Reading recovery.conf earlier
On Sat, 2009-12-05 at 15:43 -0500, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: I'm planning to read recovery.conf earlier in the startup process, so we can make a few things more recovery aware. It's a nice-to-have only. Say what? It's read at the beginning already. Before the beginning then. :-) Two reasons to move it earlier in the startup sequence of the server * Some data structures are only required in HS mode. We don't know we're in HS mode until we created shared memory and started the Startup process. If we knew ahead of time, we could skip adding the structures. * Some things in postgresql.conf need to be overridden in HS mode, for example default_transaction_read_only = false. Again, we don't know we're in HS mode until later. So I would want to read recovery.conf before we read postgresql.conf Also, AFAIK, it's not easily possible to have dependencies between settings in postgresql.conf, unless the dependencies are expressed by ordering the dependent parameters in alphabetic order. So putting the parameters in postgresql.conf wouldn't help much. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL Release Support Policy
On Sat, 2009-12-05 at 15:33 -0500, Tom Lane wrote: In any case there's no need for someone to move off a version instantly the day after the last release for it. So I really don't see why you think there would be panic updates. Hmm, well, I wasn't imagining it as a wholly rational response. I guess the existence of such a panic remains to be proven amongst our users, who I should give more credit to than their counterparts that still use other products. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] operator exclusion constraints
On Fri, Dec 04, 2009 at 11:35:52AM -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Dec 3, 2009 at 7:42 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. �I thought the consensus was to refer to them as just exclusion constraints? �I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) Well, unique constraints are tied to the opclass/index machinery too. Unless there's loud squawks I'm going to exercise committer's prerogative and make all the docs and messages just say exclusion constraint. We have constraint exclusion already, which could make this confusing. How about either the original operator exclusion constraint or the slightly easier whatever constraint names? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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 for information_schema performance
On fre, 2009-09-25 at 00:55 +0200, Joachim Wieland wrote: the attached patch addresses the performance issues of the authorization related views from information_schema (BUG #4596). It implements what Tom suggests in http://archives.postgresql.org/pgsql-bugs/2008-12/msg00144.php In the cases that I have tested both the new and the old view return the same data but I'd appreciate more tests. The patch currently does not remove the original views but renames them from xyz to old_xyz, so you can run your own tests of the new view definition vs. the old one. Committed with a bunch of cleanup. -- 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] YAML Was: CommitFest status/management
Ron Mayer escreveu: While there's no great way to make this a contrib module today, would it make sense to add such hooks for an eventual module system? I don't think so. It's easier to code a converter tool. -- Euler Taveira de Oliveira http://www.timbira.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] pgbench: new feature allowing to launch shell commands
Michael Paquier wrote: 3) Should consider how :variable interpretation should work in a \[set]shell call It is supported now. I implemented this, I made a test with your perl script, my own tests and it worked, at least no error appeared :) It looks like how you did this is a little less complicated than I had hoped for. Let me show you some examples of how I think this should work. Say naccounts = 100 and bid=123 already: \setshell aid skew.sh :naccounts runs skew.sh 100 \setshell aid skew.sh a ::naccounts c runs skew.sh a :naccounts c - do not substitute the variable if :: appears, just replace with :. Otherwise, there's no way to include a real : in the command line \setshell aid skew.h aid :naccounts :bid runs shew.sh 100 123 From looking at the code, I think that only case (1) is supported by the bits you added (haven't actually re-tested it since I know you're still working). Also, having that same substitution logic work for both shell and setshell should would be nice here. As far as reducing the amount of stuff in doCustom goes, I think what you want for this specific part is a subroutine you can pass a string that has some number of :variable references in it, returning a new string with them having the substituted values inserted in there. That's something I think it would be easier to get right as a standalone function first, and then both shell and setshell could use it. Do you have an idea of what kind of tests could be done? I don't have so much experience about common regression tests linked to pgbench. None of the regression tests use pgbench yet, partly because contrib modules like it aren't necessarily even built before the main regression tests are run. Also, it's hard to write a pgbench-based test using the current pg_regress framework. The complete non-determinism on the TPS scores for example makes it hard to avoid having a diff against any standard regression result provided. I think it would be nice to add a very complicated script that uses all these weird features for regression test purposes, but it's not so clear how we would enforce running it automatically to catch if a future change broke something. As far as the rest of your concerns, once we get this to code complete and all the bugs squashed, I can take a pass at cleaning up your docs and making sure there's not any performance regression as part of my final review. What you've added in there so far is good enough for now, I just didn't want to do the docs changes from scratch myself (and I thought it would be useful for you to get a bit of practice on that too, since they're usually required for patch submissions). If you can make the three examples above all work, and get rid of the threading bug, I'll be clear to take care of docs review/performanc tests and then pass this over for a committer to look at. It would be nice if the code was refactored a bit too first, just because it's less likely to be rejected for the code is ugly reasons if that's done first. That sort of rejection is always a real possibility with this project, particularly for something like this where it's not as obvious to everyone what the feature is useful for. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cancelling idle in transaction state
On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: This allows locks to be released, but it is complex to report the cancellation back to the client. I think the answer here is that the server should *not* report the cancellation. Rather, it should mark the transaction as failed and let the client eventually sync its state on subsequent requests that will result in InFailedTransaction ERRORs. With such a solution, COMMITs issued to administrator cancelled transactions should result in an ERROR. Well, I suppose that would only be a requirement when: BEGIN; ... some work ... idle admin zapped this transaction more idle COMMIT; -- client needs to know that this failed, and it should be something louder than a ROLLBACK tag. :P So, if a command were issued to a cancelled transaction prior to a COMMIT: BEGIN; ... some work ... idle admin zapped this transaction SELECT * FROM something; -- fails, IFX ERROR emitted to client COMMIT; -- client was already notified of the xact failure by a prior command's error, so the normal ROLLBACK would be fine. Also, if immediate notification is seen as a necessity, a WARNING with a special code could be leveraged. Oh, or maybe use a dedicated LISTEN/NOTIFY channel? LISTEN pg_darn_admin_zapped_my_xact; to opt-in for transaction cancellation events that occur in *this* backend.. [Note: this is in addition to COMMITs emitting ERRORs] I can't see immediate notification being useful excepting some rather strange situations where the client left the transaction idle to go do other expensive operations that should be immediately interrupted if this particular transaction were to be cancelled for some reason.. Such a situation might even make sense if those expensive operations somehow depended on the locks held by the transaction, but I think that's a stretch. Not to mention that the client could just occasionally poll the transaction with 'SELECT 1's; no special WARNING or NOTIFY's would be necessary. -- 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] Block-level CRC checks
It can save space because the line pointers have less alignment requirements. But I don't see any point in the current state. -- Greg On 2009-12-04, at 3:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: Greg Stark gsst...@mit.edu writes: I'm not sure why I said including ctid. We would have to move everything transactional to the line pointer, including xmin, xmax, ctid, all the hint bits, the updated flags, hot flags, etc. The only things left in the tuple header would be things that have to be there such as HAS_OIDS, HAS_NULLS, natts, hoff, etc. It would be a pretty drastic change, though a fairly logical one. I recall someone actually submitted a patch to separate out the transactional bits anyways a while back, just to save a few bytes in in-memory tuples. If we could save on disk-space usage it would be a lot more compelling. But it doesn't look to me like it really saves enough often enough to be worth so much code churn. It would also break things for indexes, which don't need all that stuff in their line pointers. More to the point, moving the same bits to someplace else on the page doesn't save anything at all. 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] Cancelling idle in transaction state
On Sat, Dec 5, 2009 at 8:13 PM, James Pye li...@jwp.name wrote: I think the answer here is that the server should *not* report the cancellation. Rather, it should mark the transaction as failed and let the client eventually sync its state on subsequent requests that will result in InFailedTransaction ERRORs. [...] Also, if immediate notification is seen as a necessity, a WARNING with a special code could be leveraged. Oh, or maybe use a dedicated LISTEN/NOTIFY channel? LISTEN pg_darn_admin_zapped_my_xact; to opt-in for transaction cancellation events that occur in *this* backend.. [Note: this is in addition to COMMITs emitting ERRORs] I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. Currently, it appears that the only messages that the server can send back asynchronously are ParameterStatus and NotificationResponse. So we need to decide whether it's feasible/better to shoehorn this functionality into one of those message types, or whether we should bump the protocol version and add a new message type (cue: panic in the streets). On first examination (and I am not an expert in this area), ParameterStatus would seem to be the better choice, because it appears to me that all clients must be prepared to cope with such messages, whereas in theory a client might be unprepared for a NotificationResponse if it never executes LISTEN. (It seems clearly preferable not to require clients to issue an explicit LISTEN in order to enable this feature.) Going with that theory, we could pick a magical parameter status value, either something like __transaction_cancelled, or maybe even something that contains a character that isn't even legal in a normal parameter, like $transaction_cancelled, if we don't think that will break any clients. Then we could just report a value change for this whenever an idle transaction is cancelled. Clients who ignore this will find out when they next issue a query; others will know immediately. ...Robert -- 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] Cancelling idle in transaction state
Robert Haas robertmh...@gmail.com writes: I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. If you must have that, send a NOTICE. I don't actually see the point though. If the client was as smart and well-coded as all that, it wouldn't be sitting on an open transaction in the first place. Currently, it appears that the only messages that the server can send back asynchronously are ParameterStatus and NotificationResponse. Using either of those is completely inappropriate. 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] Cancelling idle in transaction state
On Sat, Dec 5, 2009 at 10:15 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I think this line of thinking is on the right track. The server should certainly not send back an immediate ERROR response, because that will definitely confuse the client. Of course, any subsequent commands will report ERRORs until the client rolls back. But it also seems highly desirable for the server to send some sort of immediate, asynchronous notification, so that a sufficiently smart client can immediately report the error back to the user or take such other action as it deems appropriate. If you must have that, send a NOTICE. Ah ha! I missed that one. That's perfect. I don't actually see the point though. If the client was as smart and well-coded as all that, it wouldn't be sitting on an open transaction in the first place. Think about an interactive client. It's not the client's fault that the user has chosen to begin a transaction and then sit there cogitating, but the client would like to let the user know right away that their current transaction is defunct. ...Robert -- 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] Cancelling idle in transaction state
On Sat, 2009-12-05 at 18:13 -0700, James Pye wrote: On Dec 5, 2009, at 12:25 PM, Simon Riggs wrote: ... I'm not volunteering here, but having worked with the protocol, I do have a suggestion: Thanks. Looks like good input. With the further clarification that we use NOTIFY it seems a solution is forming. Any other takers? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
