Re: [HACKERS] CHECK NO INHERIT syntax
Excerpts from Tom Lane's message of vie jul 20 16:12:05 -0400 2012:
>
> Alvaro Herrera writes:
> > True. I have added an error check at creation time. Please suggest
> > improved wording for the message:
>
> > alvherre=# create domain positiveint2 as int check (value > 0) no inherit;
> > ERROR: CHECK constraints for domains cannot be NO INHERIT
>
> I think "CHECK constraints for domains cannot be marked NO INHERIT"
> would be fine.
Thanks.
> > ConstraintElem:
> > -CHECK opt_no_inherit '(' a_expr ')' ConstraintAttributeSpec
> > +CHECK '(' a_expr ')' opt_no_inherit ConstraintAttributeSpec
>
> This doesn't seem to me to meet the principle of least surprise. Surely
> NO INHERIT ought to be folded into ConstraintAttributeSpec so that it
> acts like other constraint decorations, ie order isn't significant.
Oh, true; that's a bit more involved. I verified it works correctly to
have a constraint marked NOT VALID NO INHERIT or the other way around.
I haven't checked whether the changes to ConstraintAttributeSpec have
side effects -- I think it's OK but I might be missing something.
Here's a (hopefully) complete patch.
--
Álvaro Herrera
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
check-no-inherit-2.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Resetting libpq connections after an app error
Hello,
apologize for bumping the question to -hackers but I got no answer
from -general. If there is a better ML to post it let me know.
In a libpq application, if there is an application error between
PQsendQuery and PQgetResult, is there a way to revert a connection
back to an usable state (i.e. from transaction status ACTIVE to IDLE)
without using the network in a blocking way? We are currently doing
while (NULL != (res = PQgetResult(conn->pgconn))) {
PQclear(res);
}
but this is blocking, and if the error had been caused by the network
down, we'll just get stuck in a poll() waiting for a timeout.
Thank you very much.
-- Daniele
--
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] Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
On Fri, Jul 20, 2012 at 3:45 PM, Tom Lane wrote: > Robert Haas writes: >> I don't particularly care for that solution; it seems like a kludge. >> I've kind of wondered whether we ought to have checks in all the ALTER >> routines that spit up if you try to ALTER an extension member from any >> place other than an extension upgrade script... but that still >> wouldn't prevent the extension owner from dropping the members out of >> the extension and then modifying them afterwards. I'm not sure we >> want to prevent that in general, but maybe there could be some >> locked-down mode that has that effect. > > Right, I wasn't too clear about that, but I meant that we'd have some > sort of locked-down state for an extension that would forbid fooling > with its contents. For development purposes, or for anybody that "knows > what they're doing", adding/subtracting/modifying member objects is > mighty handy. But a non-superuser who's loaded an extension that > contains C functions ought not have those privileges for it. I could see having such a mode. I'm not sure that it would eliminate people's desire to manually give away functions, though. In fact, thinking about a couple of our customers, I'm pretty sure it wouldn't. Now whether it's a good idea is another question, but... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Remove prepared transactions from main isolation test schedule.
On 07/20/2012 04:17 PM, Tom Lane wrote: Andrew Dunstan writes: Remove prepared transactions from main isolation test schedule. There is no point in running this test when prepared transactions are disabled, which is the default. New make targets that include the test are provided. This will save some useless waste of cycles on buildfarm machines. Having done that, shouldn't we remove prepared-transactions_1.out (the "expected" file for the prepared-transactions-disabled case)? Having a megabyte-sized test file for that case has always seemed pretty wasteful to me. Now that the test won't be run unless intentionally selected, it seems like people who are using it would expect it to actually test prepared transactions --- so having it "pass" when they're disabled seems wrong. Good point. Will do. 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] [COMMITTERS] pgsql: Remove prepared transactions from main isolation test schedule.
Andrew Dunstan writes: > Remove prepared transactions from main isolation test schedule. > There is no point in running this test when prepared transactions are > disabled, > which is the default. New make targets that include the test are provided. > This > will save some useless waste of cycles on buildfarm machines. Having done that, shouldn't we remove prepared-transactions_1.out (the "expected" file for the prepared-transactions-disabled case)? Having a megabyte-sized test file for that case has always seemed pretty wasteful to me. Now that the test won't be run unless intentionally selected, it seems like people who are using it would expect it to actually test prepared transactions --- so having it "pass" when they're disabled seems wrong. 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] CHECK NO INHERIT syntax
Alvaro Herrera writes:
> True. I have added an error check at creation time. Please suggest
> improved wording for the message:
> alvherre=# create domain positiveint2 as int check (value > 0) no inherit;
> ERROR: CHECK constraints for domains cannot be NO INHERIT
I think "CHECK constraints for domains cannot be marked NO INHERIT"
would be fine.
> ConstraintElem:
> - CHECK opt_no_inherit '(' a_expr ')'
> ConstraintAttributeSpec
> + CHECK '(' a_expr ')' opt_no_inherit
> ConstraintAttributeSpec
This doesn't seem to me to meet the principle of least surprise. Surely
NO INHERIT ought to be folded into ConstraintAttributeSpec so that it
acts like other constraint decorations, ie order isn't significant.
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] CHECK NO INHERIT syntax
Excerpts from Peter Eisentraut's message of mié jul 18 17:49:37 -0400 2012: > Sorry to raise this once again, but I still find this CHECK NO INHERIT > syntax to a bit funny. We are currently using something like > > CHECK NO INHERIT (foo > 0) > > But we already have a different syntax for attaching attributes to > constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more > sense to have > > CHECK (foo > 0) NO INHERIT Okay, given the astounding acceptance of your proposal, the attached patch fixes things in that way. This only include changes to the core code; I'll prepare documentation and regression tests tweaks while I wait for an answer to the request below. > There is also a hole in the current implementation. Domain constraints > silently allow NO INHERIT to be specified, even though other senseless > attributes are rejected. True. I have added an error check at creation time. Please suggest improved wording for the message: alvherre=# create domain positiveint2 as int check (value > 0) no inherit; ERROR: CHECK constraints for domains cannot be NO INHERIT -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support check-no-inherit.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Event Triggers reduced, v1
On Fri, Jul 20, 2012 at 1:22 PM, Thom Brown wrote: > I've just run my own set of tests against these changes, which tests > every supported DDL command (with the exception of "CREATE USER > MAPPING", "ALTER USER MAPPING", "DROP USER MAPPING", "CREATE LANGUAGE" > and "DROP LANGUAGE"), and many variants of each command, and > everything behaves exactly as expected. :) Nice! But all of those commands you just mentioned ought to work, too. The documentation probably needs some updating on that point, come to think of it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Event Triggers reduced, v1
On Fri, Jul 20, 2012 at 2:12 PM, Tom Lane wrote: > The Windows buildfarm members don't seem too happy with the latest > patch. I'm looking at this now, but am so far mystified. Something's obviously broken as regards how the trigger flags get set up, but if that were broken in a trivial way it would be broken everywhere, and it's not. Will keep searching... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
Robert Haas writes: > I don't particularly care for that solution; it seems like a kludge. > I've kind of wondered whether we ought to have checks in all the ALTER > routines that spit up if you try to ALTER an extension member from any > place other than an extension upgrade script... but that still > wouldn't prevent the extension owner from dropping the members out of > the extension and then modifying them afterwards. I'm not sure we > want to prevent that in general, but maybe there could be some > locked-down mode that has that effect. Right, I wasn't too clear about that, but I meant that we'd have some sort of locked-down state for an extension that would forbid fooling with its contents. For development purposes, or for anybody that "knows what they're doing", adding/subtracting/modifying member objects is mighty handy. But a non-superuser who's loaded an extension that contains C functions ought not have those privileges for it. 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] Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
On Fri, Jul 20, 2012 at 1:52 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane wrote: >>> Yeah, the just-code-defensively option is worth considering too. > >> After rereading this thread, I think I agree with Kevin as well. ... >> Having said that, I do believe that answer is to some extent a >> cop-out. > > I agree with that --- doing nothing at all doesn't seem like the best > option here. > >> ... on the flip side, the C code could - not to >> put too fine a point on it - be relying on just about anything. > > And with that too. The STRICT option is a fairly obvious security > hazard, but who's to say there are not others? I think it'd be easier > to make a case for forbidding a non-superuser from altering *any* > property of a C function. I'd rather start from the point of allowing > only what is clearly safe than disallowing only what is clearly unsafe. That seems like a fairly drastic overreaction. Are you going to ban renaming it or changing the owner, which are in completely different code paths? Yuck. Even if you only ban it for the main ALTER FUNCTION code path, it seems pretty draconian, because it looks to me like nearly everything else that's there is perfectly safe. I mean, assuming the guy who wrote the C code didn't do anything completely insane or malicious, setting GUCs or whatever should be perfectly OK. Honestly, if you want to change something in the code, I'm not too convinced that there's anything better than what Noah proposed originally. > Taking a step or two back, I think that the real use-case we should > be considering here is allowing non-superusers to own (or at least > install) extensions that contain C functions. We would probably want > the non-superuser to be able to drop the extension again, maybe > ALTER EXTENSION SET SCHEMA, maybe ALTER EXTENSION OWNER ... and likely > not too darn much else. Fooling with any of the contained objects > doesn't seem like something we want to permit, since it's likely that > something like a datatype is going to have dependencies on not just > specific objects' properties but their interrelationships. Moreover, it breaks dump-and-restore. > One possible approach to that is to say that the nominal owner of such > an extension only owns the extension itself, and ownership of the > contained objects is held by, say, the bootstrap superuser. There are > other ways too of course, but this way would bypass the problem of > figuring out how to restrict what an object's nominal owner can do > to it. I don't particularly care for that solution; it seems like a kludge. I've kind of wondered whether we ought to have checks in all the ALTER routines that spit up if you try to ALTER an extension member from any place other than an extension upgrade script... but that still wouldn't prevent the extension owner from dropping the members out of the extension and then modifying them afterwards. I'm not sure we want to prevent that in general, but maybe there could be some locked-down mode that has that effect. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] isolation check takes a long time
On 07/20/2012 01:56 PM, Tom Lane wrote: I'm not thrilled with replicating the test-list file either. But it is not necessary: look at the way the "bigtest" target is defined in the main regression makefile. You can just add some more test names on the command line, to be done in addition to what's in the schedule file. I think we should do it that way here too. Oh, I wasn't aware of that. Will do. 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] Event Triggers reduced, v1
The Windows buildfarm members don't seem too happy with the latest patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] isolation check takes a long time
Alvaro Herrera writes: > Excerpts from Andrew Dunstan's message of vie jul 20 13:15:12 -0400 2012: >>> Meanwhile, I would like to remove the prepared_transactions test from >>> the main isolation schedule, and add a new Make target which runs that >>> test explicitly. Is there any objection to that? > Looks reasonable. I'd like to have an "include" directive in regress > files so that we don't have to repeat the whole set in the second file, > but please don't let that wishlist item to stop you from committing this > patch. I'm not thrilled with replicating the test-list file either. But it is not necessary: look at the way the "bigtest" target is defined in the main regression makefile. You can just add some more test names on the command line, to be done in addition to what's in the schedule file. I think we should do it that way here too. 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] Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
Robert Haas writes: > On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane wrote: >> Yeah, the just-code-defensively option is worth considering too. > After rereading this thread, I think I agree with Kevin as well. ... > Having said that, I do believe that answer is to some extent a > cop-out. I agree with that --- doing nothing at all doesn't seem like the best option here. > ... on the flip side, the C code could - not to > put too fine a point on it - be relying on just about anything. And with that too. The STRICT option is a fairly obvious security hazard, but who's to say there are not others? I think it'd be easier to make a case for forbidding a non-superuser from altering *any* property of a C function. I'd rather start from the point of allowing only what is clearly safe than disallowing only what is clearly unsafe. Taking a step or two back, I think that the real use-case we should be considering here is allowing non-superusers to own (or at least install) extensions that contain C functions. We would probably want the non-superuser to be able to drop the extension again, maybe ALTER EXTENSION SET SCHEMA, maybe ALTER EXTENSION OWNER ... and likely not too darn much else. Fooling with any of the contained objects doesn't seem like something we want to permit, since it's likely that something like a datatype is going to have dependencies on not just specific objects' properties but their interrelationships. One possible approach to that is to say that the nominal owner of such an extension only owns the extension itself, and ownership of the contained objects is held by, say, the bootstrap superuser. There are other ways too of course, but this way would bypass the problem of figuring out how to restrict what an object's nominal owner can do to it. 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] isolation check takes a long time
On 07/20/2012 01:37 PM, Alvaro Herrera wrote: Excerpts from Andrew Dunstan's message of vie jul 20 13:15:12 -0400 2012: On 07/19/2012 09:54 AM, Andrew Dunstan wrote: Meanwhile, I would like to remove the prepared_transactions test from the main isolation schedule, and add a new Make target which runs that test explicitly. Is there any objection to that? Here's the patch for that. Looks reasonable. I'd like to have an "include" directive in regress files so that we don't have to repeat the whole set in the second file, but please don't let that wishlist item to stop you from committing this patch. Are you planning to have one of your animals run the prepared xacts schedule? :-) Possibly - I'm tossing up in my mind the best way to do that. (Hacking the script on a particular client would be the wrong way.) 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] isolation check takes a long time
Excerpts from Noah Misch's message of mar jul 17 16:28:32 -0400 2012: > > On Tue, Jul 17, 2012 at 01:56:19PM -0400, Alvaro Herrera wrote: > > However, there's more work to do in isolation testing. It'd be good to > > have it being routinely run in serializable isolation level, for > > example, not just in read committed. > > Except for the foreign key specs, isolation test specs request a specific > isolation level when starting their transactions. Running such specs under > different default_transaction_isolation settings primarily confirms that > "BEGIN TRANSACTION ISOLATION LEVEL x" is indistinguishable from "BEGIN" under > default_transaction_isolation = x. It might also discover transaction > isolation sensitivity in the setup/cleanup steps, which often omit explicit > transaction control. I don't think such verification justifies regularly > running thousands of tests. The foreign key tests, however, would benefit > from running under all three isolation levels. Let's control it per-spec > instead of repeating the entire suite. Understood and agreed. Maybe we could use a new directive in the spec file format for this. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] isolation check takes a long time
Excerpts from Andrew Dunstan's message of vie jul 20 13:15:12 -0400 2012: > > On 07/19/2012 09:54 AM, Andrew Dunstan wrote: > > > > > > > > Meanwhile, I would like to remove the prepared_transactions test from > > the main isolation schedule, and add a new Make target which runs that > > test explicitly. Is there any objection to that? > > > > > > > Here's the patch for that. Looks reasonable. I'd like to have an "include" directive in regress files so that we don't have to repeat the whole set in the second file, but please don't let that wishlist item to stop you from committing this patch. Are you planning to have one of your animals run the prepared xacts schedule? :-) -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] Event Triggers reduced, v1
On 20 July 2012 16:50, Robert Haas wrote: > On Wed, Jul 18, 2012 at 10:22 AM, Robert Haas wrote: >> The next step here is obviously to complete the work necessary to >> actually be able to fire these triggers, as opposed to just saying >> that we fire them. I'll write up my thoughts on that topic in a >> separate email. I don't think there's a ton of work left to be done >> there, but more than zero. > > I have committed code to do this. It's basically similar to what you > had before, but I reworked the event cache machinery heavily. I think > that the new organization will be easier to extent to meet future > needs, and it also gets rid of a lot of boilerplate code whose job was > to translate between different representations. I also committed the > PL/pgsql support code, but not the code for the other PLs. It should > be easy to rebase that work and resubmit it as a separate patch, > though, or maybe one patch per PL would be better. > > Obviously, there's quite a bit more work that could be done here -- > passing more context, add more firing points, etc. -- but now we've at > least got the basics. > > As previously threatened, I amended this code so that triggers fire > once per DDL command. So internally generated command nodes that are > used as an argument-passing mechanism do not fire triggers, for > example. I believe this is the right decision because I think, as I > mentioned in another email to Tom yesterday, that generating and > passing around command tags is a really bad practice that we should be > looking to eliminate, not institutionalize. It posed a major obstacle > to my 9.2-era efforts to clean up the behavior of our DDL under > concurrency, for example. > > I think, however, that it would be useful to have an event trigger > that is defined to fire "every time a certain type of SQL object gets > created" rather than "every time a certain command gets executed". > That could complement, not replace, this mechanism. I've just run my own set of tests against these changes, which tests every supported DDL command (with the exception of "CREATE USER MAPPING", "ALTER USER MAPPING", "DROP USER MAPPING", "CREATE LANGUAGE" and "DROP LANGUAGE"), and many variants of each command, and everything behaves exactly as expected. :) -- Thom -- 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] isolation check takes a long time
On 07/19/2012 09:54 AM, Andrew Dunstan wrote: Meanwhile, I would like to remove the prepared_transactions test from the main isolation schedule, and add a new Make target which runs that test explicitly. Is there any objection to that? Here's the patch for that. cheers andrew diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index 74baed5..d0af9d1 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -72,3 +72,13 @@ installcheck: all check: all ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule + +# versions of the check tests that include the prepared_transactions test +# it only makes sense to run these if set up to use prepared transactions, +# via TEMP_CONFIG for the check case, or via the postgresql.conf for the +# installcheck case. +installcheck_prepared_txns: all + ./pg_isolation_regress --psqldir='$(PSQLDIR)' --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule_prepared_txns + +check_prepared_txns: all + ./pg_isolation_regress --temp-install=./tmp_check --inputdir=$(srcdir) --top-builddir=$(top_builddir) --schedule=$(srcdir)/isolation_schedule_prepared_txns diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 669c0f2..2184975 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -9,7 +9,6 @@ test: ri-trigger test: partial-index test: two-ids test: multiple-row-versions -test: prepared-transactions test: fk-contention test: fk-deadlock test: fk-deadlock2 diff --git a/src/test/isolation/isolation_schedule_prepared_txns b/src/test/isolation/isolation_schedule_prepared_txns new file mode 100644 index 000..669c0f2 --- /dev/null +++ b/src/test/isolation/isolation_schedule_prepared_txns @@ -0,0 +1,16 @@ +test: simple-write-skew +test: receipt-report +test: temporal-range-integrity +test: project-manager +test: classroom-scheduling +test: total-cash +test: referential-integrity +test: ri-trigger +test: partial-index +test: two-ids +test: multiple-row-versions +test: prepared-transactions +test: fk-contention +test: fk-deadlock +test: fk-deadlock2 +test: eval-plan-qual -- 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] row literal problem
Merlin Moncure writes: > On Fri, Jul 20, 2012 at 1:31 AM, Tom Lane wrote: >> Here's a draft patch for that. It wasn't quite as ugly as I feared. >> A lot of the apparent bulk of the patch is because I chose to split >> ExecEvalVar into separate functions for the scalar and whole-row >> cases, which seemed appropriate because they now get different >> ExprState node types. > Thanks for that! Applying the patch and confirming the fix turned up > no issues. I did a perfunctory review and it all looks pretty good: > maybe ExecInitExpr could use a comment describing the > InvalidAttrNumber check though...it's somewhat common knowledge that > InvalidAttrNumber means row variables but it's also used to initialize > variables before loops scans and things like that. Thanks for testing. I added the suggested comment and made some other cosmetic improvements, and have committed this. 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] Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)
On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane wrote: > "Kevin Grittner" writes: >> Rather than trying to enforce this in the >> ALTER FUNCTION implementation, maybe we should just advise that if >> you're going to grant ownership of a C function to a role which >> might accidentally or maliciously allow it to be called with NULL >> input, the C function should return NULL in that case. > > Yeah, the just-code-defensively option is worth considering too. After rereading this thread, I think I agree with Kevin as well. In order to have a problem, you have to (a) be superuser (i.e. be a person who already has many ways of compromising the security and integrity of the system), (b) decide to grant ownership of a C function to a non-superuser, and (c) fail to verify that the function is coded in a sufficiently defensive fashion to survive whatever that user might do with it. So it seems plausible to simply define this problem as administrator error rather than a failure of security. Having said that, I do believe that answer is to some extent a cop-out. It's practical to define things that way here because the cost of checking for NULL inputs is pretty darn small. But suppose ALTER FUNCTION had a facility to change the type of a function argument. Would we then insist that any C function intended to be used in this way must check that the type of each argument matches the function's expectation? Fortunately, we don't have to decide that right now because ALTER FUNCTION doesn't allow that and probably never will. But it would certainly be awkward if we did someday allow that, because now any C function that is intended to be used in this way has to include this extra check or be labelled insecure. Short of allowing the C code to advertise the function's expectations so that CREATE/ALTER FUNCTION can cross-check them, I don't see a way out of that problem because on the flip side, the C code could - not to put too fine a point on it - be relying on just about anything. It could assert that work_mem is less than 1GB (and the user could crash it by increasing work_mem). It could crash on any day except Tuesday (we always do payroll here on Tuesdays, so why would you call it on any other day?). It could erase the entire database unless it's marked immutable. We would surely blame any of those failure modes on the person who wrote the C code, and if we make the opposite decision here, then I think we put ourselves in the awkward position of trying to adjudicate what is and is not reasonably for third parties to do in their C code. I don't really want to go there, but I do think this points to the need for extreme caution if we ever create any more function properties that C coders might be tempted to rely on, or allow any properties that C code may already be relying on to be changed in new ways. I'm going to mark this patch as rejected in the CF app, which is not intended to foreclose debate on what to do about this, but just to state that there seems to be no consensus to solve this problem in the manner proposed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using pg_upgrade on log-shipping standby servers
On Fri, Jul 20, 2012 at 12:39:12PM -0400, Aidan Van Dyk wrote:
> If you're wanting to automatically do some upgrades wouldn't an easier route
> be:
>
> 1) run pg_upgrade, up to the point where it actually start's
> copying/linking in old cluster data files, and stop the new
> postmaster.
> 2) Take a "base backup" style copy (tar, rsync, $FAVOURITE) of the new
> cluster (small, since without data files)
> 3) Have pg_upgrade leave a log of exactly which old cluster data files
> go where in the new cluster
>
> That way, anybody, any script, etc who wants to make a new "standby"
> from and old one only needs the pg_upgrade base backup (which should
> be small, no data, just catalog stuff), and the log of which old files
> to move where.
>
> The only pre-condition is that the standby's "old pg" *APPLIED* WAL up
> to the exact same point as the master's "old pg". In that case the
> standby's old cluster data files should same enough (maybe hint bits
> off?) to be used.
I am not sure what a base backup is buying us here --- base backup is
designed to create a backup while the server is running, and it is down
at that point. I think what you are suggesting is to make a data dir
copy while just the schema is in place. That is possible, but it opens
up all kinds of possible failure cases because pg_upgrade operations
have to be done in a specific order --- it feels very fragile.
I think the commands to run after pg_upgrade --link completes on both
primary and standby might be as easy as:
cd /u/pg/pgsql.old/data
find . -links 1 -exec cp {} /u/pgsql/data \;
Why would we want anything more complicated than this?
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
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] Using pg_upgrade on log-shipping standby servers
If you're wanting to automatically do some upgrades wouldn't an easier route be: 1) run pg_upgrade, up to the point where it actually start's copying/linking in old cluster data files, and stop the new postmaster. 2) Take a "base backup" style copy (tar, rsync, $FAVOURITE) of the new cluster (small, since without data files) 3) Have pg_upgrade leave a log of exactly which old cluster data files go where in the new cluster That way, anybody, any script, etc who wants to make a new "standby" from and old one only needs the pg_upgrade base backup (which should be small, no data, just catalog stuff), and the log of which old files to move where. The only pre-condition is that the standby's "old pg" *APPLIED* WAL up to the exact same point as the master's "old pg". In that case the standby's old cluster data files should same enough (maybe hint bits off?) to be used. a. On Fri, Jul 20, 2012 at 12:25 PM, Bruce Momjian wrote: > On Tue, Jul 17, 2012 at 06:02:40PM -0400, Bruce Momjian wrote: >> Second, the user files (large) are certainly identical, it is only the >> system tables (small) that _might_ be different, so rsync'ing just those >> would add the guarantee, but I know of no easy way to rsync just the >> system tables. > > OK, new idea. I said above I didn't know how to copy just the non-user > table files (which are not modified by pg_upgrade), but actually, if you > use link mode, the user files are the only files with a hard link count > of 2. I could create a script that copied from the master to the slave > only those files with a link count of one. > > -- > Bruce Momjian http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Aidan Van Dyk Create like a god, ai...@highrise.ca command like a king, http://www.highrise.ca/ work like a slave. -- 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 -i order of vacuum
Jeff Janes writes: > On Fri, Jul 20, 2012 at 7:57 AM, Amit Kapila wrote: >> The command it executes is "vacuum analyze ..", so it will do analyze also >> on table which means >> it will collect stats corresponding to table and index. > Are there stats collected on indexes? Only for expression indexes, which there aren't any of in the standard pgbench scenario. I don't see a reason not to change the ordering as you suggest. 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] Using pg_upgrade on log-shipping standby servers
On Tue, Jul 17, 2012 at 06:02:40PM -0400, Bruce Momjian wrote: > Second, the user files (large) are certainly identical, it is only the > system tables (small) that _might_ be different, so rsync'ing just those > would add the guarantee, but I know of no easy way to rsync just the > system tables. OK, new idea. I said above I didn't know how to copy just the non-user table files (which are not modified by pg_upgrade), but actually, if you use link mode, the user files are the only files with a hard link count of 2. I could create a script that copied from the master to the slave only those files with a link count of one. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Event Triggers reduced, v1
On Wed, Jul 18, 2012 at 10:22 AM, Robert Haas wrote: > The next step here is obviously to complete the work necessary to > actually be able to fire these triggers, as opposed to just saying > that we fire them. I'll write up my thoughts on that topic in a > separate email. I don't think there's a ton of work left to be done > there, but more than zero. I have committed code to do this. It's basically similar to what you had before, but I reworked the event cache machinery heavily. I think that the new organization will be easier to extent to meet future needs, and it also gets rid of a lot of boilerplate code whose job was to translate between different representations. I also committed the PL/pgsql support code, but not the code for the other PLs. It should be easy to rebase that work and resubmit it as a separate patch, though, or maybe one patch per PL would be better. Obviously, there's quite a bit more work that could be done here -- passing more context, add more firing points, etc. -- but now we've at least got the basics. As previously threatened, I amended this code so that triggers fire once per DDL command. So internally generated command nodes that are used as an argument-passing mechanism do not fire triggers, for example. I believe this is the right decision because I think, as I mentioned in another email to Tom yesterday, that generating and passing around command tags is a really bad practice that we should be looking to eliminate, not institutionalize. It posed a major obstacle to my 9.2-era efforts to clean up the behavior of our DDL under concurrency, for example. I think, however, that it would be useful to have an event trigger that is defined to fire "every time a certain type of SQL object gets created" rather than "every time a certain command gets executed". That could complement, not replace, this mechanism. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgbench -i order of vacuum
On Fri, Jul 20, 2012 at 7:57 AM, Amit Kapila wrote: >> From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Jeff Janes >> Sent: Friday, July 20, 2012 5:36 AM > > >> Is there a reason to vacuum the pgbench_* tables after the indexes on them > are built, rather than before? > >> Since the indexes are on fresh tables, they can't have anything that needs > to be cleaned. > > The command it executes is "vacuum analyze ..", so it will do analyze also > on table which means > it will collect stats corresponding to table and index. Are there stats collected on indexes? I thought all stats were on tables only, and the only reason to visit the index was to remove all-visible-dead entries. 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] pgbench -i order of vacuum
> From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Jeff Janes > Sent: Friday, July 20, 2012 5:36 AM > Is there a reason to vacuum the pgbench_* tables after the indexes on them are built, rather than before? > Since the indexes are on fresh tables, they can't have anything that needs to be cleaned. The command it executes is "vacuum analyze ..", so it will do analyze also on table which means it will collect stats corresponding to table and index. So if you do it before creation of index pgbench might behave different. In specific, from function do_analyze_rel(), it will not call compute_index_stats() if you execute the command before Creation of index. 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] row literal problem
On Fri, Jul 20, 2012 at 1:31 AM, Tom Lane wrote: > I wrote: >> I think the way to solve this is to do whatever it takes to get access >> to the subplan targetlist. We could then do something a bit cleaner >> than what the named-rowtype code is currently doing: if there are >> resjunk columns in the subplan targetlist, use the tlist to create a >> JunkFilter, and then pass the tuples through that. After that we can >> insist that the tuples don't have any extra columns. > > Here's a draft patch for that. It wasn't quite as ugly as I feared. > A lot of the apparent bulk of the patch is because I chose to split > ExecEvalVar into separate functions for the scalar and whole-row > cases, which seemed appropriate because they now get different > ExprState node types. Thanks for that! Applying the patch and confirming the fix turned up no issues. I did a perfunctory review and it all looks pretty good: maybe ExecInitExpr could use a comment describing the InvalidAttrNumber check though...it's somewhat common knowledge that InvalidAttrNumber means row variables but it's also used to initialize variables before loops scans and things like that. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SP-GiST for ranges based on 2d-mapping and quad-tree
On 13.07.2012 02:00, Alexander Korotkov wrote: Done. There are separate patch for get rid of TrickFunctionCall2 and version of SP-GiST for ranges based on that patch. Looking at the SP-GiST patch now.. It would be nice to have an introduction, perhaps as a file comment at the top of rangetypes_spgist.c, explaining how the quad tree works. I have a general idea of what a quad tree is, but it's not immediately obvious how it maps to SP-GiST. What is stored on a leaf node and an internal node? What is the 'prefix' (seems to be the centroid)? How are ranges mapped to 2D points? (the function comment of getQuadrant() is a good start for that last one) In spg_range_quad_inner_consistent(), if in->hasPrefix == true, ISTM that in all cases where 'empty' is true, 'which' is set to 0, meaning that there can be no matches in any of the quadrants. In most of the case-branches, you explicitly check for 'empty', but even in the ones where you don't, I think you end up setting which=0 if empty==true. I'm not 100% sure about the RANGESTRAT_ADJACENT case, though. Am I missing something? It would be nice to avoid the code duplication between the new bounds_adjacent() function, and the range_adjacent_internal(). Perhaps move bounds_adjacent() to rangetypes.c and use it in range_adjacent_internal() too? -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
Hi, On Friday, July 20, 2012 11:56:09 AM Benedikt Grundmann wrote: > I also noticed just know that all TopMemoryContext's after the first one > look significantly different. They contain large PortalMemory sections. > Are those related to cursors? Yes. Andres -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
On Fri, Jul 20, 2012 at 10:56 AM, Benedikt Grundmann wrote: > On Fri, Jul 20, 2012 at 10:46 AM, Benedikt Grundmann > wrote: >> > Actually I believe this must be it. I just went back and checked the library > and it does not CLOSE the cursors. This is normally not a problem as most > transactions we have run one or two queries only... I'll patch the library > to CLOSE the cursor when all the data has been delivered and test if the > error does not happen then. > Just to confirm that indeed fixed it. Sorry for the noise. Bene -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
On Fri, Jul 20, 2012 at 10:46 AM, Benedikt Grundmann wrote: > > DECLARE sqmlcursor51587 CURSOR FOR select > entry_time,source,bad_fields,isin,sedol,cusip,bloomberg,reuters,exchange_code,currency,description,bbg_instrument_type,instrument_type,specifics,definer,primary_exchange,is_primary_security,is_primary_listing,tags,bloomberg_id,status > from vw_instruments_v7 where jane_symbol = E'FOO BAR' and true and > effective_until = (select max(effective_until) from > vw_instruments_v7)" > > Sorry I imagine that the fact that this generates a cursor every time > is important > but it had honestly escaped my attention, because the library we use to query > the database uses CURSORs basically for every select, so that it can process > the data in batches (in this particular case that is conceptually unnecessary > as > the query will only return one row, but the library does not know that). > Actually I believe this must be it. I just went back and checked the library and it does not CLOSE the cursors. This is normally not a problem as most transactions we have run one or two queries only... I'll patch the library to CLOSE the cursor when all the data has been delivered and test if the error does not happen then. I also noticed just know that all TopMemoryContext's after the first one look significantly different. They contain large PortalMemory sections. Are those related to cursors? TransactionAbortContext: 32768 total in 1 blocks; 32736 free (0 chunks); 32 used Portal hash: 8380416 total in 10 blocks; 3345088 free (34 chunks); 5035328 used PortalMemory: 16769024 total in 11 blocks; 2737280 free (15 chunks); 14031744 used PortalHeapMemory: 56320 total in 9 blocks; 4320 free (0 chunks); 52000 used ExecutorState: 57344 total in 3 blocks; 15248 free (3 chunks); 42096 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 8192 total in 1 blocks; 8160 free (0 chunks); 32 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used PortalHeapMemory: 56320 total in 9 blocks; 4320 free (0 chunks); 52000 used ExecutorState: 57344 total in 3 blocks; 15248 free (3 chunks); 42096 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 8192 total in 1 blocks; 8160 free (0 chunks); 32 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used ... Thanks everyone, Bene -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
On Fri, Jul 20, 2012 at 9:10 AM, Heikki Linnakangas wrote: > On 20.07.2012 10:19, Benedikt Grundmann wrote: >> >> We yesterday encountered a program that in a degenerate case issued in >> a single transaction a huge number of selects (in a single transaction >> but each select in a separate call to PGExec) (huge = ~ 400,000). >> >> That transaction would continue to eat memory up until a point where >> calls to malloc (in aset.c) would fail and log for example: >> >> ,"out of memory","Failed on request of size 11." >> >> ... >> >> >> - Is that expected expected behaviour? The transaction was >> in READ_COMMITED mode, and my best guess is that this implies >> that some snapshot is taken before each subselect and all >> of them are only freed once the transaction is finished > > > In more complicated scenarios, with e.g subtransactions, it's normal that > there's some growth in memory usage like that. But with simple SELECTs, I > don't think there should be. > > Can you write a simple self-contained test script that reproduces this? That > would make it easier to see where the memory is going. > Assuming that it isn't obvious now that I realized that we generate a cursor every time, I will give it a shot otherwise. > PS, you should upgrade, the latest minor version in 8.4.x series is 8.4.12. > If possible, upgrading to a more recent major version would be a good idea > too. I don't know if it will help with this problem, but it might.. > We are in fact automatically doing an upgrade in testing to 9.1 every day at the moment. We plan to pull the trigger in production in a few weeks. Thanks, Bene -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
First of all thanks to everyone who has replied so far. On Fri, Jul 20, 2012 at 10:04 AM, Andres Freund wrote: > > Hi, > > On Friday, July 20, 2012 09:19:31 AM Benedikt Grundmann wrote: > > We yesterday encountered a program that in a degenerate case > > issued in a single transaction a huge number of selects (in a > > single transaction but each select in a separate call to PGExec) > > (huge = ~ 400,000). > > That transaction would continue to eat memory up until a point > > where calls to malloc (in aset.c) would fail and log for example: > > ,"out of memory","Failed on request of size 11." > Could you show us the different statements youre running in that transaction? They all look like this: DECLARE sqmlcursor51587 CURSOR FOR select entry_time,source,bad_fields,isin,sedol,cusip,bloomberg,reuters,exchange_code,currency,description,bbg_instrument_type,instrument_type,specifics,definer,primary_exchange,is_primary_security,is_primary_listing,tags,bloomberg_id,status from vw_instruments_v7 where jane_symbol = E'FOO BAR' and true and effective_until = (select max(effective_until) from vw_instruments_v7)" Sorry I imagine that the fact that this generates a cursor every time is important but it had honestly escaped my attention, because the library we use to query the database uses CURSORs basically for every select, so that it can process the data in batches (in this particular case that is conceptually unnecessary as the query will only return one row, but the library does not know that). > Are you using any user defined functions, deferred foreign keys, or anything > like that? No there are several versions of the above view and while the one mentioned above contains calls to regexp_replace I can reproduce the same behaviour with a different version of the view that just renames columns of the underlying table. > After youve got that "out of memory" message, the log should show > a list of memory contexts with the amount of memory allocated in > each. Could you include that in a mail? We are using csv logging, which through me off for a moment because I couldn't find it in there. But indeed in the .log file I see memory contexts but I don't know how to correlate them. I assume they only get written when out of memory happens, so I have included below the very first one. TopMemoryContext: 268528136 total in 31 blocks; 37640 free (160 chunks); 268490496 used TopTransactionContext: 24576 total in 2 blocks; 14872 free (4 chunks); 9704 used Local Buffer Lookup Table: 2088960 total in 8 blocks; 234944 free (25 chunks); 1854016 used Type information cache: 24576 total in 2 blocks; 11888 free (5 chunks); 12688 used Operator lookup cache: 24576 total in 2 blocks; 11888 free (5 chunks); 12688 used PL/PgSQL function context: 24576 total in 2 blocks; 14384 free (14 chunks); 10192 used CFuncHash: 8192 total in 1 blocks; 1680 free (0 chunks); 6512 used Rendezvous variable hash: 8192 total in 1 blocks; 1680 free (0 chunks); 6512 used PLpgSQL function cache: 24520 total in 2 blocks; 3744 free (0 chunks); 20776 used Operator class cache: 8192 total in 1 blocks; 1680 free (0 chunks); 6512 used MessageContext: 131072 total in 5 blocks; 54792 free (5 chunks); 76280 used smgr relation table: 24576 total in 2 blocks; 5696 free (4 chunks); 18880 used TransactionAbortContext: 32768 total in 1 blocks; 32736 free (0 chunks); 32 used Portal hash: 8192 total in 1 blocks; 1680 free (0 chunks); 6512 used PortalMemory: 8192 total in 1 blocks; 7888 free (1 chunks); 304 used PortalHeapMemory: 1024 total in 1 blocks; 848 free (0 chunks); 176 used ExecutorState: 65600 total in 4 blocks; 33792 free (18 chunks); 31808 used ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used Relcache by OID: 24576 total in 2 blocks; 11792 free (3 chunks); 12784 used CacheMemoryContext: 1342128 total in 21 blocks; 290016 free (11 chunks); 1052112 used idx_raw_cfd_bear_commodity_position_records_position_date: 2048 total in 1 blocks; 752 free (0 chunks); 1296 used CachedPlan: 3072 total in 2 blocks; 2008 free (2 chunks); 1064 used CachedPlanSource: 3072 total in 2 blocks; 1656 free (0 chunks); 1416 used SPI Plan: 1024 total in 1 blocks; 808 free (0 chunks); 216 used CachedPlan: 3072 total in 2 blocks; 1984 free (1 chunks); 1088 used CachedPlanSource: 3072 total in 2 blocks; 1696 free (0 chunks); 1376 used SPI Plan: 1024 total in 1 blocks; 808 free (0 chunks); 216 used CachedPlan: 1024 total in 1 blocks; 312 free (0 chunks); 712 used CachedPlanSource: 3072 total in 2 blocks; 1584 free (0 chunks); 1488 used SPI Plan: 1024 total in 1 blocks; 832 free (0 chunks); 192 used CachedPlan: 1024 total in 1 blocks; 304 free (0 chunks); 720 used CachedPlanSource: 3072 total in 2 blocks; 1960 free (3 chunks); 1112 used SPI Plan: 1024 total in 1 blocks; 808 free (0 chunks); 216 used CachedPlan: 1024 total in 1 blocks; 304 free (0 chunks); 720 used
Re: [HACKERS] pgsql_fdw in contrib
On Thu, Jul 19, 2012 at 6:06 PM, Kohei KaiGai wrote: > Hanada-san, > > What about the status of your patch? Sorry for absence. I have been struggling with connection recovery issue, but I haven't fixed it yet. So I'd like to withdraw this patch from this CF and mark it as "returned with feedback" because it is far from commit at the moment. Of course fixing naming issue too is in my TODO list. > Even though the 1st commit-fest is getting closed soon, > I'd like to pay efforts for reviewing to pull up the status of > pgsql_fdw into "ready for committer" by beginning of the > upcoming commit-fest. Thanks again, I'll try harder this summer. ;-) Regards, -- Shigeru Hanada -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
Hi, On Friday, July 20, 2012 09:19:31 AM Benedikt Grundmann wrote: > We yesterday encountered a program that in a degenerate case issued in > a single transaction a huge number of selects (in a single transaction > but each select in a separate call to PGExec) (huge = ~ 400,000). > > That transaction would continue to eat memory up until a point where > calls to malloc (in aset.c) would fail and log for example: > > ,"out of memory","Failed on request of size 11." Could you show us the different statements youre running in that transaction? Are you using any user defined functions, deferred foreign keys, or anything like that? After youve got that "out of memory" message, the log should show a list of memory contexts with the amount of memory allocated in each. Could you include that in a mail? Greetings, Andres -- 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] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
On 20.07.2012 10:19, Benedikt Grundmann wrote: We yesterday encountered a program that in a degenerate case issued in a single transaction a huge number of selects (in a single transaction but each select in a separate call to PGExec) (huge = ~ 400,000). That transaction would continue to eat memory up until a point where calls to malloc (in aset.c) would fail and log for example: ,"out of memory","Failed on request of size 11." ... - Is that expected expected behaviour? The transaction was in READ_COMMITED mode, and my best guess is that this implies that some snapshot is taken before each subselect and all of them are only freed once the transaction is finished In more complicated scenarios, with e.g subtransactions, it's normal that there's some growth in memory usage like that. But with simple SELECTs, I don't think there should be. Can you write a simple self-contained test script that reproduces this? That would make it easier to see where the memory is going. PS, you should upgrade, the latest minor version in 8.4.x series is 8.4.12. If possible, upgrading to a more recent major version would be a good idea too. I don't know if it will help with this problem, but it might.. -- 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
[HACKERS] postgres 8.4.9: running out of memory (malloc fails) when running a transaction that runs a LOT of selects
We yesterday encountered a program that in a degenerate case issued in a single transaction a huge number of selects (in a single transaction but each select in a separate call to PGExec) (huge = ~ 400,000). That transaction would continue to eat memory up until a point where calls to malloc (in aset.c) would fail and log for example: ,"out of memory","Failed on request of size 11." Both from that transaction and random others. In additional observation of interest to us was that while both VIRT and RES was growing VIRT was always roughly double of RES. Which seems to indicate that whatever allocations were done not all of the memory allocated was actually touched (server is a Centos6 box). So I have two questions: - Is that expected expected behaviour? The transaction was in READ_COMMITED mode, and my best guess is that this implies that some snapshot is taken before each subselect and all of them are only freed once the transaction is finished - Any recommendations on the use of overcommit? We had it disabled on the assumption that with overcommit the OOM killer might kill a random process and that it is better to instead have a process that is actually allocating fail (and on the additional assumption that any memory allocated by postgres would actually be touched). This is not a huge production issue for us as we can fix the program to no longer issue huge numbers of selects. But we would like to understand. Thank you very much, Bene PS: The machine has huge amounts of memory and during normal operations it looks like this: -bash-4.1$ cat /proc/meminfo MemTotal: 49413544 kB MemFree: 1547604 kB Buffers:5808 kB Cached: 43777988 kB SwapCached:0 kB Active: 18794732 kB Inactive: 27309980 kB Active(anon): 13786796 kB Inactive(anon): 1411296 kB Active(file):5007936 kB Inactive(file): 25898684 kB Unevictable: 71928 kB Mlocked: 55576 kB SwapTotal: 2047992 kB SwapFree:2047992 kB Dirty: 12100 kB Writeback: 79684 kB AnonPages: 2393372 kB Mapped: 12887392 kB Shmem: 12871676 kB Slab:1050468 kB SReclaimable: 190068 kB SUnreclaim: 860400 kB KernelStack:4832 kB PageTables: 450584 kB NFS_Unstable: 23312 kB Bounce:0 kB WritebackTmp: 0 kB CommitLimit:26754764 kB Committed_AS: 17394312 kB VmallocTotal: 34359738367 kB VmallocUsed: 120472 kB VmallocChunk: 34333956900 kB HardwareCorrupted: 0 kB AnonHugePages: 1599488 kB HugePages_Total: 0 HugePages_Free:0 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB DirectMap4k:5604 kB DirectMap2M: 2078720 kB DirectMap1G:48234496 kB -- 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] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.
On 20/07/12 08:59, Jan Urbański wrote:
On 18/07/12 17:17, Heikki Linnakangas wrote:
On 14.07.2012 17:50, Jan Urbański wrote:
If pg_do_encoding_conversion() throws an error, you don't get a chance
to call Py_DECREF() to release the string. Is that a problem?
If an error occurs in PLy_traceback(), after incrementing
recursion_depth, you don't get a chance to decrement it again. I'm not
sure if the Py* function calls can fail, but at least seemingly trivial
things like initStringInfo() can throw an out-of-memory error.
Of course you're right (on both accounts).
Here's a version with a bunch of PG_TRies thrown in.
Silly me, playing tricks with postincrements before fully waking up.
Here's v3, with a correct inequality test for exceeding the traceback
recursion test.
J
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
new file mode 100644
index c375ac0..0ad8358
*** a/src/pl/plpython/plpy_elog.c
--- b/src/pl/plpython/plpy_elog.c
*** static char *get_source_line(const char
*** 28,33
--- 28,41
/*
+ * Guard agains re-entrant calls to PLy_traceback, which can happen if
+ * traceback formatting functions raise Python errors.
+ */
+ #define TRACEBACK_RECURSION_LIMIT 2
+ static int recursion_depth = 0;
+
+
+ /*
* Emit a PG error or notice, together with any available info about
* the current Python error, previously set by PLy_exception_set().
* This should be used to propagate Python errors into PG. If fmt is
*** static char *get_source_line(const char
*** 38,46
void
PLy_elog(int elevel, const char *fmt,...)
{
! char *xmsg;
! char *tbmsg;
! int tb_depth;
StringInfoData emsg;
PyObject *exc,
*val,
--- 46,54
void
PLy_elog(int elevel, const char *fmt,...)
{
! char *xmsg = NULL;
! char *tbmsg = NULL;
! int tb_depth = 0;
StringInfoData emsg;
PyObject *exc,
*val,
*** PLy_elog(int elevel, const char *fmt,...
*** 62,68
}
PyErr_Restore(exc, val, tb);
! PLy_traceback(&xmsg, &tbmsg, &tb_depth);
if (fmt)
{
--- 70,90
}
PyErr_Restore(exc, val, tb);
! if (recursion_depth++ < TRACEBACK_RECURSION_LIMIT)
! {
! PG_TRY();
! {
! PLy_traceback(&xmsg, &tbmsg, &tb_depth);
! }
! PG_CATCH();
! {
! recursion_depth--;
! PG_RE_THROW();
! }
! PG_END_TRY();
! }
!
! recursion_depth--;
if (fmt)
{
diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c
new file mode 100644
index 4aabafc..94c035e
*** a/src/pl/plpython/plpy_util.c
--- b/src/pl/plpython/plpy_util.c
*** PLy_free(void *ptr)
*** 61,126
PyObject *
PLyUnicode_Bytes(PyObject *unicode)
{
! PyObject *rv;
! const char *serverenc;
! /*
! * Map PostgreSQL encoding to a Python encoding name.
! */
! switch (GetDatabaseEncoding())
{
! case PG_SQL_ASCII:
! /*
! * Mapping SQL_ASCII to Python's 'ascii' is a bit bogus. Python's
! * 'ascii' means true 7-bit only ASCII, while PostgreSQL's
! * SQL_ASCII means that anything is allowed, and the system doesn't
! * try to interpret the bytes in any way. But not sure what else
! * to do, and we haven't heard any complaints...
! */
! serverenc = "ascii";
! break;
! case PG_WIN1250:
! serverenc = "cp1250";
! break;
! case PG_WIN1251:
! serverenc = "cp1251";
! break;
! case PG_WIN1252:
! serverenc = "cp1252";
! break;
! case PG_WIN1253:
! serverenc = "cp1253";
! break;
! case PG_WIN1254:
! serverenc = "cp1254";
! break;
! case PG_WIN1255:
! serverenc = "cp1255";
! break;
! case PG_WIN1256:
! serverenc = "cp1256";
! break;
! case PG_WIN1257:
! serverenc = "cp1257";
! break;
! case PG_WIN1258:
! serverenc = "cp1258";
! break;
! case PG_WIN866:
! serverenc = "cp866";
! break;
! case PG_WIN874:
! serverenc = "cp874";
! break;
! default:
! /* Other encodings have the same name in Python. */
! serverenc = GetDatabaseEncodingName();
! break;
}
! rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! if (rv == NULL)
! PLy_elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
return rv;
}
--- 61,106
PyObject *
PLyUnicode_Bytes(PyObject *unicode)
{
! PyObject *bytes, *rv;
! char *utf8string, *encoded;
! /* first encode the Python unicode object with UTF-8 */
! bytes = PyUnicode_AsUTF8String(unicode);
! if (bytes == NULL)
! PLy_elog(ERROR, "could not convert Python Unicode object to bytes");
!
! utf8string = PyBytes_AsString(bytes);
! if (utf8string == NULL) {
! Py_DECREF(bytes);
! PLy_elog(ERROR, "could not extract bytes from encoded string");
! }
!
! /* then convert to server encoding */
! PG_TRY();
{
! encoded = (char *) pg_do_encoding_conversion(
! (unsigned char *) utf8string,
! strlen(utf8string),
! PG_UTF8,
! GetDatabaseEncoding()
Re: [HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.
On 18/07/12 17:17, Heikki Linnakangas wrote:
On 14.07.2012 17:50, Jan Urbański wrote:
If pg_do_encoding_conversion() throws an error, you don't get a chance
to call Py_DECREF() to release the string. Is that a problem?
If an error occurs in PLy_traceback(), after incrementing
recursion_depth, you don't get a chance to decrement it again. I'm not
sure if the Py* function calls can fail, but at least seemingly trivial
things like initStringInfo() can throw an out-of-memory error.
Of course you're right (on both accounts).
Here's a version with a bunch of PG_TRies thrown in.
Cheers,
Jan
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
new file mode 100644
index c375ac0..9944f72
*** a/src/pl/plpython/plpy_elog.c
--- b/src/pl/plpython/plpy_elog.c
*** static char *get_source_line(const char
*** 28,33
--- 28,41
/*
+ * Guard agains re-entrant calls to PLy_traceback, which can happen if
+ * traceback formatting functions raise Python errors.
+ */
+ #define TRACEBACK_RECURSION_LIMIT 2
+ static int recursion_depth = 0;
+
+
+ /*
* Emit a PG error or notice, together with any available info about
* the current Python error, previously set by PLy_exception_set().
* This should be used to propagate Python errors into PG. If fmt is
*** static char *get_source_line(const char
*** 38,46
void
PLy_elog(int elevel, const char *fmt,...)
{
! char *xmsg;
! char *tbmsg;
! int tb_depth;
StringInfoData emsg;
PyObject *exc,
*val,
--- 46,54
void
PLy_elog(int elevel, const char *fmt,...)
{
! char *xmsg = NULL;
! char *tbmsg = NULL;
! int tb_depth = 0;
StringInfoData emsg;
PyObject *exc,
*val,
*** PLy_elog(int elevel, const char *fmt,...
*** 62,68
}
PyErr_Restore(exc, val, tb);
! PLy_traceback(&xmsg, &tbmsg, &tb_depth);
if (fmt)
{
--- 70,90
}
PyErr_Restore(exc, val, tb);
! if (recursion_depth++ <= TRACEBACK_RECURSION_LIMIT)
! {
! PG_TRY();
! {
! PLy_traceback(&xmsg, &tbmsg, &tb_depth);
! }
! PG_CATCH();
! {
! recursion_depth--;
! PG_RE_THROW();
! }
! PG_END_TRY();
! }
!
! recursion_depth--;
if (fmt)
{
diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c
new file mode 100644
index 4aabafc..94c035e
*** a/src/pl/plpython/plpy_util.c
--- b/src/pl/plpython/plpy_util.c
*** PLy_free(void *ptr)
*** 61,126
PyObject *
PLyUnicode_Bytes(PyObject *unicode)
{
! PyObject *rv;
! const char *serverenc;
! /*
! * Map PostgreSQL encoding to a Python encoding name.
! */
! switch (GetDatabaseEncoding())
{
! case PG_SQL_ASCII:
! /*
! * Mapping SQL_ASCII to Python's 'ascii' is a bit bogus. Python's
! * 'ascii' means true 7-bit only ASCII, while PostgreSQL's
! * SQL_ASCII means that anything is allowed, and the system doesn't
! * try to interpret the bytes in any way. But not sure what else
! * to do, and we haven't heard any complaints...
! */
! serverenc = "ascii";
! break;
! case PG_WIN1250:
! serverenc = "cp1250";
! break;
! case PG_WIN1251:
! serverenc = "cp1251";
! break;
! case PG_WIN1252:
! serverenc = "cp1252";
! break;
! case PG_WIN1253:
! serverenc = "cp1253";
! break;
! case PG_WIN1254:
! serverenc = "cp1254";
! break;
! case PG_WIN1255:
! serverenc = "cp1255";
! break;
! case PG_WIN1256:
! serverenc = "cp1256";
! break;
! case PG_WIN1257:
! serverenc = "cp1257";
! break;
! case PG_WIN1258:
! serverenc = "cp1258";
! break;
! case PG_WIN866:
! serverenc = "cp866";
! break;
! case PG_WIN874:
! serverenc = "cp874";
! break;
! default:
! /* Other encodings have the same name in Python. */
! serverenc = GetDatabaseEncodingName();
! break;
}
! rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! if (rv == NULL)
! PLy_elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
return rv;
}
--- 61,106
PyObject *
PLyUnicode_Bytes(PyObject *unicode)
{
! PyObject *bytes, *rv;
! char *utf8string, *encoded;
! /* first encode the Python unicode object with UTF-8 */
! bytes = PyUnicode_AsUTF8String(unicode);
! if (bytes == NULL)
! PLy_elog(ERROR, "could not convert Python Unicode object to bytes");
!
! utf8string = PyBytes_AsString(bytes);
! if (utf8string == NULL) {
! Py_DECREF(bytes);
! PLy_elog(ERROR, "could not extract bytes from encoded string");
! }
!
! /* then convert to server encoding */
! PG_TRY();
{
! encoded = (char *) pg_do_encoding_conversion(
! (unsigned char *) utf8string,
! strlen(utf8string),
! PG_UTF8,
! GetDatabaseEncoding());
! }
! PG_CATCH();
! {
! Py_DECREF(bytes);
! PG_RE_THROW();
}
+ PG_END_TRY();
! /* finally, build a bytes object in the server encoding */
! rv = PyBytes_FromStringAndSize
