[COMMITTERS] pgsql: Rework 'MOVE ALL' to 'ALTER .. ALL IN TABLESPACE'
Rework 'MOVE ALL' to 'ALTER .. ALL IN TABLESPACE' As 'ALTER TABLESPACE .. MOVE ALL' really didn't change the tablespace but instead changed objects inside tablespaces, it made sense to rework the syntax and supporting functions to operate under the 'ALTER (TABLE|INDEX|MATERIALIZED VIEW)' syntax and to be in tablecmds.c. Pointed out by Alvaro, who also suggested the new syntax. Back-patch to 9.4. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/3c4cf080879b386d4ed1814667aca025caafe608 Modified Files -- doc/src/sgml/ref/alter_index.sgml | 13 ++ doc/src/sgml/ref/alter_materialized_view.sgml |2 + doc/src/sgml/ref/alter_table.sgml | 20 ++- doc/src/sgml/ref/alter_tablespace.sgml| 78 --- doc/src/sgml/release-9.4.sgml |5 +- src/backend/commands/tablecmds.c | 171 +++ src/backend/commands/tablespace.c | 179 - src/backend/nodes/copyfuncs.c | 11 +- src/backend/nodes/equalfuncs.c|9 +- src/backend/parser/gram.y | 165 ++- src/backend/tcop/utility.c| 20 ++- src/include/commands/tablecmds.h |2 + src/include/commands/tablespace.h |1 - src/include/nodes/nodes.h |2 +- src/include/nodes/parsenodes.h|7 +- src/test/regress/input/tablespace.source |5 +- src/test/regress/output/tablespace.source |5 +- src/tools/pgindent/typedefs.list |2 +- 18 files changed, 305 insertions(+), 392 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Rework 'MOVE ALL' to 'ALTER .. ALL IN TABLESPACE'
Rework 'MOVE ALL' to 'ALTER .. ALL IN TABLESPACE' As 'ALTER TABLESPACE .. MOVE ALL' really didn't change the tablespace but instead changed objects inside tablespaces, it made sense to rework the syntax and supporting functions to operate under the 'ALTER (TABLE|INDEX|MATERIALIZED VIEW)' syntax and to be in tablecmds.c. Pointed out by Alvaro, who also suggested the new syntax. Back-patch to 9.4. Branch -- REL9_4_STABLE Details --- http://git.postgresql.org/pg/commitdiff/d9b2bc45cf75f913490f1b3ce9b9263509b26704 Modified Files -- doc/src/sgml/ref/alter_index.sgml | 13 ++ doc/src/sgml/ref/alter_materialized_view.sgml |2 + doc/src/sgml/ref/alter_table.sgml | 20 ++- doc/src/sgml/ref/alter_tablespace.sgml| 78 --- doc/src/sgml/release-9.4.sgml |5 +- src/backend/commands/tablecmds.c | 171 +++ src/backend/commands/tablespace.c | 179 - src/backend/nodes/copyfuncs.c | 11 +- src/backend/nodes/equalfuncs.c|9 +- src/backend/parser/gram.y | 165 ++- src/backend/tcop/utility.c| 20 ++- src/include/commands/tablecmds.h |2 + src/include/commands/tablespace.h |1 - src/include/nodes/nodes.h |2 +- src/include/nodes/parsenodes.h|7 +- src/test/regress/input/tablespace.source |5 +- src/test/regress/output/tablespace.source |5 +- src/tools/pgindent/typedefs.list |2 +- 18 files changed, 305 insertions(+), 392 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Sure, but I think Greg's point is that this could be tested by a
> black-box functional test ("does it print something it shouldn't")
> rather than a white-box test that necessarily depends on a whole lot
> of *other* planner choices that don't have much to do with the point
> in question. You already got bit by variances in the choice of join
> type, which is not what the test is about.
Yes, but as I said, I'm *also* doing the black-box functional test..
Perhaps that means this extra EXPLAIN test is overkill, but, after
trying to run down whatever build animal 'hamerkop' has done to break
the tablespace regression test, I'm definitely keen to err on the side
of 'more information'.
> I think the test is okay as-is as long as we don't see more failures
> from it; but if we do see any more I'd suggest rewriting as per Greg's
> suggestion rather than trying to constrain the plan choice even more
> narrowly.
The 'rewriting', in this case, would simply be removing the 'explain'
part of the test and keeping the rest. If you or Greg see an issue with
the test that actually does the 'raise notice' for every value seen by
the 'snoop' function or it doesn't match your 'black-box' expectation,
please let me know and I can try to refine it..
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests
Greg, * Greg Stark (st...@mit.edu) wrote: > But the original goal seems like it would be easier and better done with an > immutable function which lies and calls elog to leak information. That's > the actual attack this is supposed to protect against anyways. Uh, yes, that's what the explain is about- making sure that the 'snoop' function in the regression test doesn't get pushed down. It *also* runs the function which raises a notice for each item the function can see, and verifies that only those values are returned.. > That would make the tests more robust against other changes causing > failures. Even things like changing explain output formatting for example. Sure, but there's a whole slew of tests that would have to change if we changed the explain output, not just this one. I don't think we really want to make a policy against doing EXPLAIN in regression tests, but if so, we'd need to go change quite a few tests which have been working pretty well to date.. Thanks, Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests
* Andrew Dunstan (and...@dunslane.net) wrote: > Are you really sure we can do this consistently? The regression > tests have to run against all sorts of settings, including those we > have no control over via installcheck. Sure? No. However, there are quite a few existing regression tests that do the same (with costs off, of course), and things on the buildfarm look more-or-less clean again, so I'm at least hopeful. I can simplify the tests some if they end up causing issues. Thanks, Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Make a dedicated AlterTblSpcStmt production
Make a dedicated AlterTblSpcStmt production Given that ALTER TABLESPACE has moved on from just existing for general purpose rename/owner changes, it deserves its own top-level production in the grammar. This also cleans up the RenameStmt to only ever be used for actual RENAMEs again- it really wasn't appropriate to hide non-RENAME productions under there. Noted by Alvaro. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/5f508b6dea19b66961c645bf5e5c427ac3af8359 Modified Files -- src/backend/parser/gram.y | 239 +++-- 1 file changed, 124 insertions(+), 115 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add ANALYZE into regression tests
Add ANALYZE into regression tests Looks like we can end up with different plans happening on the buildfarm, which breaks the regression tests when we include EXPLAIN output (which is done in the regression tests for updatable security views, to ensure that the user-defined function isn't pushed down to a level where it could view the rows before the security quals are applied). This adds in ANALYZE to hopefully make the plans consistent. The ANALYZE ends up changing the original plan too, so the update looks bigger than it really is. The new plan looks perfectly valid, of course. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/b3e6593716efef901fcc847f33256c6b49958898 Modified Files -- src/test/regress/expected/updatable_views.out | 232 +++-- src/test/regress/sql/updatable_views.sql |4 + 2 files changed, 102 insertions(+), 134 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Make security barrier views automatically updatable
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > Make security barrier views automatically updatable > > For the record, this should have bumped catversion, because it > broke stored views. Given that I'd just done a bump a few hours > earlier, there's probably no need for a retrospective catversion > change, but just so you know: any patch that touches readfuncs.c > probably needs a catversion change. Ah, yeah, makes sense. Will keep that in mind. Thanks, Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Make security barrier views automatically updatable
Make security barrier views automatically updatable Views which are marked as security_barrier must have their quals applied before any user-defined quals are called, to prevent user-defined functions from being able to see rows which the security barrier view is intended to prevent them from seeing. Remove the restriction on security barrier views being automatically updatable by adding a new securityQuals list to the RTE structure which keeps track of the quals from security barrier views at each level, independently of the user-supplied quals. When RTEs are later discovered which have securityQuals populated, they are turned into subquery RTEs which are marked as security_barrier to prevent any user-supplied quals being pushed down (modulo LEAKPROOF quals). Dean Rasheed, reviewed by Craig Ringer, Simon Riggs, KaiGai Kohei Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/842faa714c0454d67e523f5a0b6df6500e9bc1a5 Modified Files -- doc/src/sgml/ref/create_view.sgml | 19 +- src/backend/commands/tablecmds.c |6 +- src/backend/commands/view.c |6 +- src/backend/nodes/copyfuncs.c |1 + src/backend/nodes/equalfuncs.c|1 + src/backend/nodes/nodeFuncs.c |4 + src/backend/nodes/outfuncs.c |1 + src/backend/nodes/readfuncs.c |1 + src/backend/optimizer/plan/planner.c | 45 +- src/backend/optimizer/prep/Makefile |2 +- src/backend/optimizer/prep/prepsecurity.c | 466 +++ src/backend/optimizer/prep/prepunion.c| 60 ++- src/backend/rewrite/rewriteHandler.c | 53 ++- src/include/nodes/parsenodes.h|1 + src/include/optimizer/prep.h |5 + src/include/rewrite/rewriteHandler.h |1 - src/test/regress/expected/create_view.out |2 +- src/test/regress/expected/updatable_views.out | 620 +++-- src/test/regress/sql/updatable_views.sql | 180 ++- 19 files changed, 1372 insertions(+), 102 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allocate fresh memory for post_opts/exec_path
Allocate fresh memory for post_opts/exec_path Instead of having read_post_opts() depend on the memory allocated for the config file (which is now getting free'd), pg_strdup() for post_opts and exec_path (similar to how it's being done elsewhere). Noted by Thom Brown. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/dd917bb793b27f8c7616f0e64f9a119e8d98eb24 Modified Files -- src/bin/pg_ctl/pg_ctl.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix issues with pg_ctl
* Thom Brown (t...@linux.com) wrote: > On 5 March 2014 06:33, Stephen Frost wrote: > > Fix issues with pg_ctl [...] > thom@swift ~/dbtest $ sh: 1: ���: not found Yeesh. Always with the simple ones it seems. Yeah, I see what the issue here is, will fix. Thanks! Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Fix issues with pg_ctl
Fix issues with pg_ctl The new, small, free_readfile managed to have bug in it which could cause it to try and free something it shouldn't, and fix the case where it was being called with an invalid pointer leading to a segfault. Noted by Bruce, issues introduced and fixed by me. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/eb933162cdcbcaa5c56c75eb21b9c055af9748a0 Modified Files -- src/bin/pg_ctl/pg_ctl.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Various Coverity-spotted fixes
K, will check it out. Thanks! On Tuesday, March 4, 2014, Bruce Momjian wrote: > On Sun, Mar 2, 2014 at 03:14:35AM +0000, Stephen Frost wrote: > > Various Coverity-spotted fixes > > > > A number of issues were identified by the Coverity scanner and are > > addressed in this patch. None of these appear to be security issues > > and many are mostly cosmetic changes. > > > > Short comments for each of the changes follows. > > > > Correct the semi-colon placement in be-secure.c regarding SSL retries. > > Remove a useless comparison-to-NULL in proc.c (value is dereferenced > > prior to this check and therefore can't be NULL). > > Add checking of chmod() return values to initdb. > > Fix a couple minor memory leaks in initdb. > > Fix memory leak in pg_ctl- involves free'ing the config file contents. > -- > > I am seeing a pg_ctl crash if you run 'pg_ctl status' when the server is > running: > > *** glibc detected *** pg_ctl: free(): invalid pointer: > 0x011c42c8 *** > > The error is coming from the new free_readfile() function. I believe > the error was introduced in this commit. > > -- > Bruce Momjian > > http://momjian.us > EnterpriseDB http://enterprisedb.com > > + Everyone has their own god. + > >
[COMMITTERS] pgsql: Another round of Coverity fixes
Another round of Coverity fixes Additional non-security issues/improvements spotted by Coverity. In backend/libpq, no sense trying to protect against port->hba being NULL after we've already dereferenced it in the switch() statement. Prevent against possible overflow due to 32bit arithmitic in basebackup throttling (not yet released, so no security concern). Remove nonsensical check of array pointer against NULL in procarray.c, looks to be a holdover from 9.1 and earlier when there were pointers being used but now it's just an array. Remove pointer check-against-NULL in tsearch/spell.c as we had already dereferenced it above (in the strcmp()). Remove dead code from adt/orderedsetaggs.c, isnull is checked immediately after each tuplesort_getdatum() call and if true we return, so no point checking it again down at the bottom. Remove recently added minor error-condition memory leak in pg_regress. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/5592ebac55460866da867df5c783c34e3c9a7cae Modified Files -- src/backend/libpq/auth.c | 18 +++--- src/backend/replication/basebackup.c |3 ++- src/backend/storage/ipc/procarray.c|9 +++-- src/backend/tsearch/spell.c|2 +- src/backend/utils/adt/orderedsetaggs.c |5 + src/test/regress/pg_regress.c | 12 +--- 6 files changed, 23 insertions(+), 26 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Various Coverity-spotted fixes
Various Coverity-spotted fixes A number of issues were identified by the Coverity scanner and are addressed in this patch. None of these appear to be security issues and many are mostly cosmetic changes. Short comments for each of the changes follows. Correct the semi-colon placement in be-secure.c regarding SSL retries. Remove a useless comparison-to-NULL in proc.c (value is dereferenced prior to this check and therefore can't be NULL). Add checking of chmod() return values to initdb. Fix a couple minor memory leaks in initdb. Fix memory leak in pg_ctl- involves free'ing the config file contents. Use an int to capture fgetc() return instead of an enum in pg_dump. Fix minor memory leaks in pg_dump. (note minor change to convertOperatorReference()'s API) Check fclose()/remove() return codes in psql. Check fstat(), find_my_exec() return codes in psql. Various ECPG memory leak fixes. Check find_my_exec() return in ECPG. Explicitly ignore pqFlush return in libpq error-path. Change PQfnumber() to avoid doing an strdup() when no changes required. Remove a few useless check-against-NULL's (value deref'd beforehand). Check rmtree(), malloc() results in pg_regress. Also check get_alternative_expectfile() return in pg_regress. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/b1aebbb6a86e96d7b8f3035ac730dfc24652496c Modified Files -- src/backend/libpq/be-secure.c |2 +- src/backend/storage/lmgr/proc.c|3 +- src/bin/initdb/initdb.c| 36 +-- src/bin/pg_ctl/pg_ctl.c| 34 ++ src/bin/pg_dump/pg_backup_archiver.c |4 +- src/bin/pg_dump/pg_dump.c | 79 ++-- src/bin/psql/command.c | 12 +++-- src/bin/psql/copy.c| 15 -- src/bin/psql/startup.c |7 ++- src/interfaces/ecpg/ecpglib/execute.c | 27 +-- src/interfaces/ecpg/preproc/ecpg.c |6 ++- src/interfaces/ecpg/preproc/type.c | 76 ++ src/interfaces/ecpg/preproc/variable.c |5 +- src/interfaces/libpq/fe-connect.c |2 +- src/interfaces/libpq/fe-exec.c | 28 +-- src/test/regress/pg_regress.c | 27 ++- 16 files changed, 290 insertions(+), 73 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Fix crash in json_to_record().
* Jeff Davis (jda...@postgresql.org) wrote: > Fix crash in json_to_record(). > > json_to_record() depends on get_call_result_type() for the tuple > descriptor of the record that should be returned, but in some cases > that cannot be determined. Add a guard to check if the tuple > descriptor has been properly resolved, similar to other callers of > get_call_result_type(). > > Also add guard for two other callers of get_call_result_type() in > jsonfuncs.c. Although json_to_record() is the only actual bug, it's a > good idea to follow convention. Ah, awesome, thanks. I had it in my notes to go check that as Coverity had complained about it and now I don't have to. Thanks again! Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Further pg_dump / ftello improvements
Further pg_dump / ftello improvements Make ftello error-checking consistent to all calls and remove a bit of ftello-related code which has been #if 0'd out since 2001. Note that we are not concerned with the ftello() call under snprintf() failing as it is just building a string to call exit_horribly() with; printing -1 in such a case is fine. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/dfb1e9bdc0d0a506899b11038c7fce9631cac9fe Modified Files -- src/bin/pg_dump/pg_backup_archiver.c |3 +-- src/bin/pg_dump/pg_backup_tar.c | 17 +++-- 2 files changed, 4 insertions(+), 16 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Minor pg_dump improvements
* Tom Lane (t...@sss.pgh.pa.us) wrote: > grep shows me a couple of other places where the result of ftello doesn't > seem to be getting checked for error. Odd that Coverity didn't notice > those. At least two of those are in a #if 0 block... since 2001 (pg_backup_tar.c:_tarGetHeader). I'm thinking we may be better off removing that code rather than continuing to pull it along (and update it- there were at least three commits fixing things in that block after it had been #if 0'd out). Another technically had a check, but it was late, I've got a patch to improve that. The last is inside a snprintf() which is just building a string to call exit_horribly() with- seems like that's safe enough? Thanks, Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Minor pg_dump improvements
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > The only remaining place we still clear errno in pg_dump is in > > pg_backup_archive.c:checkSeek() around a similar ftello call, perhaps > > that should be changed to check the result instead also? > > Agreed; we should be using the same coding pattern wherever we call > ftello. Ok, I'll take care of that in a couple of hours (have to step out for a bit). > I suspect that this code may be left over from coping with some ancient > non-spec-compliant version of ftello? Probably not worth digging in > the archives to find out. The Single Unix Spec v2 says that the result > is (off_t) -1 on error, and we generally assume that platforms are at > least compliant with that. I had been wondering the same, but agree w/ your assessment. > grep shows me a couple of other places where the result of ftello doesn't > seem to be getting checked for error. Odd that Coverity didn't notice > those. I'll try to figure out why it didn't, I would have expected it to. It's possible I just hadn't gotten to those yet or someone decided they were non-issues. Thanks, Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Minor pg_dump improvements
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> I'm pretty sure you broke _CloseArchive with this hunk: > > > That'd be pretty frustrating as my testing didn't exhibit any issues. > > It's possible that errno is accidentally zero when we get there, but > the code as-committed surely can't be called bulletproof. Agreed, oversight on my part. The only remaining place we still clear errno in pg_dump is in pg_backup_archive.c:checkSeek() around a similar ftello call, perhaps that should be changed to check the result instead also? Thanks, Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Focus on ftello result < 0 instead of errno
Focus on ftello result < 0 instead of errno Rather than reset errno (or just hope that its cleared already), check just the result of the ftello for < 0 to determine if there was an issue. Oversight by me, pointed out by Tom. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/5e8e794e3be9fbeddf6f2e2c0515dd0f04c784ec Modified Files -- src/bin/pg_dump/pg_backup_custom.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Minor pg_dump improvements
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost writes:
> > Minor pg_dump improvements
>
> I'm pretty sure you broke _CloseArchive with this hunk:
That'd be pretty frustrating as my testing didn't exhibit any issues.
> @@ -708,6 +708,9 @@ _CloseArchive(ArchiveHandle *AH)
> {
> WriteHead(AH);
> tpos = ftello(AH->FH);
> + if (tpos < 0 || errno)
> + exit_horribly(modulename, "could not determine seek position in
> archive file: %s\n",
> + strerror(errno));
> WriteToc(AH);
> ctx->dataStart = _getFilePos(AH, ctx);
>
> There's no reason to assume errno is zero at entry, and in any case
> it should not be necessary to test for anything except tpos < 0.
Good point.
> I'm not sure why _ReopenArchive is coded like it was; seems like
> tpos < 0 should be a sufficient test there too, and we shouldn't have to
> reset errno beforehand.
Agreed, I'll update that also.
Thanks,
Stephen
signature.asc
Description: Digital signature
[COMMITTERS] pgsql: Minor pg_dump improvements
Minor pg_dump improvements Improve pg_dump by checking results on various fgetc() calls which previously were unchecked, ditto for ftello. Also clean up a couple of very minor memory leaks by waiting to allocate structures until after the initial check(s). Issues spotted by Coverity. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/cfa1b4a711dd03f824a9c3ab50911e61419d1eeb Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 27 +-- src/bin/pg_dump/pg_backup_custom.c |5 - src/bin/pg_dump/pg_dump.c|8 ++-- 3 files changed, 31 insertions(+), 9 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Check dup2() results in syslogger
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> In short, this patch was ill considered. Please revert. If we need > >> to silence a Coverity complaint, perhaps a cast-to-void will do? > > > Sure, I'll adjust it accordingly. > > Feel free to improve the comment if you think it could be clearer. I hemmed and hawed over it and tried to improve it but I'm not convinced that I did. Still, I went ahead and at least got the revert committed. If anyone feels the comment change hurts more than helps, let me know. Thanks, Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Revert dup2() checking in syslogger.c
Revert dup2() checking in syslogger.c Per the expanded comment- As we're just trying to reset these to go to DEVNULL, there's not much point in checking for failure from the close/dup2 calls here, if they fail then presumably the file descriptors are closed and any writes will go into the bitbucket anyway. Pointed out by Tom. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/aef61bf433a9e9b6e2d98b0fdcce8562c3ad526f Modified Files -- src/backend/postmaster/syslogger.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Check dup2() results in syslogger
* Tom Lane (t...@sss.pgh.pa.us) wrote: > You shouldn't really raise that argument against the guy who made the > original commit in question ;-). Figures. :) Not sure how I missed that. [...] Right, I had followed that. > Now ideally, the way we do that is to reconnect its stderr to /dev/null, > but if either the open(DEVNULL) or the dup2() calls were to fail, it will > presumably still work to leave the file descriptors just-plain-closed. > If the syslogger process then attempts to write on stderr, libc's internal > write() calls will fail, but so what? We wanted the output to go to the > bit bucket anyhow. Ok, I see your point that it wouldn't much matter if we tried to complain at this point about the dup2() calls failing. > In short, this patch was ill considered. Please revert. If we need > to silence a Coverity complaint, perhaps a cast-to-void will do? Sure, I'll adjust it accordingly. Thanks, Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Check dup2() results in syslogger
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > Check dup2() results in syslogger > > Consistently check the dup2() call results throughout syslogger.c. > > It's pretty unlikely that they'll error out, but if they do, > > ereport(FATAL) instead of blissfully continuing on. > > Meh. Have you actually tested that an ereport(FATAL) is capable of doing > anything sane right there, with so much syslogger initialization left to > do, and no working stderr? Given that we were already doing so later on in the same function, it struck me as appropriate. I agree that the case is a bit interesting to consider. > Please note also that the comment just above > this implies that we are deliberately ignoring any failures here, so I > think FATAL was probably the wrong thing in any case. We were explicitly ignoring the errors from the close(), I don't believe those comments were intended to apply to the dup2() calls as well, based on my reading of the commit history. > > Spotted by the Coverity scanner. > > I fear this is mere Coverity-appeasement that has broken code that used > to work. Those dup2() calls look likely to only fail in a case of running out of file handles or similar, which struck me as a good reason to ereport(FATAL), similar to the setsid() failure which is checked for below. I could have just done (void) dup2() instead to make it clear that we were intentionally ignoring the results to please Coverity, and probably should have adjusted the close() calls that way. The last adjustment to this code was actually to avoid the dup2() calls causing failures *regardless* of our ignoring the result code due to a Windows' feature in CRT called Paramter Validation (per Heikki's commit message). Heikki, any thoughts on the right answer here? I admit that I didn't go to the effort of forcing the dup2() calls to fail to see what happens, though I did play around w/ killing off the syslogger forcibly and making sure it came back, which appeared to work fine. Thanks, Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Fix minor leak in pg_dump
Fix minor leak in pg_dump Move allocation to after we check the remote server version, to avoid a possible, very minor, memory leak. This makes us more consistent throughout as most places in pg_dump are done in the same way (due, in part, to previous fixes like this). Spotted by the Coverity scanner. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/152d24f5ddbc535bb437b57856fa3c7c5c630472 Modified Files -- src/bin/pg_dump/pg_dump.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Check dup2() results in syslogger
Check dup2() results in syslogger Consistently check the dup2() call results throughout syslogger.c. It's pretty unlikely that they'll error out, but if they do, ereport(FATAL) instead of blissfully continuing on. Spotted by the Coverity scanner. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/790eaa699e4a9626d8a610ec5844e1fd70d73b4e Modified Files -- src/backend/postmaster/syslogger.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Use E, not e, for escaping in example docs
Use E, not e, for escaping in example docs From the Department of Nitpicking, be consistent with other escaping and use 'E' instead of 'e' to escape the string in the example docs for GET DISAGNOSTICS stack = PG_CONTEXT. Noticed by Department Chief Magnus Hagander. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/00ba97365d356823c48c02147b4cd66f8f06b1d6 Modified Files -- doc/src/sgml/plpgsql.sgml |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Avoid minor leak in parallel pg_dump
Avoid minor leak in parallel pg_dump During parallel pg_dump, a worker process closing the connection caused a minor memory leak (particularly minor as we are likely about to exit anyway). Instead, free the memory in this case prior to returning NULL to indicate connection closed. Spotting by the Coverity scanner. Back patch to 9.3 where this was introduced. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/8cb90b21af3cc52c21d8a43e2d9f125113ad9f4f Modified Files -- src/bin/pg_dump/parallel.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Avoid minor leak in parallel pg_dump
Avoid minor leak in parallel pg_dump During parallel pg_dump, a worker process closing the connection caused a minor memory leak (particularly minor as we are likely about to exit anyway). Instead, free the memory in this case prior to returning NULL to indicate connection closed. Spotting by the Coverity scanner. Back patch to 9.3 where this was introduced. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/6794a9f9a194e24862e60a918eac031b7641686c Modified Files -- src/bin/pg_dump/parallel.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: ALTER TABLESPACE ... MOVE ... OWNED BY
ALTER TABLESPACE ... MOVE ... OWNED BY Add the ability to specify the objects to move by who those objects are owned by (as relowner) and change ALL to mean ALL objects. This makes the command always operate against a well-defined set of objects and not have the objects-to-be-moved based on the role of the user running the command. Per discussion with Simon and Tom. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/fbe19ee3b87590f1006d072be5fecf8a33d4e9f5 Modified Files -- doc/src/sgml/ref/alter_tablespace.sgml | 35 +- src/backend/commands/tablespace.c | 29 +++ src/backend/commands/user.c|3 +- src/backend/nodes/copyfuncs.c |3 ++ src/backend/nodes/equalfuncs.c |3 ++ src/backend/parser/gram.y | 63 +--- src/include/commands/user.h|1 + src/include/nodes/parsenodes.h |5 ++- 8 files changed, 115 insertions(+), 27 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow type_func_name_keywords in even more places
Allow type_func_name_keywords in even more places A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed type_func_name_keywords to be used in more places, including role identifiers. Unfortunately, that commit missed out on cases where name_list was used for lists-of-roles, eg: for DROP ROLE. This resulted in the unfortunate situation that you could CREATE a role with a type_func_name_keywords-allowed identifier, but not DROP it (directly- ALTER could be used to rename it to something which could be DROP'd). This extends allowing type_func_name_keywords to places where role lists can be used. Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/cbd850bf609c5b1d51874ba9611723cb17da8626 Modified Files -- src/backend/parser/gram.y | 48 + 1 file changed, 27 insertions(+), 21 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow type_func_name_keywords in even more places
Allow type_func_name_keywords in even more places A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed type_func_name_keywords to be used in more places, including role identifiers. Unfortunately, that commit missed out on cases where name_list was used for lists-of-roles, eg: for DROP ROLE. This resulted in the unfortunate situation that you could CREATE a role with a type_func_name_keywords-allowed identifier, but not DROP it (directly- ALTER could be used to rename it to something which could be DROP'd). This extends allowing type_func_name_keywords to places where role lists can be used. Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/d1e3070f0483cef09ebfa91772e70e604f2ad2db Modified Files -- src/backend/parser/gram.y | 46 + 1 file changed, 26 insertions(+), 20 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow type_func_name_keywords in even more places
Allow type_func_name_keywords in even more places A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed type_func_name_keywords to be used in more places, including role identifiers. Unfortunately, that commit missed out on cases where name_list was used for lists-of-roles, eg: for DROP ROLE. This resulted in the unfortunate situation that you could CREATE a role with a type_func_name_keywords-allowed identifier, but not DROP it (directly- ALTER could be used to rename it to something which could be DROP'd). This extends allowing type_func_name_keywords to places where role lists can be used. Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was. Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/f2eede9b584305472dd3fd58e1690766c378ab34 Modified Files -- src/backend/parser/gram.y | 48 + 1 file changed, 27 insertions(+), 21 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow type_func_name_keywords in even more places
Allow type_func_name_keywords in even more places A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed type_func_name_keywords to be used in more places, including role identifiers. Unfortunately, that commit missed out on cases where name_list was used for lists-of-roles, eg: for DROP ROLE. This resulted in the unfortunate situation that you could CREATE a role with a type_func_name_keywords-allowed identifier, but not DROP it (directly- ALTER could be used to rename it to something which could be DROP'd). This extends allowing type_func_name_keywords to places where role lists can be used. Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/c0e6169e16a84a55f6b3c1f802fe74004cc81592 Modified Files -- src/backend/parser/gram.y | 46 + 1 file changed, 26 insertions(+), 20 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow type_func_name_keywords in even more places
Allow type_func_name_keywords in even more places A while back, 2c92edad48796119c83d7dbe6c33425d1924626d allowed type_func_name_keywords to be used in more places, including role identifiers. Unfortunately, that commit missed out on cases where name_list was used for lists-of-roles, eg: for DROP ROLE. This resulted in the unfortunate situation that you could CREATE a role with a type_func_name_keywords-allowed identifier, but not DROP it (directly- ALTER could be used to rename it to something which could be DROP'd). This extends allowing type_func_name_keywords to places where role lists can be used. Back-patch to 9.0, as 2c92edad48796119c83d7dbe6c33425d1924626d was. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/6c36f383df728866d7085c155cbe45ebc07b195f Modified Files -- src/backend/parser/gram.y | 46 + 1 file changed, 26 insertions(+), 20 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add CREATE TABLESPACE ... WITH ... Options
Add CREATE TABLESPACE ... WITH ... Options Tablespaces have a few options which can be set on them to give PG hints as to how the tablespace behaves (perhaps it's faster for sequential scans, or better able to handle random access, etc). These options were only available through the ALTER TABLESPACE command. This adds the ability to set these options at CREATE TABLESPACE time, removing the need to do both a CREATE TABLESPACE and ALTER TABLESPACE to get the correct options set on the tablespace. Vik Fearing, reviewed by Michael Paquier. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/5254958e924cd54f33d37026d85483fef986060d Modified Files -- doc/src/sgml/ref/create_tablespace.sgml | 23 ++- src/backend/commands/tablespace.c | 12 +++- src/backend/nodes/copyfuncs.c |1 + src/backend/nodes/equalfuncs.c|1 + src/backend/parser/gram.y |3 ++- src/include/nodes/parsenodes.h|1 + src/test/regress/input/tablespace.source | 10 ++ src/test/regress/output/tablespace.source | 13 + 8 files changed, 61 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Add ALTER TABLESPACE ... MOVE command This adds a 'MOVE' sub-command to ALTER TABLESPACE which allows moving sets of objects from one tablespace to another. This can be extremely handy and avoids a lot of error-prone scripting. ALTER TABLESPACE ... MOVE will only move objects the user owns, will notify the user if no objects were found, and can be used to move ALL objects or specific types of objects (TABLES, INDEXES, or MATERIALIZED VIEWS). Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/76e91b38ba64e1da70ea21744b342cb105ea3400 Modified Files -- doc/src/sgml/ref/alter_tablespace.sgml| 59 +- src/backend/commands/tablespace.c | 171 + src/backend/nodes/copyfuncs.c | 15 +++ src/backend/nodes/equalfuncs.c| 14 +++ src/backend/parser/gram.y | 46 +++- src/backend/tcop/utility.c| 14 +++ src/include/commands/tablespace.h |1 + src/include/nodes/nodes.h |1 + src/include/nodes/parsenodes.h| 10 ++ src/include/parser/kwlist.h |1 + src/test/regress/input/tablespace.source |7 +- src/test/regress/output/tablespace.source |8 +- src/tools/pgindent/typedefs.list |1 + 13 files changed, 340 insertions(+), 8 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/d2636486b3fe13d855b0109f15efaa5f4e00adef Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- REL8_4_STABLE Details --- http://git.postgresql.org/pg/commitdiff/0fb4e3cebb32628bdd92d8445ff61d23eb73af48 Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/86e58ae023a93e778cec195b19ffe7ccb01f7038 Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/1fe06595ab9124e47a6d7935e1796e7c31c03c1f Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/6f25c62d788ea6312fe718ed57a3d169d8efc066 Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow SET TABLESPACE to database default
Allow SET TABLESPACE to database default We've always allowed CREATE TABLE to create tables in the database's default tablespace without checking for CREATE permissions on that tablespace. Unfortunately, the original implementation of ALTER TABLE ... SET TABLESPACE didn't pick up on that exception. This changes ALTER TABLE ... SET TABLESPACE to allow the database's default tablespace without checking for CREATE rights on that tablespace, just as CREATE TABLE works today. Users could always do this through a series of commands (CREATE TABLE ... AS SELECT * FROM ...; DROP TABLE ...; etc), so let's fix the oversight in SET TABLESPACE's original implementation. Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/e70c4282136e07ab3a0e57f3c811de80e4318d9f Modified Files -- src/backend/commands/tablecmds.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/b37c90f11e3c239b999f98ffd3bbea6b8253fffa Modified Files -- src/interfaces/libpq/fe-secure.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/5eaa369e2e669e0928f2531329825cf4f4d5c884 Modified Files -- src/interfaces/libpq/fe-secure.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/73c4e527a4d264d632ef29708a13b53883600ff5 Modified Files -- src/interfaces/libpq/fe-secure.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/1ba5fe8dd4e35cba574858538c689aabdea25e73 Modified Files -- src/interfaces/libpq/fe-secure.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- REL8_4_STABLE Details --- http://git.postgresql.org/pg/commitdiff/d6dc44a87928fe2b40127ae7001aca1469d69fce Modified Files -- src/interfaces/libpq/fe-secure.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix SSL deadlock risk in libpq
Fix SSL deadlock risk in libpq In libpq, we set up and pass to OpenSSL callback routines to handle locking. When we run out of SSL connections, we try to clean things up by de-registering the hooks. Unfortunately, we had a few calls into the OpenSSL library after these hooks were de-registered during SSL cleanup which lead to deadlocking. This moves the thread callback cleanup to be after all SSL-cleanup related OpenSSL library calls. I've been unable to reproduce the deadlock with this fix. In passing, also move the close_SSL call to be after unlocking our ssl_config mutex when in a failure state. While it looks pretty unlikely to be an issue, it could have resulted in deadlocks if we ended up in this code path due to something other than SSL_new failing. Thanks to Heikki for pointing this out. Back-patch to all supported versions; note that the close_SSL issue only goes back to 9.0, so that hunk isn't included in the 8.4 patch. Initially found and reported by Vesa-Matti J Kari; many thanks to both Heikki and Andres for their help running down the specific issue and reviewing the patch. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/d707a8ccd87d2a3d307c7bd19b1d30d3cb9b3ca5 Modified Files -- src/interfaces/libpq/fe-secure.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Improve handling of pthread_mutex_lock error case
Improve handling of pthread_mutex_lock error case We should really be reporting a useful error along with returning a valid return code if pthread_mutex_lock() throws an error for some reason. Add that and back-patch to 9.0 as the prior patch. Pointed out by Alvaro Herrera Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/71127756af12ed64da9e82639f88b6ed13e5f600 Modified Files -- src/interfaces/libpq/fe-secure.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Improve handling of pthread_mutex_lock error case
Improve handling of pthread_mutex_lock error case We should really be reporting a useful error along with returning a valid return code if pthread_mutex_lock() throws an error for some reason. Add that and back-patch to 9.0 as the prior patch. Pointed out by Alvaro Herrera Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/f019c021119c632da396894769349fa6433e91da Modified Files -- src/interfaces/libpq/fe-secure.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Improve handling of pthread_mutex_lock error case
Improve handling of pthread_mutex_lock error case We should really be reporting a useful error along with returning a valid return code if pthread_mutex_lock() throws an error for some reason. Add that and back-patch to 9.0 as the prior patch. Pointed out by Alvaro Herrera Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/820739cba95622033527f60467a264db0ee91f76 Modified Files -- src/interfaces/libpq/fe-secure.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Improve handling of pthread_mutex_lock error case
Improve handling of pthread_mutex_lock error case We should really be reporting a useful error along with returning a valid return code if pthread_mutex_lock() throws an error for some reason. Add that and back-patch to 9.0 as the prior patch. Pointed out by Alvaro Herrera Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/1e8e324326be8fa907b51ba262cf0a21cb015ca3 Modified Files -- src/interfaces/libpq/fe-secure.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Improve handling of pthread_mutex_lock error case
Improve handling of pthread_mutex_lock error case We should really be reporting a useful error along with returning a valid return code if pthread_mutex_lock() throws an error for some reason. Add that and back-patch to 9.0 as the prior patch. Pointed out by Alvaro Herrera Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/8359ed806f3300b79f110f1ac216c58c0732d05c Modified Files -- src/interfaces/libpq/fe-secure.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add locking around SSL_context usage in libpq
Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/f31016365565e03c3c698554eacc1c897fd35049 Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add locking around SSL_context usage in libpq
Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/0b821b8d7c137fbec215f8da286b4a40e53b13d5 Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add locking around SSL_context usage in libpq
Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/aad2a630b1b163038ea904e16a59e409020f5828 Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add locking around SSL_context usage in libpq
Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/55754380f36d098551bd55dd49e27f64dd1c8d2f Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Add locking around SSL_context usage in libpq
Add locking around SSL_context usage in libpq I've been working with Nick Phillips on an issue he ran into when trying to use threads with SSL client certificates. As it turns out, the call in initialize_SSL() to SSL_CTX_use_certificate_chain_file() will modify our SSL_context without any protection from other threads also calling that function or being at some other point and trying to read from SSL_context. To protect against this, I've written up the attached (based on an initial patch from Nick and much subsequent discussion) which puts locks around SSL_CTX_use_certificate_chain_file() and all of the other users of SSL_context which weren't already protected. Nick Phillips, much reworked by Stephen Frost Back-patch to 9.0 where we started loading the cert directly instead of using a callback. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/9d7f66bc6c620f8c7548fc65d0e8e160615d5267 Modified Files -- src/interfaces/libpq/fe-secure.c | 56 -- 1 file changed, 53 insertions(+), 3 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Allow a context to be passed in for error handling
Allow a context to be passed in for error handling As pointed out by Tom Lane, we can allow other users of the error handler callbacks to provide their own memory context by adding the context to use to ErrorData and using that instead of explicitly using ErrorContext. This then allows GetErrorContextStack() to be called from inside exception handlers, so modify plpgsql to take advantage of that and add an associated regression test for it. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/ddef1a39c6798ffae899acd08ff92329dd304085 Modified Files -- src/backend/utils/error/elog.c| 126 - src/include/utils/elog.h |3 + src/pl/plpgsql/src/pl_gram.y |4 +- src/test/regress/expected/plpgsql.out | 98 + src/test/regress/sql/plpgsql.sql | 56 +++ 5 files changed, 221 insertions(+), 66 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
* Tom Lane (t...@sss.pgh.pa.us) wrote: > [...] a better idea is to add a memory-context-to-use field to struct > ErrorData. We'd initialize it to ErrorContext when pushing a stack > entry for normal error processing, but GetErrorContextStack could > do something different. This would eliminate most of the explicit > references to ErrorContext in elog.c. Ah, fantastic idea. > If you want I'll draft something up. I'll take care of it, thanks for the idea! Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Pavel,
First, please only quote the relevant parts of the email when
responding.
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
> I used a ErrorContext because I wasn't sure so errcontext and similar
> function can work in different context. Now I look there and there
> should be well initialized ErrorDataStack, due
>
> int
> set_errcontext_domain(const char *domain)
> {
> <-->ErrorData *edata = &errordata[errordata_stack_depth];
>
> <-->/* we don't bother incrementing recursion_depth */
> <-->CHECK_STACK_DEPTH();
>
> <-->edata->context_domain = domain;
>
> <-->return 0;
> }
>
> but MemoryContext can be any - so probably some private context is ideal.
While set_errcontext_domain() doesn't care about the MemoryContext, per
se, the errcontext() macro further calls errcontext_msg() which is
currently set up to explicitly use ErrorContext. Perhaps an elog.c
global to tell errcontext_msg() to not switch memory contexts, but what
happens if there's an error thrown by a callback function..?
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
* Tom Lane (t...@sss.pgh.pa.us) wrote: > 1. Just run the whole business in the caller's context and leave it up > to the caller to worry about whether it needs to clean up memory usage. I'd certainly be fine with that, and had considered it, but it looks like errcontext_msg() (which is called by the callbacks through the errcontext() macro) switches to ErrorContext for its work, meaning we have to clean up that context anyway. In my initial review I hadn't considered the possibility of modifying what ErrorContext is pointing to. As the error system may end up getting called by a callback function, it seems like changing the global ErrorContext would be a bad idea. Forgive me if I'm missing something obvious in how we could simply use the caller's context for this as I'd surely be much happier with that. > 2. Create a temporary context underneath CurrentMemoryContext, use that, > and then delete it when done. That'd be fine with me also, but runs the same issue as above. > The only thing that needs to be considered an error condition is if the > errordata stack is too full to allow creation of the extra entry needed > by this function, which is an improbable situation. Other than > temporarily stacking and then unstacking that entry, you don't need to > have any side-effects on the state of the error subsystem. If we can address the memory context issue in a simple way then I'll certainly set this up to be reentrant safe and to handle pushing and popping itself on the errordata stack. > One other minor gripe is that the function is documented as returning > the "call stack", which a C programmer would think means something > entirely different from what is actually output. I think you need to > use a different phrase there. "error context stack" would be OK. Works for me. I had already tried to reword it to address that exact issue, but "error context stack" does sound better than "PG call stack." Thanks! Stephen signature.asc Description: Digital signature
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > I'm not following your reasoning here. This *has* to be called in an > error case, before you're outside the error processing context. > Otherwise there would be no data available to be printed. > > In short: FlushErrorState, by definition, destroys the information that > GetErrorContextStack looks at. So in the current implementation, > GetErrorContextStack is burning its bridges behind it. That's at the > very least a surprising behavior. I am betting that it will have > unpleasant consequences for any sort of nested-error scenario. I've just pushed up some much needed improvements to the comments which hopefully clarify that this function is using error_context_stack and calling the callbacks set up there by callers above on the PG call stack. Also, hopefully this makes it clear that errrordata is required to be empty when this function is called and therefore it can be cleaned up when exiting with FlushErrorState. Perhaps it would be better to try and work out a way for this to be reentrant safe and be callable from an exception handler, but it certainly wasn't part of the original intent and being able to support either being called under an exception handler or not would essentially require checking if anything is above us on the stack and, if not, cleaning things up anyway, or trusting the above caller to handle it and skipping it. I'm happy to rework this or even revert it if this use of the error_context_stack is just too grotty, but this certainly looks like a useful capability to have. Thanks! Stephen signature.asc Description: Digital signature
[COMMITTERS] pgsql: Improvements to GetErrorContextStack()
Improvements to GetErrorContextStack() As GetErrorContextStack() borrowed setup and tear-down code from other places, it was less than clear that it must only be called as a top-level entry point into the error system and can't be called by an exception handler (unlike the rest of the error system, which is set up to be reentrant-safe). Being called from an exception handler is outside the charter of GetErrorContextStack(), so add a bit more protection against it, improve the comments addressing why we have to set up an errordata stack for this function at all, and add a few more regression tests. Lack of clarity pointed out by Tom Lane; all bugs are mine. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/9bd0feeba85fae411e01798d5a5d76b70333e38e Modified Files -- src/backend/utils/error/elog.c| 50 +++--- src/test/regress/expected/plpgsql.out | 62 +++-- src/test/regress/sql/plpgsql.sql | 18 -- 3 files changed, 105 insertions(+), 25 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
On Thursday, July 25, 2013, Stephen Frost wrote: > On Wednesday, July 24, 2013, Stephen Frost wrote: > >> Unfortunately, I don't have the code in front of me at the moment, but I >> was pretty sure FlushErrorState cleans up the intermediate information set >> up during an individual error call and doesn't completely clear the stack >> info (tho I can't think of what, if anything, does..) or multiple "raise >> notices" wouldn't each contain the stack info in their output.. >> >> I'll certainly look through this again and dream up some additional test >> cases for it, in the morning. >> > > Couldn't help if- reading code on a phone isn't ideal, but that's a > different discussion. > > You're right- resetting the stack depth doesn't make any sense here, which > FlushErrorState does. I think clearing the memory context should be > alright but that's a different story. Curious that the raise notice in the > regression test didn't blow up, which is part of why I thought it was fine, > but a subsequent call to raise notice in the same function would be a good > test (along with another call to this function..) and I think wouldn't > produce a stack trace here, which would be quite wrong. > > Will address once I'm back I front of something I can actually write code > on. > Alright, no, and clearly I should have run this down before committing it, but I knew the raise notice after the call to get diag meant we must not be completely destroying the stack. The errdata stack is different from the context stack info. errdata is for reentrant error calls, which this function should never be a part of. Were that to happen, at least the Assert around the recursion check should fire. As for if we *should* call FlushErrorState, I think we need to, to reset ErrorContext and move the errdata stack depth back to -1, where it should be. Now, we might just forgo adding ourselves onto the stack, as we don't set a callback anyway, but the errdata stack depth should certainly be at -1 when we leave, or we haven't prevented reentrant calls into GetErrorStack. This definitely deserves more commentary and I'll see about adding that also. Thanks for reviewing! Stephen
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
On Wednesday, July 24, 2013, Stephen Frost wrote: > Unfortunately, I don't have the code in front of me at the moment, but I > was pretty sure FlushErrorState cleans up the intermediate information set > up during an individual error call and doesn't completely clear the stack > info (tho I can't think of what, if anything, does..) or multiple "raise > notices" wouldn't each contain the stack info in their output.. > > I'll certainly look through this again and dream up some additional test > cases for it, in the morning. > Couldn't help if- reading code on a phone isn't ideal, but that's a different discussion. You're right- resetting the stack depth doesn't make any sense here, which FlushErrorState does. I think clearing the memory context should be alright but that's a different story. Curious that the raise notice in the regression test didn't blow up, which is part of why I thought it was fine, but a subsequent call to raise notice in the same function would be a good test (along with another call to this function..) and I think wouldn't produce a stack trace here, which would be quite wrong. Will address once I'm back I front of something I can actually write code on. Thanks, Stephen
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
On Wednesday, July 24, 2013, Tom Lane wrote: > Stephen Frost > writes: > > That said, I'll work on making this more independent of the error > handling > > and see if it can be made to use an independent memory context and try to > > tighten it up to ensure it isn't called in an error case. Future callers > > may try to. > > I'm not following your reasoning here. This *has* to be called in an > error case, before you're outside the error processing context. > Otherwise there would be no data available to be printed. It has to be called after the context callbacks have been set up by the levels above it on the stack (eg: SPI calls), but that's not the same as being called from an actual exception. The idea here is to simply execute the callbacks for each level above it on the stack and collect the strings they create with errcontext(). In short: FlushErrorState, by definition, destroys the information that > GetErrorContextStack looks at. So in the current implementation, > GetErrorContextStack is burning its bridges behind it. That's at the > very least a surprising behavior. I am betting that it will have > unpleasant consequences for any sort of nested-error scenario. > Unfortunately, I don't have the code in front of me at the moment, but I was pretty sure FlushErrorState cleans up the intermediate information set up during an individual error call and doesn't completely clear the stack info (tho I can't think of what, if anything, does..) or multiple "raise notices" wouldn't each contain the stack info in their output.. I'll certainly look through this again and dream up some additional test cases for it, in the morning. Thanks, Stephen
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
On Wednesday, July 24, 2013, Stephen Frost wrote: > Still, if there's a reasonable way to collect the stack info without going > through the error callbacks, without implementing a duplicate system, I'm > all ears. > That said, I'll work on making this more independent of the error handling and see if it can be made to use an independent memory context and try to tighten it up to ensure it isn't called in an error case. Future callers may try to. Thanks, Stephen
Re: [COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Tom, On Wednesday, July 24, 2013, Tom Lane wrote: > Stephen Frost > writes: > > Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL > > I don't find it to be a terribly good idea that GetErrorContextStack > does FlushErrorState(). Doesn't that imply that it can't safely be > used from inside error processing, which is more or less exactly > where it is intended to be used? I would certainly think it surprising > that that call destroys all information about the error. It's not intended (nor allowed, if I got it right) in an error context. I will admit that it's not a wonderful situation to have it using the error handling's internal components like this, but that's also where it has to go for the callbacks to get the information needed. > For the same reason, it's rather dubious that it uses ErrorContext as > working space. There might not be a heck of a lot of space left there, > and I don't think that construction of this string really counts as > error processing. It seems to me to be something done outside the error > subsystem. > My concerns and thoughts around this were what may happen if a callback throws an error and it still makes me a bit nervous but It seems like it should work. Still, if there's a reasonable way to collect the stack info without going through the error callbacks, without implementing a duplicate system, I'm all ears. Thanks! Stephen
[COMMITTERS] pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL This adds the ability to get the call stack as a string from within a PL/PgSQL function, which can be handy for logging to a table, or to include in a useful message to an end-user. Pavel Stehule, reviewed by Rushabh Lathia and rather heavily whacked around by Stephen Frost. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/831283256796d1c20858862b568d73e505eb4a84 Modified Files -- doc/src/sgml/plpgsql.sgml | 57 +++ src/backend/utils/error/elog.c| 69 + src/include/utils/elog.h |2 + src/pl/plpgsql/src/pl_exec.c | 10 + src/pl/plpgsql/src/pl_funcs.c |2 + src/pl/plpgsql/src/pl_gram.y |6 +++ src/pl/plpgsql/src/pl_scanner.c |1 + src/pl/plpgsql/src/plpgsql.h |1 + src/test/regress/expected/plpgsql.out | 48 +++ src/test/regress/sql/plpgsql.sql | 33 10 files changed, 229 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: WITH CHECK OPTION support for auto-updatable VIEWs
WITH CHECK OPTION support for auto-updatable VIEWs For simple views which are automatically updatable, this patch allows the user to specify what level of checking should be done on records being inserted or updated. For 'LOCAL CHECK', new tuples are validated against the conditionals of the view they are being inserted into, while for 'CASCADED CHECK' the new tuples are validated against the conditionals for all views involved (from the top down). This option is part of the SQL specification. Dean Rasheed, reviewed by Pavel Stehule Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/4cbe3ac3e86790d05c569de4585e5075a62a9b41 Modified Files -- doc/src/sgml/ref/alter_view.sgml |5 + doc/src/sgml/ref/create_view.sgml | 199 ++ src/backend/access/common/reloptions.c| 14 + src/backend/catalog/information_schema.sql|8 +- src/backend/catalog/sql_features.txt |4 +- src/backend/commands/tablecmds.c | 36 +++ src/backend/commands/view.c | 68 + src/backend/executor/execMain.c | 43 +++ src/backend/executor/nodeModifyTable.c| 33 +++ src/backend/nodes/copyfuncs.c | 18 ++ src/backend/nodes/equalfuncs.c| 15 + src/backend/nodes/nodeFuncs.c | 14 + src/backend/nodes/outfuncs.c | 15 + src/backend/nodes/readfuncs.c | 18 ++ src/backend/optimizer/plan/createplan.c | 15 +- src/backend/optimizer/plan/planner.c | 30 +- src/backend/parser/gram.y | 43 ++- src/backend/rewrite/rewriteHandler.c | 117 +++- src/bin/pg_dump/pg_dump.c | 16 +- src/bin/pg_dump/pg_dump.h |1 + src/include/catalog/catversion.h |2 +- src/include/commands/view.h |2 + src/include/executor/executor.h |2 + src/include/nodes/execnodes.h |4 + src/include/nodes/nodes.h |1 + src/include/nodes/parsenodes.h| 23 ++ src/include/nodes/plannodes.h |1 + src/include/optimizer/planmain.h |3 +- src/include/rewrite/rewriteHandler.h |4 + src/include/utils/rel.h | 34 +++ src/test/regress/expected/create_view.out |2 +- src/test/regress/expected/updatable_views.out | 363 + src/test/regress/sql/updatable_views.sql | 199 ++ 33 files changed, 1245 insertions(+), 107 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Use correct parameter name for view_option_value
Use correct parameter name for view_option_value The documentation for ALTER VIEW had a minor copy-and-paste error in defining the parameters. Noticed when reviewing the WITH CHECK OPTION patch. Backpatch to 9.2 where this was first introduced. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/f2719f6975398727ed492ecbcb2028bd134e5f6b Modified Files -- doc/src/sgml/ref/alter_view.sgml |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Use correct parameter name for view_option_value
Use correct parameter name for view_option_value The documentation for ALTER VIEW had a minor copy-and-paste error in defining the parameters. Noticed when reviewing the WITH CHECK OPTION patch. Backpatch to 9.2 where this was first introduced. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/cce5d681be7abfd9f48c28151ebf2b242f8ba438 Modified Files -- doc/src/sgml/ref/alter_view.sgml |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Use correct parameter name for view_option_value
Use correct parameter name for view_option_value The documentation for ALTER VIEW had a minor copy-and-paste error in defining the parameters. Noticed when reviewing the WITH CHECK OPTION patch. Backpatch to 9.2 where this was first introduced. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/dd8ea2eb5e996f3f3dfd928e20aa2462c4bd9c63 Modified Files -- doc/src/sgml/ref/alter_view.sgml |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Check get_tle_by_resno() result before deref
Check get_tle_by_resno() result before deref When creating a sort to support a group by, we need to look up the target entry in the target list by the resno using get_tle_by_resno(). This particular code-path didn't check the result prior to attempting to dereference it, while all other callers did. While I can't see a way for this usage of get_tle_by_resno() to fail (you can't ask for a column to be sorted on which isn't included in the group by), it's probably best to check that we didn't end up with a NULL somehow anyway than risk the segfault. I'm willing to back-patch this if others feel it's necessary, but my guess is new features are what might tickle this rather than anything existing. Missing check spotted by the Coverity scanner. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/4ed22e891f9915b02b753ee8763a8f2438234fc6 Modified Files -- src/backend/optimizer/plan/createplan.c |3 +++ 1 file changed, 3 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Clean up pg_basebackup libpq usage
Clean up pg_basebackup libpq usage When using libpq, it's generally preferrable to just use the strings which are in the PQ structures instead of copying them out, so do that instead in BaseBackup(), eliminating the strcpy()'s used there. Also, in ReceiveAndUnpackTarFile(), check the string length for the directory returned by the server for the tablespace path. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/2f397a08de49e28e3430ff7278f4648757dea2a3 Modified Files -- src/bin/pg_basebackup/pg_basebackup.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Correct off-by-one when reading from pipe
Correct off-by-one when reading from pipe In pg_basebackup.c:reached_end_position(), we're reading from an internal pipe with our own background process but we're possibly reading more bytes than will actually fit into our buffer due to an off-by-one error. As we're reading from an internal pipe there's no real risk here, but it's good form to not depend on such convenient arrangements. Bug spotted by the Coverity scanner. Back-patch to 9.2 where this showed up. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/03010366b6fb61aac0998f234478cc745ff97b0c Modified Files -- src/bin/pg_basebackup/pg_basebackup.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Correct off-by-one when reading from pipe
Correct off-by-one when reading from pipe In pg_basebackup.c:reached_end_position(), we're reading from an internal pipe with our own background process but we're possibly reading more bytes than will actually fit into our buffer due to an off-by-one error. As we're reading from an internal pipe there's no real risk here, but it's good form to not depend on such convenient arrangements. Bug spotted by the Coverity scanner. Back-patch to 9.2 where this showed up. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/a3957ac156da7e31da46a2d8e3b6c7669333cc99 Modified Files -- src/bin/pg_basebackup/pg_basebackup.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Correct off-by-one when reading from pipe
Correct off-by-one when reading from pipe In pg_basebackup.c:reached_end_position(), we're reading from an internal pipe with our own background process but we're possibly reading more bytes than will actually fit into our buffer due to an off-by-one error. As we're reading from an internal pipe there's no real risk here, but it's good form to not depend on such convenient arrangements. Bug spotted by the Coverity scanner. Back-patch to 9.2 where this showed up. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/22b7f5c5aa1dc2909e110b171b03d6e0c85dcd43 Modified Files -- src/bin/pg_basebackup/pg_basebackup.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Check version before allocating PQExpBuffer
Check version before allocating PQExpBuffer In pg_dump.c:getEventTriggers, check what major version we are on before calling createPQExpBuffer() to avoid leaking that bit of memory. Leak discovered by the Coverity scanner. Back-patch to 9.3 where support for dumping event triggers was added. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/3355443fb188b86d59ca90912d5456b427c29116 Modified Files -- src/bin/pg_dump/pg_dump.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Check version before allocating PQExpBuffer
Check version before allocating PQExpBuffer In pg_dump.c:getEventTriggers, check what major version we are on before calling createPQExpBuffer() to avoid leaking that bit of memory. Leak discovered by the Coverity scanner. Back-patch to 9.3 where support for dumping event triggers was added. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/8126bfb5b5f0b413455edd23ff3bf08d83f1cddc Modified Files -- src/bin/pg_dump/pg_dump.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Fix resource leak in initdb -X option
Fix resource leak in initdb -X option When creating the symlink for the xlog directory, free the string which stores the link location. Not really an issue but it doesn't hurt to be good about this- prior cleanups have fixed similar issues. Leak found by the Coverity scanner. Not back-patching as I don't see it being worth the code churn. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/5461d36b5b7d99e237b2f63a7975e6430727cb0b Modified Files -- src/bin/initdb/initdb.c |1 + 1 file changed, 1 insertion(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Be sure to close() file descriptor on error case
Be sure to close() file descriptor on error case In receivelog.c:writeTimeLineHistoryFile(), we were not properly closing the open'd file descriptor in error cases. While this wouldn't matter much if we were about to exit due to such an error, that's not the case with pg_receivexlog as it can be a long-running process and these errors are non-fatal. This resource leak was found by the Coverity scanner. Back-patch to 9.3 where this issue first appeared. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/cec62efd0e551a56635b47ea4185ec27a6840de7 Modified Files -- src/bin/pg_basebackup/receivelog.c |2 ++ 1 file changed, 2 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Be sure to close() file descriptor on error case
Be sure to close() file descriptor on error case In receivelog.c:writeTimeLineHistoryFile(), we were not properly closing the open'd file descriptor in error cases. While this wouldn't matter much if we were about to exit due to such an error, that's not the case with pg_receivexlog as it can be a long-running process and these errors are non-fatal. This resource leak was found by the Coverity scanner. Back-patch to 9.3 where this issue first appeared. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/b68a1fc7ff1a50a7282e8edff7c51333274c3334 Modified Files -- src/bin/pg_basebackup/receivelog.c |2 ++ 1 file changed, 2 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- REL9_2_STABLE Details --- http://git.postgresql.org/pg/commitdiff/89c09fe02d9aa77aea28525b97229f61f1d5e471 Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/e9010b992640d1dbf212cbbab40a00093515f16f Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/273dcd16282c8014a14a9ecbf467459b8702e745 Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- REL9_1_STABLE Details --- http://git.postgresql.org/pg/commitdiff/3cb7a393e8e93b1e67bb1e0d361dd1eb7d928ae7 Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- REL9_0_STABLE Details --- http://git.postgresql.org/pg/commitdiff/5174bc2240c7e873912b4061285df2d475ce6639 Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Ensure 64bit arithmetic when calculating tapeSpace
Ensure 64bit arithmetic when calculating tapeSpace In tuplesort.c:inittapes(), we calculate tapeSpace by first figuring out how many 'tapes' we can use (maxTapes) and then multiplying the result by the tape buffer overhead for each. Unfortunately, when we are on a system with an 8-byte long, we allow work_mem to be larger than 2GB and that allows maxTapes to be large enough that the 32bit arithmetic can overflow when multiplied against the buffer overhead. When this overflow happens, we end up adding the overflow to the amount of space available, causing the amount of memory allocated to be larger than work_mem. Note that to reach this point, you have to set work mem to at least 24GB and be sorting a set which is at least that size. Given that a user who can set work_mem to 24GB could also set it even higher, if they were looking to run the system out of memory, this isn't considered a security issue. This overflow risk was found by the Coverity scanner. Back-patch to all supported branches, as this issue has existed since before 8.4. Branch -- REL8_4_STABLE Details --- http://git.postgresql.org/pg/commitdiff/4285fb9ff5adb8449576b242908ae646a0e959b4 Modified Files -- src/backend/utils/sort/tuplesort.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: pg_receivexlog - Exit on failure to parse
pg_receivexlog - Exit on failure to parse In streamutil.c:GetConnection(), upgrade failure to parse the connection string to an exit(1) instead of simply returning NULL. Most callers already immediately exited, but pg_receivexlog would loop on this case, continually trying to re-parse the connection string (which can't be changed after pg_receivexlog has started). GetConnection() was already expected to exit(1) in some cases (eg: failure to allocate memory or if unable to determine the integer_datetimes flag), so this change shouldn't surprise anyone. Began looking at this due to the Coverity scanner complaining that we were leaking err_msg in this case- no longer an issue since we just exit(1) immediately. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/d368a301b3a4bf5fec17e81c630adddeac80a7fc Modified Files -- src/bin/pg_basebackup/streamutil.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: pg_receivexlog - Exit on failure to parse
pg_receivexlog - Exit on failure to parse In streamutil.c:GetConnection(), upgrade failure to parse the connection string to an exit(1) instead of simply returning NULL. Most callers already immediately exited, but pg_receivexlog would loop on this case, continually trying to re-parse the connection string (which can't be changed after pg_receivexlog has started). GetConnection() was already expected to exit(1) in some cases (eg: failure to allocate memory or if unable to determine the integer_datetimes flag), so this change shouldn't surprise anyone. Began looking at this due to the Coverity scanner complaining that we were leaking err_msg in this case- no longer an issue since we just exit(1) immediately. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/f5acde9380e164eab10b2dc3281bb1c07f690803 Modified Files -- src/bin/pg_basebackup/streamutil.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: During parallel pg_dump, free commands from master
During parallel pg_dump, free commands from master The command strings read by the child processes during parallel pg_dump, after being read and handled, were not being free'd. This patch corrects this relatively minor memory leak. Leak found by the Coverity scanner. Back patch to 9.3 where parallel pg_dump was introduced. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/234e4cf6e1eac2f0e514379a2a533ffb71b33732 Modified Files -- src/bin/pg_dump/parallel.c |3 +++ 1 file changed, 3 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: During parallel pg_dump, free commands from master
During parallel pg_dump, free commands from master The command strings read by the child processes during parallel pg_dump, after being read and handled, were not being free'd. This patch corrects this relatively minor memory leak. Leak found by the Coverity scanner. Back patch to 9.3 where parallel pg_dump was introduced. Branch -- REL9_3_STABLE Details --- http://git.postgresql.org/pg/commitdiff/8839e7362c68470f8db66acdfa60b95a1c5312cf Modified Files -- src/bin/pg_dump/parallel.c |3 +++ 1 file changed, 3 insertions(+) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Additional spelling corrections
Additional spelling corrections A few more minor spelling corrections, no functional changes. Thom Brown Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/f129615fe72f70868a86862b663dd7d78dd5cb71 Modified Files -- src/backend/access/hash/README |4 ++-- src/backend/access/nbtree/README|2 +- src/backend/utils/adt/windowfuncs.c |2 +- src/interfaces/libpq/fe-exec.c |2 +- 4 files changed, 5 insertions(+), 5 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
[COMMITTERS] pgsql: Minor spelling fixes
Minor spelling fixes Fix a few spelling mistakes. Per bug report #8193 from Lajos Veres. Branch -- master Details --- http://git.postgresql.org/pg/commitdiff/c9fc28a7f12e27d530e2657c9dc6080fbfbe8a14 Modified Files -- contrib/pg_upgrade/relfilenode.c|2 +- src/backend/access/transam/xlogreader.c |2 +- src/backend/utils/adt/tsrank.c |8 src/tools/pgindent/typedefs.list|2 +- 4 files changed, 7 insertions(+), 7 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
