Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 00:46, Alex Hunsaker bada...@gmail.com wrote: On Tue, Feb 2, 2010 at 22:50, Tom Lane t...@sss.pgh.pa.us wrote: Alex Hunsaker bada...@gmail.com writes: On Tue, Feb 2, 2010 at 21:38, Tom Lane t...@sss.pgh.pa.us wrote: Alex Hunsaker bada...@gmail.com writes: Yeah the both is gross. How about: plperl.on_plperl_init plperl.on_plperlu_init plperl.on_init ? Well its already in. Well *that's* easily fixed. I think it's a bad idea, because it's unclear what you should put there and what the security implications are. I can't speak for its virtue, maybe Tim, Andrew? Ahh I think i figured it out. plperl.on_trusted_init runs *inside* of the safe. So you cant do unsafe things like use this or that module. plperl.on_init runs on init *outside* of the safe so you can use modules and what not. So now I can use say Digest::SHA without tossing the baby out with the bath water (just using plperlu). Gaping security whole? Maybe, no more so than installing an insecure C/plperlu function as you have to edit postgresql.conf to change it. Right? Maybe we should have: plperl.on_plperl_safe_init (runs inside of the safe, PGC_USERSET) plperl.on_plperl_init (runs outside safe, PGC_SUSET) plperl.on_plpleru_init (PGC_SUSET) All of the above have no SPI/database access. I think we can gt away with PGC_USERSET on safe_init as it wont allow you to do anything scary like play with security definer functions or redefine functions etc... There does seem to be the risk that I may not have plperl GRANTed but I can make any plperl function elog(ERROR) as long as they have not loaded plperl via a plperl_safe_init. We can probably fix that if people think its a valid dos/attack vector. Comments? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Streaming replication and message type header
On Tue, Jan 19, 2010 at 12:20 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
Tom Lane wrote:
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
Simon Riggs wrote:
Do we need a new record type for that, is there a handy record type to
bounce from?
After starting streaming, slices of WAL are sent as CopyData messages.
The CopyData payload begins with an XLogRecPtr, followed by the WAL
data. That payload format needs to be extended with a 'message type'
field and a new message type for the timestamps need to be added.
Whether or not anyone bothers with the timestamp message, I think adding
a message type header is a Must Fix item. A protocol with no provision
for extension is certainly going to bite us in the rear before long.
Agreed a message type header is a good idea, although we don't expect
streaming replication and the protocol to work across different major
versions anyway.
The attached patch adds a message type header into the payload in
CopyData message sent from walsender to walreceiver, to make the
replication protocol more extensible.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/doc/src/sgml/protocol.sgml
--- b/doc/src/sgml/protocol.sgml
***
*** 4179,4190 The commands accepted in walsender mode are:
already been recycled. On success, server responds with a
CopyOutResponse message, and backend starts to stream WAL as CopyData
messages.
/para
para
! The payload in each CopyData message consists of an XLogRecPtr,
! indicating the starting point of the WAL in the message, immediately
! followed by the WAL data itself.
/para
para
A single WAL record is never split across two CopyData messages. When
--- 4179,4243
already been recycled. On success, server responds with a
CopyOutResponse message, and backend starts to stream WAL as CopyData
messages.
+ The payload in CopyData message consists of the following format.
/para
para
! variablelist
! varlistentry
! term
! XLogData (B)
! /term
! listitem
! para
! variablelist
! varlistentry
! term
! Byte1('w')
! /term
! listitem
! para
! Identifies the message as WAL data.
! /para
! /listitem
! /varlistentry
! varlistentry
! term
! Int32
! /term
! listitem
! para
! The log file number of the LSN, indicating the starting point of
! the WAL in the message.
! /para
! /listitem
! /varlistentry
! varlistentry
! term
! Int32
! /term
! listitem
! para
! The byte offset of the LSN, indicating the starting point of
! the WAL in the message.
! /para
! /listitem
! /varlistentry
! varlistentry
! term
! Bytereplaceablen/replaceable
! /term
! listitem
! para
! Data that forms part of WAL data stream.
! /para
! /listitem
! /varlistentry
! /variablelist
! /para
! /listitem
! /varlistentry
! /variablelist
/para
para
A single WAL record is never split across two CopyData messages. When
*** a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
--- b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
***
*** 48,55 static char *recvBuf = NULL;
/* Prototypes for interface functions */
static bool libpqrcv_connect(char *conninfo, XLogRecPtr startpoint);
! static bool libpqrcv_receive(int timeout, XLogRecPtr *recptr, char **buffer,
! int *len);
static void libpqrcv_disconnect(void);
/* Prototypes for private functions */
--- 48,55
/* Prototypes for interface functions */
static bool libpqrcv_connect(char *conninfo, XLogRecPtr startpoint);
! static bool libpqrcv_receive(int timeout, unsigned char *type,
! char **buffer, int *len);
static void libpqrcv_disconnect(void);
/* Prototypes for private functions */
***
*** 236,248 libpqrcv_disconnect(void)
}
/*
! * Receive any WAL records available from XLOG stream, blocking for
* maximum of 'timeout' ms.
*
* Returns:
*
! * True if data was received. *recptr, *buffer and *len are set to
! * the WAL location of the received data, buffer holding it, and length,
* respectively.
*
* False if no data was available within timeout, or wait was interrupted
--- 236,248
}
/*
! * Receive any messages available from XLOG stream, blocking for
* maximum of 'timeout' ms.
*
* Returns:
*
! * True if data was received. *type, *buffer and *len are set to
! * the type of the received data, buffer holding it, and length,
* respectively.
*
*
Re: [HACKERS] Review of Writeable CTE Patch
Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Here's an updated patch. This includes the fix mentioned earlier, some comment improvements and making CopySnapshot() static again. I also changed all references to this feature to DML WITH for consistency. I'm not sure if we want to keep it, but it should now be easier to change in the future. Hi, I'm reviewing the writable CTE patch. The code logic seems to be pretty good, but I have a couple of comments about error cases: * Did we have a consensus about user-visible DML WITH messages? The term is used in error messages in many places, for example: DML WITH without RETURNING is only allowed inside an unreferenced CTE Since we don't use DML WITH nor CTE in documentation, I'd like to avoid such technical acronyms in logs if we had better names, or we should have a section to explain them in docs. * What can I do to get Recursive DML WITH statements are not supported message? I get syntax errors before I get the message because We don't support UNIONs with RETURNING queries. Am I missing something? =# UPDATE tbl SET i = i + 1 WHERE i = 1 UNION ALL UPDATE tbl SET i = i + 1 WHERE i = 2; ERROR: syntax error at or near UNION * The patch includes regression tests, but no error cases in it. More test cases are needed for stupid queries. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
Hi, On 2010-02-03 11:04 UTC+2, Takahiro Itagaki wrote: Hi, I'm reviewing the writable CTE patch. The code logic seems to be pretty good, but I have a couple of comments about error cases: * Did we have a consensus about user-visible DML WITH messages? The term is used in error messages in many places, for example: DML WITH without RETURNING is only allowed inside an unreferenced CTE Since we don't use DML WITH nor CTE in documentation, I'd like to avoid such technical acronyms in logs if we had better names, or we should have a section to explain them in docs. We have yet to reach a consensus on the name for this feature. I don't think we have any really good candidates, but I like DML WITH best so far. * What can I do to get Recursive DML WITH statements are not supported message? I get syntax errors before I get the message because We don't support UNIONs with RETURNING queries. Am I missing something? =# UPDATE tbl SET i = i + 1 WHERE i = 1 UNION ALL UPDATE tbl SET i = i + 1 WHERE i = 2; ERROR: syntax error at or near UNION WITH RECURSIVE t AS (INSERT INTO foo SELECT * FROM t) VALUES(true); would do that. You can do the same with UPDATE .. FROM and DELETE .. USING. * The patch includes regression tests, but no error cases in it. More test cases are needed for stupid queries. Ok, I'll add these and send an updated patch in a few hours. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Listen / Notify - what to do when the queue is full
On Wed, Feb 3, 2010 at 2:05 AM, Jeff Davis pg...@j-davis.com wrote: Thanks, very well spotted... Actually the same is true for LISTEN... I have reworked the patch to do the changes to listenChannels only in the post-commit functions. I'm worried that this creates the opposite problem: that a LISTEN transaction might commit before a NOTIFY transaction, and yet miss the notification. See the following comment and let me know if you agree... ! /* ! * Exec_ListenBeforeCommit --- subroutine for AtCommit_NotifyBeforeCommit ! * ! * Note that we do only set our pointer here and do not yet add the channel to ! * listenChannels. Since our transaction could still roll back we do this only ! * after commit. We know that our tail pointer won't move between here and ! * directly after commit, so we won't miss a notification. ! */ However this introduces a new problem when an initial LISTEN aborts: Then we are not listening to anything but for other backends it looks like we were. This is tracked by the boolean variable backendExecutesInitialListen and gets cleaned up in AtAbort_Notify(). It seems safest to me to add a backend (LISTEN) to the list before commit, and remove a backend (UNLISTEN) after commit. That way we are sure to only receive spurious notifications, and can't miss any. If a LISTEN aborted we would not only receive a few spurious notifications from it but would receive notifications on this channel forever even though we have never executed LISTEN on it successfully. Joachim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PITR - Bug or feature?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Fujii Masao wrote: On Mon, Feb 1, 2010 at 7:33 PM, Rafael Martinez r.m.guerr...@usit.uio.no wrote: Any thoughts about this? Is this a bug or a 'feature'? This is not a bug. Since pg_start_backup() uses %X/%X (not %08X/%08X) as the format of WAL location, the length of the second number of the WAL location could be less than 8. Instead of calculating the name of the backup history file for yourself, how about using pg_xlogfile_name() or pg_xlogfile_name_offset()? Thanks for the answer. We have updated our code and started using pg_xlogfile_name() in our PITR script. Everything works perfect now. When we started using PITR with version 8.1, we didn't have these functions and that was the reason we were using the value returned by pg_start_backup() to find out the last WAL to keep after PITR was finnish. regards, - -- Rafael Martinez, r.m.guerr...@usit.uio.no Center for Information Technology Services University of Oslo, Norway PGP Public Key: http://folk.uio.no/rafael/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.7 (GNU/Linux) iD8DBQFLaUXLBhuKQurGihQRAqNpAKCLCc6MDhGONJi5fTgStFoC+PP6hgCdHqVC yDfsC1erRWxFJRCF305Bbg8= =Brbz -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication and message type header
Fujii Masao wrote: On Tue, Jan 19, 2010 at 12:20 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Tom Lane wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Simon Riggs wrote: Do we need a new record type for that, is there a handy record type to bounce from? After starting streaming, slices of WAL are sent as CopyData messages. The CopyData payload begins with an XLogRecPtr, followed by the WAL data. That payload format needs to be extended with a 'message type' field and a new message type for the timestamps need to be added. Whether or not anyone bothers with the timestamp message, I think adding a message type header is a Must Fix item. A protocol with no provision for extension is certainly going to bite us in the rear before long. Agreed a message type header is a good idea, although we don't expect streaming replication and the protocol to work across different major versions anyway. The attached patch adds a message type header into the payload in CopyData message sent from walsender to walreceiver, to make the replication protocol more extensible. Ok, commmitted. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby: Relation-specific deferred conflict resolution
On Tue, 2010-02-02 at 20:27 +0200, Heikki Linnakangas wrote: I'd appreciate it if you could review the relation-specific conflict patch, 'cos it's still important. One fundamental gripe I have about that approach is that it's hard to predict when you will be saved by the cache and when your query will be canceled. For example, the patch stores only one latestRemovedXid value per lock partition. So if you have two tables that hash to different lock partitions, and are never both accessed in a single transaction, the cache will save your query every time. So far so good, but then you do a dump/restore, and the tables happen to be assigned to the same lock partition. Oops, a system that used to work fine starts to get snapshot too old errors. It's often better to be consistent and predictable, even if it means cancelling more queries. I think wë́'d need to have a much more fine-grained system before it's worthwhile to do deferred resolution. There's just too much false sharing otherwise. ISTM that this is exactly backwards. There is already way too many false positives and this patch would reduce them very significantly. Plus the cancelation is hardly predictable since it relies on whether or not a btree delete takes place during execution and the arrival time and rate of those is sporadic. There is no safe, predicatable behaviour in the current code. The gripe about the cache cannot be a fundamental one, since we can easily change the size and mechanism by which the cache operates without changing the patch very much at all. I am being told this area is a must-fix issue for this release. Tom's reaction to this issue (on other thread) illustrates that beautifully: On Sun, 2010-01-31 at 15:41 -0500, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: (snip) 2. no matter if they haven't accessed the index being cleaned (they might later, is the thinking...) That seems seriously horrid. What is the rationale for #2 in particular? I would hope that at worst this would affect sessions that are actively competing for the index being cleaned. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: NaN/Inf fix for ECPG Re: [HACKERS] out-of-scope cursor errors
Michael Meskes írta:
On Tue, Feb 02, 2010 at 03:34:24PM +0100, Boszormenyi Zoltan wrote:
Here's the new patch with the updated regression test.
Committed. Thanks a lot.
Michael
Tom Lane committed a fix for Windows, there was a missing
#include float.h
in execute.c, but there is another problem on Windows, which
I can't fix since I don't have a Windows build system.
The linker also complains about missing _isnan(). Can someone help?
The equivalent of -lm is needed for the cl linker command.
Also, another oversight needs fixing on my part, for which
the patch is atttached. The INSERT statement in nan_test.pgc
contains platform dependent strings, inf instead of infinity.
Best regards,
Zoltán Böszörményi
--
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics
--
Zoltán Böszörményi
Cybertec Schönig Schönig GmbH
http://www.postgresql.at/
diff -dcrpN pgsql.orig/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c pgsql/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c
*** pgsql.orig/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c 2010-02-02 17:09:12.0 +0100
--- pgsql/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c 2010-02-03 10:43:24.0 +0100
*** if (sqlca.sqlcode 0) sqlprint ( );}
*** 66,72
if (sqlca.sqlcode 0) sqlprint ( );}
#line 24 nan_test.pgc
! { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, insert into nantest1 ( id , d ) values ( 1 , 'nan' :: float8 ) , ( 2 , 'inf' :: float8 ) , ( 3 , '-inf' :: float8 ), ECPGt_EOIT, ECPGt_EORT);
#line 25 nan_test.pgc
if (sqlca.sqlcode 0) sqlprint ( );}
--- 66,72
if (sqlca.sqlcode 0) sqlprint ( );}
#line 24 nan_test.pgc
! { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, insert into nantest1 ( id , d ) values ( 1 , 'nan' :: float8 ) , ( 2 , 'infinity' :: float8 ) , ( 3 , '-infinity' :: float8 ), ECPGt_EOIT, ECPGt_EORT);
#line 25 nan_test.pgc
if (sqlca.sqlcode 0) sqlprint ( );}
diff -dcrpN pgsql.orig/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.stderr pgsql/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.stderr
*** pgsql.orig/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.stderr 2010-02-02 17:09:12.0 +0100
--- pgsql/src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.stderr 2010-02-03 10:43:24.0 +0100
***
*** 8,14
[NO_PID]: sqlca: code: 0, state: 0
[NO_PID]: ecpg_execute on line 24: OK: CREATE TABLE
[NO_PID]: sqlca: code: 0, state: 0
! [NO_PID]: ecpg_execute on line 25: query: insert into nantest1 ( id , d ) values ( 1 , 'nan' :: float8 ) , ( 2 , 'inf' :: float8 ) , ( 3 , '-inf' :: float8 ); with 0 parameter(s) on connection regress1
[NO_PID]: sqlca: code: 0, state: 0
[NO_PID]: ecpg_execute on line 25: using PQexec
[NO_PID]: sqlca: code: 0, state: 0
--- 8,14
[NO_PID]: sqlca: code: 0, state: 0
[NO_PID]: ecpg_execute on line 24: OK: CREATE TABLE
[NO_PID]: sqlca: code: 0, state: 0
! [NO_PID]: ecpg_execute on line 25: query: insert into nantest1 ( id , d ) values ( 1 , 'nan' :: float8 ) , ( 2 , 'infinity' :: float8 ) , ( 3 , '-infinity' :: float8 ); with 0 parameter(s) on connection regress1
[NO_PID]: sqlca: code: 0, state: 0
[NO_PID]: ecpg_execute on line 25: using PQexec
[NO_PID]: sqlca: code: 0, state: 0
diff -dcrpN pgsql.orig/src/interfaces/ecpg/test/pgtypeslib/nan_test.pgc pgsql/src/interfaces/ecpg/test/pgtypeslib/nan_test.pgc
*** pgsql.orig/src/interfaces/ecpg/test/pgtypeslib/nan_test.pgc 2010-02-02 17:09:12.0 +0100
--- pgsql/src/interfaces/ecpg/test/pgtypeslib/nan_test.pgc 2010-02-03 10:43:24.0 +0100
*** main(void)
*** 22,28
exec sql connect to REGRESSDB1;
exec sql create table nantest1 (id int4, d float8);
! exec sql insert into nantest1 (id, d) values (1, 'nan'::float8), (2, 'inf'::float8), (3, '-inf'::float8);
exec sql declare cur cursor for select id, d, d from nantest1;
exec sql open cur;
--- 22,28
exec sql connect to REGRESSDB1;
exec sql create table nantest1 (id int4, d float8);
! exec sql insert into nantest1 (id, d) values (1, 'nan'::float8), (2, 'infinity'::float8), (3, '-infinity'::float8);
exec sql declare cur cursor for select id, d, d from nantest1;
exec sql open cur;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Write a WAL record whenever we perform an operation without
Fujii Masao wrote: On Mon, Feb 1, 2010 at 7:40 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: So you get those messages when the table is *not* a temporary table. I can see now what Fujii was trying to say. His patch seems Ok, though perhaps it would be better to move the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). When I put it in heap_sync(), I didn't take into account that it's sometimes called just to flush buffers from buffer cache, not to fsync() non-WAL-logged operations. As you said, I moved the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). Here is the patch. Committed. The use_wal parameter to end_heap_rewrite() was not necessary, that information is already in RewriteState, so I took that out. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Write a WAL record whenever we perform an operation without
Fujii Masao wrote: On Mon, Feb 1, 2010 at 7:40 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: So you get those messages when the table is *not* a temporary table. I can see now what Fujii was trying to say. His patch seems Ok, though perhaps it would be better to move the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). When I put it in heap_sync(), I didn't take into account that it's sometimes called just to flush buffers from buffer cache, not to fsync() non-WAL-logged operations. As you said, I moved the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). Here is the patch. Committed. The use_wal parameter to end_heap_rewrite() was not necessary, that information is already in RewriteState, so I took that out. Are we going to bump up frontend/backend protocol version 3.0 to 3.x or some such? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Write a WAL record whenever we perform an operation without
Tatsuo Ishii wrote: Fujii Masao wrote: On Mon, Feb 1, 2010 at 7:40 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: So you get those messages when the table is *not* a temporary table. I can see now what Fujii was trying to say. His patch seems Ok, though perhaps it would be better to move the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). When I put it in heap_sync(), I didn't take into account that it's sometimes called just to flush buffers from buffer cache, not to fsync() non-WAL-logged operations. As you said, I moved the responsibility of calling XLogReportUnloggedStatement() to the callers of heap_sync(). Here is the patch. Committed. The use_wal parameter to end_heap_rewrite() was not necessary, that information is already in RewriteState, so I took that out. Are we going to bump up frontend/backend protocol version 3.0 to 3.x or some such? No, this doesn't affect the normal FE/BE protocol. The message header was added to the streaming replication messages that are sent within CopyData messages. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
On Tue, Feb 2, 2010 at 7:45 PM, Robert Haas robertmh...@gmail.com wrote: I think you're probably right, but it's not clear what the new name should be until we have a comment explaining what the function is responsible for. So I wrote some comments but wasn't going to repost the patch with the unchanged name without explanation... But I think you're right though I was looking at it the other way around. I want to have an API for a two-stage sync and of course if I do that I'll comment it to explain that clearly. The gist of the comments was that the function is preparing to fsync to initiate the i/o early and allow the later fsync to fast -- but also at the same time have the beneficial side-effect of avoiding cache poisoning. It's not clear that the two are necessarily linked though. Perhaps we need two separate apis, though it'll be hard to keep them separate on all platforms. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] rbtree test data
On Tue, 2 Feb 2010, Robert Haas wrote: On Sat, Jan 30, 2010 at 1:12 AM, Oleg Bartunov o...@sai.msu.su wrote: I made available test data I used on http://www.sai.msu.su/~megera/wiki/2009-07-27, so anyone can reproduce my results. You can download data http://www.sai.msu.su/~megera/postgres/files/links2.sql.gz, it's big (580Mb) Ugh. My system has been sitting here for four hours trying to create the first GIN index. I think I'm going to have to give up. My settings (only relevant) on my desktop ( 8Gb RAM, Intel(R) Core(TM)2 Duo CPU E6750 @ 2.66GHz : shared_buffers = 512MB #32MB# min 128kB work_mem = 32MB #1MB# min 64kB maintenance_work_mem = 256MB #16MB # min 1MB effective_cache_size = 1GB #128MB Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
On 02/03/10 12:53, Greg Stark wrote: On Tue, Feb 2, 2010 at 7:45 PM, Robert Haasrobertmh...@gmail.com wrote: I think you're probably right, but it's not clear what the new name should be until we have a comment explaining what the function is responsible for. So I wrote some comments but wasn't going to repost the patch with the unchanged name without explanation... But I think you're right though I was looking at it the other way around. I want to have an API for a two-stage sync and of course if I do that I'll comment it to explain that clearly. The gist of the comments was that the function is preparing to fsync to initiate the i/o early and allow the later fsync to fast -- but also at the same time have the beneficial side-effect of avoiding cache poisoning. It's not clear that the two are necessarily linked though. Perhaps we need two separate apis, though it'll be hard to keep them separate on all platforms. I vote for two seperate apis - sure, there will be some unfortunate overlap for most unixoid platforms but its sure better possibly to allow adding more platforms later at a centralized place than having to analyze every place where the api is used. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [CFReview] Red-Black Tree
Can you rename RED and BLACK to RBRED and RBBLACK? Yes, of course, done. Any objections to commit? -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ rbtree-0.9.gz Description: Unix tar archive -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] use of dblink_build_sql_insert() induces a server crash
Hi All,
Testcase:
create table foo (a int );
postgres=# SELECT dblink_build_sql_insert('foo','1 2',2,'{\0\,
\a\}','{\99\, \xyz\}');
HINT: Use the escape string syntax for escapes, e.g., E'\r\n'.
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
Version: Latest
Description: The dblink_build_sql_insert()/get_tuple_of_interest functions
is not taking care number of attributes in the target.
PFA patch to fix the same.
Thanks,
Rushabh Lathia
(www.EnterpriseDB.com)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 276c7e1..a067309 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2083,6 +2083,11 @@ get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **
/* internal error */
elog(ERROR, SPI connect failure - returned %d, ret);
+ if (pknumatts tupdesc-natts)
+ ereport(ERROR,
+(errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(statement has more expression then specified relation)));
+
/*
* Build sql statement to look up tuple of interest Use src_pkattvals as
* the criteria.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Greetings, hackers! The flurry of patches that vendors have recently been making to OpenSSL to address the potential man-in-the-middle attack during SSL renegotiation have disabled SSL renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL renegotiation, such as PostgreSQL, start failing. I’ve noticed such failures on Mac OS X 10.6.2 after installing Security Update 2010-001 (which is when Apple distributed their OpenSSL patch): http://support.apple.com/kb/HT4004 OpenSSL CVE-ID: CVE-2009-3555 Available for: Mac OS X v10.5.8, Mac OS X Server v10.5.8, Mac OS X v10.6.2, Mac OS X Server v10.6.2 Impact: An attacker with a privileged network position may capture data or change the operations performed in sessions protected by SSL Description: A man-in-the-middle vulnerability exists in the SSL and TLS protocols. Further information is available at http://www.phonefactor.com/sslgap A change to the renegotiation protocol is underway within the IETF. This update disables renegotiation in OpenSSL as a preventive security measure. After installing Security Update 2010-001, any libpq connection to the server that exchanges more than 512MB of data (the RENEGOTIATION_LIMIT defined in src/backend/libpq/be-secure.c) will trigger an SSL renegotiation, which fails, which disconnects the client. I observed the problem on both PostgreSQL 8.1.19 and PostgreSQL 8.4.2 (those are the only versions I have in production). I have been working around the problem by disabling SSL renegotiation entirely in my PostgreSQL servers, commenting out lines 316-339 in src/backend/libpq/be-secure.c. There have been reports of such SSL-related breakage on other platforms, too: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=560205 Thanks! Happy hacking! - Chris -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Alex Hunsaker wrote: Well its already in. Well *that's* easily fixed. I think it's a bad idea, because it's unclear what you should put there and what the security implications are. I can't speak for its virtue, maybe Tim, Andrew? plperl.on_perl_init runs when the library is loaded. That makes it useful for preloading perl modules and similar tasks. The other two in the patch under disccussion run when the relevant interpreter is first used in the current session. That makes them appropriate for doing things like loading specific initial settings (e.g. in the interpreter's %_SHARED). I'm not going to be pleased if, having had a substantial debate on the patch that contained on_perl_init a week or so ago there are now attempts to rip it out. As I commented when I committed it: The final thing that persuaded me that no great damage would be done by on_perl_init was the realization that we already have the ability to do more or less the same thing anyway via standard Perl mechanisms, and I'd be very surprised if enterprising Perl users hadn't made use of it. Regarding the naming of the params, I'm not keen to have more than one custom_variable_class for plperl. Within that, maybe we can bikeshed the names a bit. I don't have terribly strong feelings. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
On Wed, Feb 3, 2010 at 6:53 AM, Greg Stark gsst...@mit.edu wrote: On Tue, Feb 2, 2010 at 7:45 PM, Robert Haas robertmh...@gmail.com wrote: I think you're probably right, but it's not clear what the new name should be until we have a comment explaining what the function is responsible for. So I wrote some comments but wasn't going to repost the patch with the unchanged name without explanation... But I think you're right though I was looking at it the other way around. I want to have an API for a two-stage sync and of course if I do that I'll comment it to explain that clearly. The gist of the comments was that the function is preparing to fsync to initiate the i/o early and allow the later fsync to fast -- but also at the same time have the beneficial side-effect of avoiding cache poisoning. It's not clear that the two are necessarily linked though. Perhaps we need two separate apis, though it'll be hard to keep them separate on all platforms. Well, maybe we should start with a discussion of what kernel calls you're aware of on different platforms and then we could try to put an API around it. I mean, right now all you've got is POSIX_FADV_DONTNEED, so given just that I feel like the API could simply be pg_dontneed() or something. It's hard to design a general framework based on one example. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [CFReview] Red-Black Tree
2010/2/3 Teodor Sigaev teo...@sigaev.ru: Can you rename RED and BLACK to RBRED and RBBLACK? Yes, of course, done. Any objections to commit? I would like to see point #2 of the following email addressed before commit. As things stand, it is not clear (at least to me) whether this is a win. http://archives.postgresql.org/pgsql-hackers/2010-01/msg02552.php ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] remove contrib/xml2
Robert Haas escribió:
On Mon, Feb 1, 2010 at 5:23 PM, Andrew Dunstan and...@dunslane.net wrote:
Robert Haas wrote:
(2) add a very, very large warning that this will crash if you do
almost anything with it.
I think that's an exaggeration. Certain people are known to be using it
quite successfully.
Hmm. Well, all I know is that the first thing I tried crashed the server.
CREATE TABLE xpath_test (id integer NOT NULL, t xml);
INSERT INTO xpath_test VALUES (1, 'docint1/int/doc');
SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int', 'true')
as t(id int4);
This trivial patch lingering on my system fixes this crasher (this is
for the 8.3 branch). It makes the problem in alloc set ExprContext
warning show up instead.
There are still lotsa other holes, but hey, this is a start ...
Index: contrib/xml2/xpath.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/contrib/xml2/xpath.c,v
retrieving revision 1.16.2.1
diff -c -p -r1.16.2.1 xpath.c
*** contrib/xml2/xpath.c26 Mar 2008 01:19:11 - 1.16.2.1
--- contrib/xml2/xpath.c27 Jan 2010 15:30:56 -
*** xpath_table(PG_FUNCTION_ARGS)
*** 793,798
--- 793,801
*/
pgxml_parser_init();
+ PG_TRY();
+ {
+
/* For each row i.e. document returned from SPI */
for (i = 0; i proc; i++)
{
*** xpath_table(PG_FUNCTION_ARGS)
*** 929,934
--- 932,944
if (xmldoc)
pfree(xmldoc);
}
+ }
+ PG_CATCH();
+ {
+ xmlCleanupParser();
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
xmlCleanupParser();
/* Needed to flag completeness in 7.3.1. 7.4 defines it as a no-op. */
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: NaN/Inf fix for ECPG Re: [HACKERS] out-of-scope cursor errors
On Wed, Feb 03, 2010 at 10:59:57AM +0100, Boszormenyi Zoltan wrote: Also, another oversight needs fixing on my part, for which the patch is atttached. The INSERT statement in nan_test.pgc contains platform dependent strings, inf instead of infinity. Committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber mes...@jabber.org VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On Wed, Feb 3, 2010 at 4:09 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Hi, On 2010-02-03 11:04 UTC+2, Takahiro Itagaki wrote: Hi, I'm reviewing the writable CTE patch. The code logic seems to be pretty good, but I have a couple of comments about error cases: * Did we have a consensus about user-visible DML WITH messages? The term is used in error messages in many places, for example: DML WITH without RETURNING is only allowed inside an unreferenced CTE Since we don't use DML WITH nor CTE in documentation, I'd like to avoid such technical acronyms in logs if we had better names, or we should have a section to explain them in docs. We have yet to reach a consensus on the name for this feature. I don't think we have any really good candidates, but I like DML WITH best so far. Why can't we complain about the actual SQL statement the user issued? Like, say: INSERT requires RETURNING when used within a referenced CTE ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)
On 02/03/10 14:42, Robert Haas wrote: On Wed, Feb 3, 2010 at 6:53 AM, Greg Starkgsst...@mit.edu wrote: On Tue, Feb 2, 2010 at 7:45 PM, Robert Haasrobertmh...@gmail.com wrote: I think you're probably right, but it's not clear what the new name should be until we have a comment explaining what the function is responsible for. So I wrote some comments but wasn't going to repost the patch with the unchanged name without explanation... But I think you're right though I was looking at it the other way around. I want to have an API for a two-stage sync and of course if I do that I'll comment it to explain that clearly. The gist of the comments was that the function is preparing to fsync to initiate the i/o early and allow the later fsync to fast -- but also at the same time have the beneficial side-effect of avoiding cache poisoning. It's not clear that the two are necessarily linked though. Perhaps we need two separate apis, though it'll be hard to keep them separate on all platforms. Well, maybe we should start with a discussion of what kernel calls you're aware of on different platforms and then we could try to put an API around it. In linux there is sync_file_range. On newer Posixish systems one can emulate that with mmap() and msync() (in batches obviously). No idea about windows. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 06:41, Andrew Dunstan and...@dunslane.net wrote: Alex Hunsaker wrote: Well its already in. Well *that's* easily fixed. I think it's a bad idea, because it's unclear what you should put there and what the security implications are. I can't speak for its virtue, maybe Tim, Andrew? Regarding the naming of the params, I'm not keen to have more than one custom_variable_class for plperl. Within that, maybe we can bikeshed the names a bit. I don't have terribly strong feelings. Hey! I don't think were quite to that nasty B word yet :) I would argue that treating plperl and plperlu as the same language just because it shares the same code is a mistake. But I hate the idea of two custom_variable_classes for plperl(u) as well. Which is why I quickly switched to plperl.on_plperl(u)_init. Any thoughts on those? Again maybe people think the original names are fine... *shrug*. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On Wed, Feb 3, 2010 at 5:31 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: On 2010-02-03 11:04, Takahiro Itagaki wrote: * The patch includes regression tests, but no error cases in it. More test cases are needed for stupid queries. Here's an updated patch. Some thoughts: The comments in standard_ExecutorStart() don't do a good job of explaining WHY the code does what it does - they just recapitulate what you can already see from reading the code. You say If there are DML WITH statements, we always need to use the CID and copy the snapshot. That's self-evident from the following code. What's not clear is why this is necessary, and the comment doesn't make any attempt to explain it. The second half of the if statement has the same problem. In ExecTopLevelStmtIsReadOnly, you might perhaps want to rephase the comment in a way that doesn't use the word Ehm. Like maybe: Even if this function returns true, the statement might still contain INSERT, UPDATE, or DELETE statements within a CTE; we only check the top-level statement. Also, there should be a newline immediately before the function name, per our usual style conventions. InitPlan makes some references to leader scan states, but there's no explanation of what exactly those are. The comment in analyzeCTE that says Many of these conditions are impossible given restrictions of the grammar, but check 'em anyway. makes less sense with this patch than it did formerly and may need to be rethought... and I'm not sure there's any reason to change this elog() an Assert. In both analyzeCTE() and checkWellFormedRecursion(), I don't like just removing the assertions there; we should try to assert something a bit more sensible, like maybe !IsA(cte-ctequery, Query). This patch removes a number of other assertions as well, but I don't know enough about those other spots to judge whether all of those cases are sensible. The only change to parse_relation.c is the addition of a #include; is this needed? The documentation changes for INSERT, UPDATE, and DELETE seem inadequate, because they add a reference to with_query with no corresponding explanation of what a with_query might be. The limitations of INSERT/UPDATE/DELETE-within-WITH should be documented somewhere: top level CTE only, and no DO ALSO or conditional DO INSTEAD rules. If we don't intend to remove this limitation in a future release, we should probably also document that. I believe there are some other caveats that we've discussed before, too, though I'm not sure if they're still true. Stuff like: - CTEs will be executed to completion in sequential order before the main statement begins execution - each CTE will see the results of CTEs already executed, and the main statement will see the results of all CTEs - but queries within each CTE still won't see their own updates (a reference to whatever section of the manual we talk about this in would probably be good) - possible pitfalls of CTEs not being pipelined ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 9:51 AM, Alex Hunsaker bada...@gmail.com wrote: Hey! I don't think were quite to that nasty B word yet :) I would argue that treating plperl and plperlu as the same language just because it shares the same code is a mistake. But I hate the idea of two custom_variable_classes for plperl(u) as well. Which is why I quickly switched to plperl.on_plperl(u)_init. Any thoughts on those? Again maybe people think the original names are fine... *shrug*. I like plperl.on_plperl(u)_init better than plperl.on_(un)trusted_init. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Partial Page Writes documentaiton mention
Our manual mentions that you can turn off partial page writes if your file system guarantees full page writes. We used to mention ReiserFS 4 as an example, but I have changed that example to mention ZFS, because ZFS is now more popular. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
On Wed, Feb 3, 2010 at 6:24 AM, Chris Campbell chris_campb...@mac.com wrote: The flurry of patches that vendors have recently been making to OpenSSL to address the potential man-in-the-middle attack during SSL renegotiation have disabled SSL renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL renegotiation, such as PostgreSQL, start failing. Should we think about adding a GUC to disable renegotiation until this blows over? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Robert Haas wrote: On Wed, Feb 3, 2010 at 6:24 AM, Chris Campbell chris_campb...@mac.com wrote: The flurry of patches that vendors have recently been making to OpenSSL to address the potential man-in-the-middle attack during SSL renegotiation have disabled SSL renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL renegotiation, such as PostgreSQL, start failing. Should we think about adding a GUC to disable renegotiation until this blows over? hmm I wonder if we should not go as far as removing the whole renegotiation code, from the field it seems that there are very very few daemons actually doing that kind forced renegotiation. Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Largeobject Access Controls (r2460)
2010/2/1 KaiGai Kohei kai...@ak.jp.nec.com: I again wonder whether we are on the right direction. I believe the proposed approach is to dump blob metadata if and only if you are also dumping blob contents, and to do all of this for data dumps but not schema dumps. That seems about right to me. Originally, the reason why we decide to use per blob toc entry was that BLOB ACLS entry needs a few exceptional treatments in the code. But, if we deal with BLOB ITEM entry as data contents, it will also need additional exceptional treatments. But the new ones are less objectionable, maybe. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Robert Haas robertmh...@gmail.com writes: Should we think about adding a GUC to disable renegotiation until this blows over? Bad idea: once set, it'll never get unset, thus leaving installations with a weakened security posture even after they've installed fixed versions of openssl. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
On Wed, Feb 3, 2010 at 10:21 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Should we think about adding a GUC to disable renegotiation until this blows over? Bad idea: once set, it'll never get unset, thus leaving installations with a weakened security posture even after they've installed fixed versions of openssl. That's a problem, but our current posture of holding our breath doesn't seem to be working either. If we insist on shipping code that doesn't work with currently-distributed versions of OpenSSL, people will do things like, say, shut SSL off. Or packagers of PostgreSQL will apply patches that disable it unconditionally, leaving us with no control. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Robert Haas robertmh...@gmail.com writes: On Wed, Feb 3, 2010 at 9:51 AM, Alex Hunsaker bada...@gmail.com wrote: Hey! I don't think were quite to that nasty B word yet :) I would argue that treating plperl and plperlu as the same language just because it shares the same code is a mistake. But I hate the idea of two custom_variable_classes for plperl(u) as well. Which is why I quickly switched to plperl.on_plperl(u)_init. Any thoughts on those? Again maybe people think the original names are fine... *shrug*. I like plperl.on_plperl(u)_init better than plperl.on_(un)trusted_init. I agree. But the question in my mind is the relationship between plperl and plperlu. I agree with the upthread comment that it would be better if the init strings for them were entirely separate. ISTM we have got three categories here: plperl init done outside Safe (must be SUSET) plperl init done inside Safe (can be USERSET) plperlu init (must be SUSET) and there is no good reason to conflate the first and third, nor to insist that one must be a subset of the other, which AFAICS is the effect of the current design. So we need a naming scheme that takes some account of this. Perhaps plperl.plperl_init plperl.plperl_safe_init plperl.plperlu_init ? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Assorted cleanups in preparation for using a map file to support
Simon Riggs si...@2ndquadrant.com writes: On Wed, 2010-02-03 at 01:14 +, Tom Lane wrote: 1. Get rid of inval.c's dependency on relfilenode, by not having it emit smgr invalidations as a result of relcache flushes. Instead, smgr sinval messages are sent directly from smgr.c when an actual relation delete or truncate is done. This makes considerably more structural sense and allows elimination of a large number of useless smgr inval messages that were formerly sent even in cases where nothing was changing at the physical-relation level. Note that this reintroduces the concept of nontransactional inval messages, but that's okay --- because the messages are sent by smgr.c, they will be sent in Hot Standby slaves, just from a lower logical level than before. Presumably this means that SHAREDINVALSMGR_ID messages are no longer part of the invalidation messages attached to a commit record? Correct. If so, there is some minor code cleanup and comment changes in ProcessCommittedInvalidationMessages(). Would you like me to do that, or should we wait? I saw that. I didn't touch it because it's not directly relevant to what I'm doing right now, but I would like to go back and see whether that routine can't be got rid of completely. It seems to me to be a very klugy substitute for having enough information. I'm inclined to think that we should emit an sinval message (or maybe better a separate WAL entry) for initfile removal, instead of trying to reverse-engineer whether one happened. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
On Wed, Feb 3, 2010 at 10:21 AM, Tom Lane t...@sss.pgh.pa.us wrote: Bad idea: once set, it'll never get unset, thus leaving installations with a weakened security posture even after they've installed fixed versions of openssl. regards, tom lane One might argue that the current method is already weakened as it is measured by the amount of data sent instead of of a length of time. A session could live a long time under the 512MB threshold depending on the queries that are being performed. Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] rbtree test data
On Wed, Feb 3, 2010 at 7:00 AM, Oleg Bartunov o...@sai.msu.su wrote: On Tue, 2 Feb 2010, Robert Haas wrote: On Sat, Jan 30, 2010 at 1:12 AM, Oleg Bartunov o...@sai.msu.su wrote: I made available test data I used on http://www.sai.msu.su/~megera/wiki/2009-07-27, so anyone can reproduce my results. You can download data http://www.sai.msu.su/~megera/postgres/files/links2.sql.gz, it's big (580Mb) Ugh. My system has been sitting here for four hours trying to create the first GIN index. I think I'm going to have to give up. My settings (only relevant) on my desktop ( 8Gb RAM, Intel(R) Core(TM)2 Duo CPU E6750 @ 2.66GHz : shared_buffers = 512MB #32MB # min 128kB work_mem = 32MB #1MB # min 64kB maintenance_work_mem = 256MB #16MB # min 1MB effective_cache_size = 1GB #128MB OK, that helped. Although it was still long. :-) ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
Robert Haas robertmh...@gmail.com writes: On Wed, Feb 3, 2010 at 4:09 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: We have yet to reach a consensus on the name for this feature. I don't think we have any really good candidates, but I like DML WITH best so far. Why can't we complain about the actual SQL statement the user issued? Like, say: INSERT requires RETURNING when used within a referenced CTE We could probably make that work for error messages, but what about documentation? It's going to be awkward to write something like INSERT/UPDATE/DELETE RETURNING every time we need to make a general statement about the behavior of all three. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
Hi, On 2010-02-03 16:09 UTC+2, Robert Haas wrote: Why can't we complain about the actual SQL statement the user issued? Like, say: INSERT requires RETURNING when used within a referenced CTE The SELECT equivalent of this query looks like this: = with recursive t as (select * from t) values(true); ERROR: recursive query t does not have the form non-recursive-term UNION [ALL] recursive-term but I didn't want to throw people off to think that they can use INSERT/UPDATE/RETURNING in a RECURSIVE CTE, just to get complaints about syntax error. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On Wed, Feb 3, 2010 at 10:58 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Feb 3, 2010 at 4:09 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: We have yet to reach a consensus on the name for this feature. I don't think we have any really good candidates, but I like DML WITH best so far. Why can't we complain about the actual SQL statement the user issued? Like, say: INSERT requires RETURNING when used within a referenced CTE We could probably make that work for error messages, but what about documentation? It's going to be awkward to write something like INSERT/UPDATE/DELETE RETURNING every time we need to make a general statement about the behavior of all three. The current patch includes a total of 5 lines of text documenting this new feature (plus one example), so the issue doesn't really arise. If, as I believe, more documentation is needed, then we may need to think about how to handle this, but it's hard to speculate without a bit more context. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Michael Ledford mledf...@gmail.com writes: One might argue that the current method is already weakened as it is measured by the amount of data sent instead of of a length of time. A session could live a long time under the 512MB threshold depending on the queries that are being performed. Renegotiation after X amount of data is the recommended method AFAIK, because it limits the volume of data available to cryptanalysis. What makes you think that elapsed time is relevant at all? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [CFReview] Red-Black Tree
On Wed, Feb 3, 2010 at 8:48 AM, Robert Haas robertmh...@gmail.com wrote: 2010/2/3 Teodor Sigaev teo...@sigaev.ru: Can you rename RED and BLACK to RBRED and RBBLACK? Yes, of course, done. Any objections to commit? I would like to see point #2 of the following email addressed before commit. As things stand, it is not clear (at least to me) whether this is a win. http://archives.postgresql.org/pgsql-hackers/2010-01/msg02552.php Specifically, on this web page: http://www.sai.msu.su/~megera/wiki/2009-04-03 There is a section that begins with this line of text: Repeat test with 100,000 identical records varying array length (len). That test shows rbtree being a third slower than HEAD. But there's not enough information on that web page to replicate that test, so it's hard to speculate on what may be going wrong. I don't think we should commit this until we understand that. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
Robert Haas robertmh...@gmail.com writes: On Wed, Feb 3, 2010 at 10:58 AM, Tom Lane t...@sss.pgh.pa.us wrote: We could probably make that work for error messages, but what about documentation? It's going to be awkward to write something like INSERT/UPDATE/DELETE RETURNING every time we need to make a general statement about the behavior of all three. The current patch includes a total of 5 lines of text documenting this new feature (plus one example), so the issue doesn't really arise. Well, that's certainly not going to be nearly sufficient. I think what you meant is Marko hasn't bothered with documentation. There is going to need to be discussion in the RULES chapter, in the page describing returned command tags, and probably six other places that aren't coming to me in the time it takes to type this sentence. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On 2010-02-03 16:53 UTC+2, Robert Haas wrote: Some thoughts: The comments in standard_ExecutorStart() don't do a good job of explaining WHY the code does what it does - they just recapitulate what you can already see from reading the code. You say If there are DML WITH statements, we always need to use the CID and copy the snapshot. That's self-evident from the following code. What's not clear is why this is necessary, and the comment doesn't make any attempt to explain it. The second half of the if statement has the same problem. Ok, I'll try to make this more clear. In ExecTopLevelStmtIsReadOnly, you might perhaps want to rephase the comment in a way that doesn't use the word Ehm. Like maybe: Even if this function returns true, the statement might still contain INSERT, UPDATE, or DELETE statements within a CTE; we only check the top-level statement. Also, there should be a newline immediately before the function name, per our usual style conventions. That comment tries to emphasize the fact that I can't think of any reasonable name for that particular function. If the name looks OK, I can update the comment. The comment in analyzeCTE that says Many of these conditions are impossible given restrictions of the grammar, but check 'em anyway. makes less sense with this patch than it did formerly and may need to be rethought... and I'm not sure there's any reason to change this elog() an Assert. Ok, I'll look at this. In both analyzeCTE() and checkWellFormedRecursion(), I don't like just removing the assertions there; we should try to assert something a bit more sensible, like maybe !IsA(cte-ctequery, Query). This patch removes a number of other assertions as well, but I don't know enough about those other spots to judge whether all of those cases are sensible. I'll look through these again. The only change to parse_relation.c is the addition of a #include; is this needed? No, I thought I had removed that long time ago. Will remove. The documentation changes for INSERT, UPDATE, and DELETE seem inadequate, because they add a reference to with_query with no corresponding explanation of what a with_query might be. Ok, I'll add this. The limitations of INSERT/UPDATE/DELETE-within-WITH should be documented somewhere: top level CTE only, and no DO ALSO or conditional DO INSTEAD rules. If we don't intend to remove this limitation in a future release, we should probably also document that. I believe there are some other caveats that we've discussed before, too, though I'm not sure if they're still true. Stuff like: - CTEs will be executed to completion in sequential order before the main statement begins execution - each CTE will see the results of CTEs already executed, and the main statement will see the results of all CTEs - but queries within each CTE still won't see their own updates (a reference to whatever section of the manual we talk about this in would probably be good) - possible pitfalls of CTEs not being pipelined Right. The documentation in its current state is definitely lacking. I've tried to focus all the time I have in making this patch technically good. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Assorted cleanups in preparation for using a map file to support
On Wed, 2010-02-03 at 10:48 -0500, Tom Lane wrote: If so, there is some minor code cleanup and comment changes in ProcessCommittedInvalidationMessages(). Would you like me to do that, or should we wait? I saw that. I didn't touch it because it's not directly relevant to what I'm doing right now, but I would like to go back and see whether that routine can't be got rid of completely. It seems to me to be a very klugy substitute for having enough information. I'm inclined to think that we should emit an sinval message (or maybe better a separate WAL entry) for initfile removal, instead of trying to reverse-engineer whether one happened. An additional sinval message type would work. There is a requirement for us to run RelationCacheInitFileInvalidate() both before and after the other messages. So we would need to append and prepend the new message type onto the array of messages if transInvalInfo-RelcacheInitFileInval is true. That way we would just do SendSharedInvalidMessages() in xact_redo_commit and remove ProcessCommittedInvalidationMessages(), adding other code to handle the inval message. Doesn't seem any easier though. Another WAL record would definitely be cumbersome. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. I think this was the main concern? * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) It's hard to convey the underlying issues in a sentence or two. I tried. I'm not sure it's worth it. (Though if anyone thinks it is I'd be happy to get some specific suggestions for the wording to use.) - The utf8fix code has been greatly simplified. Yeah to the point that it makes me wonder if the old code had some reason to spin up the FunctionCall stuff. Do you happen to know? Before my refactoring led me to add safe_eval(), FunctionCall was 'the natural way' to invoke code inside the Safe compartment. The tests dont seem to pass :( this is from a make installcheck-world + ERROR: unrecognized configuration parameter plperl.on_trusted_init If I throw a LOAD 'plperl'; at the top of those sql files it works... Ah. That's be because I've got custom_variable_classes = 'plperl' in my postgresql.conf. I'll add LOAD 'plperl'; to the top of those tests. The only quibble I have with the docs is: + If the code fails with an error it will abort the initialization and + propagate out to the calling query, causing the current transaction or + subtransaction to be aborted. Any changes within the perl won't be + undone. If the literalplperl/ language is used again the + initialization will be repeated. Instead of Any changes within the perl won't be undone. Maybe Changes to the perl interpreter will not be undone ? In an earlier patch I'd used the word interpreter quite often. When polishing up the doc changes in response to Tom's feedback I opted to avoid that word when describing on_*trusted_init. This looks like a case where I removed 'interpreter' but didn't fixup the rest of the sentence. I'd prefer to simplify the sentence further, so I've changed it to Any changes within perl won't be undone. On Wed, Feb 03, 2010 at 01:06:03AM -0700, Alex Hunsaker wrote: plperl.on_trusted_init runs *inside* of the safe. So you cant do unsafe things like use this or that module. Yes. It's effectively the same as having a DO '...' language plperl; that's guaranteed to run before any other use of plperl. plperl.on_init runs on init *outside* of the safe so you can use modules and what not. So now I can use say Digest::SHA without tossing the baby out with the bath water (just using plperlu). Gaping security whole? Maybe, no more so than installing an insecure C/plperlu function as you have to edit postgresql.conf to change it. Right? Right. I'll emphasise the point that only plperlu code has access to anything loaded by plperl.on_init (aka .on_perl_init). Currently plperl code doesn't. There seemed to be some confusion upthread about how the GUCs work together so I'll recap here (using the original GUC names): When plperl.so is loaded (possibly in the postmaster due to shared_preload_libraries) a perl interpreter is created using whatever version of perl the server was configured with. That perl is initialized as if it had been started using: perl -e $(cat plc_perlboot.pl) If on_perl_init is set then the the initialization is effectively: perl -e $(cat plc_perlboot.pl) -e $on_perl_init That perl interpreter is now in a virgin 'unspecialized' state. From that state the interpreter may become either a plperl or plperlu interpreter depending on whichever language is first used. When an interpreter transitions from the initial state to become specialized for plperlu for a particular user then plperl.on_untrusted_init is eval'd. When an interpreter transitions from the initial state to become specialized for plperl then plc_safe_ok.pl is executed to create the Safe compartment and then plperl.on_trusted_init is eval'd *inside* the compartment. So, if all GUCs were set then plperl code would run in a perl initialized with on_perl_init + on_trusted_init, and plperlu code would run in a perl initialized with on_perl_init + on_untrusted_init. To add some context, I envisage plperl.on_perl_init being a stable value that typically pre-loads a set of modules etc., while the on_*trusted_init GUCs will typically be used for short-term debug/testing. Which is why on_untrusted_init (SUSET) is still useful with on_perl_init (SUSET). Maybe we should have: plperl.on_plperl_safe_init (runs inside of the safe, PGC_USERSET) plperl.on_plperl_init (runs outside safe, PGC_SUSET) plperl.on_plpleru_init (PGC_SUSET) Which, except for the
Re: [HACKERS] Review of Writeable CTE Patch
On Wed, Feb 3, 2010 at 11:18 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: On 2010-02-03 16:53 UTC+2, Robert Haas wrote: Some thoughts: The comments in standard_ExecutorStart() don't do a good job of explaining WHY the code does what it does - they just recapitulate what you can already see from reading the code. You say If there are DML WITH statements, we always need to use the CID and copy the snapshot. That's self-evident from the following code. What's not clear is why this is necessary, and the comment doesn't make any attempt to explain it. The second half of the if statement has the same problem. Right. The documentation in its current state is definitely lacking. I've tried to focus all the time I have in making this patch technically good. Outside of documentation issues, where do we stand? Do you need help with the documentation? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [CFReview] Red-Black Tree
On Wed, 3 Feb 2010, Robert Haas wrote: On Wed, Feb 3, 2010 at 8:48 AM, Robert Haas robertmh...@gmail.com wrote: 2010/2/3 Teodor Sigaev teo...@sigaev.ru: Can you rename RED and BLACK to RBRED and RBBLACK? Yes, of course, done. Any objections to commit? I would like to see point #2 of the following email addressed before commit. As things stand, it is not clear (at least to me) whether this is a win. http://archives.postgresql.org/pgsql-hackers/2010-01/msg02552.php Specifically, on this web page: http://www.sai.msu.su/~megera/wiki/2009-04-03 There is a section that begins with this line of text: Repeat test with 100,000 identical records varying array length (len). That test shows rbtree being a third slower than HEAD. But there's not enough information on that web page to replicate that test, so it's hard to speculate on what may be going wrong. I don't think we should commit this until we understand that. Robert, Mark described the test he did http://archives.postgresql.org/pgsql-hackers/2010-01/msg02927.php Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CommitFest Status Summary - 2010-02-03
Here's an overview of where we stand with the remaining 14 patches, according to my best understanding of the situation. * Fix large object support in pg_dump - I haven't looked at this, but it seems like it's relatively close to being ready for commit. We need to get this one done as it is a release blocker (IMO). * Remove gcc dependency in definition of inline functions - We have been waiting for an updated patch since January 19th. If there is no update in the next few days, I will marked this as Returned with Feedback. * Faster CREATE DATABASE by delaying fsync - There seems to be a consensus that this is doing something sensible, but we're still arguing about how things should be named and documented. Doesn't seem like anything insurmountable, though. * More frame options in window functions - Tom has listed himself as the committer on this one, so I infer that he is planning to commit it after suitable editorialization. This is a big patch. * Writeable CTEs - Another big patch. As per today's discussion, at a minimum, it still needs some cleanup work and quite a bit of additional documentation. I have set it back to Waiting on Author. Even once that's fixed, I suspect this is going to need some attention from Tom prior to commit. * Provide rowcount for utility SELECTs - There's some debate about whether this is a good idea, but I believe most people are in favor of it, provided that we can cover all cases. This one really needs some more review. * Make PostgreSQL binaries use the new PQconnectdbParams() libpq functions - I believe Joe Conway is working on this. * rbtree - I have done a lot of work reviewing this, and Mark Cave-Ayland has done some work on it, too. But there are some unanswered performance questions that need to be addressed before commit. This is another one that could really use some more eyes on it. * knngist - The third remaining big patch. Mark Cave-Ayland volunteered to review this one, too, but so far no review has been posted on -hackers. I know that the PostGIS folks would really like to have this, but time is growing short. * plpython3 - It doesn't seem likely that this will go into core for 9.0, but Nathan Boley is going to review it anyhow, which is a good thing. * Add on_trusted_init and on_untrusted_init to plperl - Still arguing about the GUC names and details, but seems to be more or less on track. * Package namespace and Safe init cleanup for plperl - From the discussion, this one seems to be in pretty good shape, too. * Listen / Notify rewrite - The fourth and final major patch remaining for this CommitFest. There are still ongoing discussions about the concurrency handling in this patch, and some other open issues, too, like limiting the payload to ASCII. I know this is another big feature people would like to see in, but we're running out of time to get it stabilized. * xpath non-nodeset result enabling - We've been waiting on an updated patch since January 17th. On the 28th, Scott Bailey said he might take a look at it, but we've heard nothing since. If there is no update in the next couple of days, I will mark this Returned with Feedback. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [CFReview] Red-Black Tree
2010/2/3 Oleg Bartunov o...@sai.msu.su: I would like to see point #2 of the following email addressed before commit. As things stand, it is not clear (at least to me) whether this is a win. http://archives.postgresql.org/pgsql-hackers/2010-01/msg02552.php Specifically, on this web page: http://www.sai.msu.su/~megera/wiki/2009-04-03 There is a section that begins with this line of text: Repeat test with 100,000 identical records varying array length (len). That test shows rbtree being a third slower than HEAD. But there's not enough information on that web page to replicate that test, so it's hard to speculate on what may be going wrong. I don't think we should commit this until we understand that. Robert, Mark described the test he did http://archives.postgresql.org/pgsql-hackers/2010-01/msg02927.php So why did he get totally different answers than you? It's not enough to say somebody else did a test and got better numbers than we did, so let's use theirs. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On 2010-02-03 18:41 UTC+2, Merlin Moncure wrote: On Wed, Feb 3, 2010 at 11:18 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Right. The documentation in its current state is definitely lacking. I've tried to focus all the time I have in making this patch technically good. Do you need help with the documentation? I'm going to work on the documentation tonight, but it will probably need some work from a native English speaker after I'm done. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Is there a way to detect when the SSL library has renegotiation disabled? (Either at compile-time or runtime, although runtime would definitely be better because we’ll change our behavior if/when the user updates their SSL library.) If so, we could skip renegotiation when it’s disabled in the library, but otherwise perform renegotiation like we normally do (every 512 MB, I think it is). Also, the official OpenSSL patch provides a way for the application to re-enable renegotiation. I don’t think all implementations will do so, though (e.g., some vendors might have patched it differently). - Chris -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of Writeable CTE Patch
On Wed, Feb 3, 2010 at 11:18 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: In ExecTopLevelStmtIsReadOnly, you might perhaps want to rephase the comment in a way that doesn't use the word Ehm. Like maybe: Even if this function returns true, the statement might still contain INSERT, UPDATE, or DELETE statements within a CTE; we only check the top-level statement. Also, there should be a newline immediately before the function name, per our usual style conventions. That comment tries to emphasize the fact that I can't think of any reasonable name for that particular function. If the name looks OK, I can update the comment. Name seems fine. Just fix the comment. The limitations of INSERT/UPDATE/DELETE-within-WITH should be documented somewhere: top level CTE only, and no DO ALSO or conditional DO INSTEAD rules. If we don't intend to remove this limitation in a future release, we should probably also document that. I believe there are some other caveats that we've discussed before, too, though I'm not sure if they're still true. Stuff like: - CTEs will be executed to completion in sequential order before the main statement begins execution - each CTE will see the results of CTEs already executed, and the main statement will see the results of all CTEs - but queries within each CTE still won't see their own updates (a reference to whatever section of the manual we talk about this in would probably be good) - possible pitfalls of CTEs not being pipelined Right. The documentation in its current state is definitely lacking. I've tried to focus all the time I have in making this patch technically good. Well, technically good is certainly a good place to start. :-) Of course, we need the docs, too. Thanks for your work on this. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby and VACUUM FULL
I wrote: * We can not change the toast rel OID of a shared catalog -- there's no way to propagate that into the other copies of pg_class. So we need to rejigger the logic for heap rewriting a little bit. Toast rel swapping has to be handled by swapping their relfilenodes not their OIDs. This is no big deal as far as cluster.c itself is concerned, but the tricky part is that when we write new toasted values into the new toast rel, the TOAST pointers going into the new heap have to be written with the original toast-table OID value not the one that the transient target toast rel has got. This is doable but it would uglify the TOAST API a bit I think. I've been playing around with different alternatives for solving the problem of toast-pointer OIDs, but I keep coming back to the above as being the least invasive and most robust answer. There are two basic ways that we could do it: pass the OID to use to the toast logic, which would require adding a parameter to heap_insert and a number of other places; or add a field to struct Relation that says when inserting a TOAST pointer in this relation, use this OID as the toast-table OID value in the pointer, even if that's different from what the table's OID appears to be. The latter seems like less of a notational change, so I'm leaning to that, but wanted to see if anyone prefers the other. We could avoid this hackery if there were a way for Relation structs to point at either the old or the new physical relation (relfilenode); then we'd not need the transient new heap relation during CLUSTER/VF, which would be good for reducing catalog churn. I've concluded that that's too large a change to undertake for 9.0, but it might be interesting to try in the future. So I'd prefer that what we do for now touch as little code as possible so as to be easy to revert; hence I'm not wanting to change heap_insert's signature. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
On Wed, Feb 3, 2010 at 11:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Renegotiation after X amount of data is the recommended method AFAIK, because it limits the volume of data available to cryptanalysis. What makes you think that elapsed time is relevant at all? regards, tom lane You are correct. In that volume of data also matters. It depends on what kind of attack you are trying to minimize here. In my particular use case I fluctuate between idle and busy but mostly low bandwidth. You have four different primary cases that you are possible: 1) timed method on an idle link (or low usage) 2) timed method on a busy link (or high usage) 3) data limit on an idle link (or low usage) 4) data limit on a busy link (or high usage) The timed method is more optimal for case 1. The data limit is more optimal for case 4. Case 2 gives an attacker more data to do crypto-analysis. Case 3 gives an attacker more time to work with the current secret key on a live link. Depending on your use case, being able to work with a live link is worse than working with the data postmortem. There is I'm sure some hybrid in the middle between these where optimal lives. Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Chris Campbell chris_campb...@mac.com writes: Is there a way to detect when the SSL library has renegotiation disabled? Probably not. The current set of emergency security patches would certainly not have exposed any new API that would help us tell this :-( If said patches were done properly they'd have also turned an application-level renegotiation request into a no-op, instead of breaking apps by making it fail --- but apparently they were not done properly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
On Wed, Feb 3, 2010 at 11:52 AM, Michael Ledford mledf...@gmail.com wrote: On Wed, Feb 3, 2010 at 11:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Renegotiation after X amount of data is the recommended method AFAIK, because it limits the volume of data available to cryptanalysis. What makes you think that elapsed time is relevant at all? You are correct. In that volume of data also matters. It depends on what kind of attack you are trying to minimize here. In my particular use case I fluctuate between idle and busy but mostly low bandwidth. You have four different primary cases that you are possible: This may all be true, but I think we're getting off track. If we force ANY negotiation (whether based on time or bytes transferred), we will, apparently, break things. So I think that means we should have a way to disable that behavior, for fear of dissuading people from using SSL (or PostgreSQL) altogether, or hacking their own copies of the source in ways that may be even uglier. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby and VACUUM FULL
Tom Lane wrote: I've been playing around with different alternatives for solving the problem of toast-pointer OIDs, but I keep coming back to the above as being the least invasive and most robust answer. There are two basic ways that we could do it: pass the OID to use to the toast logic, which would require adding a parameter to heap_insert and a number of other places; or add a field to struct Relation that says when inserting a TOAST pointer in this relation, use this OID as the toast-table OID value in the pointer, even if that's different from what the table's OID appears to be. The latter seems like less of a notational change, so I'm leaning to that, but wanted to see if anyone prefers the other. We could avoid this hackery if there were a way for Relation structs to point at either the old or the new physical relation (relfilenode); then we'd not need the transient new heap relation during CLUSTER/VF, which would be good for reducing catalog churn. I've concluded that that's too large a change to undertake for 9.0, but it might be interesting to try in the future. So I'd prefer that what we do for now touch as little code as possible so as to be easy to revert; hence I'm not wanting to change heap_insert's signature. I don't think any of this affects pg_migrator, but if it does, please let me know. When I hear TOAST and OID used in the same sentence, my ears perk up. :-) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Tom Lane wrote: Chris Campbell chris_campb...@mac.com writes: Is there a way to detect when the SSL library has renegotiation disabled? Probably not. The current set of emergency security patches would certainly not have exposed any new API that would help us tell this :-( If said patches were done properly they'd have also turned an application-level renegotiation request into a no-op, instead of breaking apps by making it fail --- but apparently they were not done properly. Yea, and also keep in mind any SSL library checks need to be done at run-time (because I believe openssl is usually linked as a shared object), which even further limits our options. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby and VACUUM FULL
On Wed, 2010-02-03 at 11:50 -0500, Tom Lane wrote: I've concluded that that's too large a change to undertake for 9.0 The purpose of this was to make the big changes in 9.0. If we aren't going to do that it seems like we shouldn't bother at all. So why not flip back to the easier approach of make something work for HS only and then do everything you want to do in the next release? The burst radius of the half-way changes you are proposing seems high in comparison. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 09:31, Tim Bunce tim.bu...@pobox.com wrote: On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. I think this was the main concern? * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) It's hard to convey the underlying issues in a sentence or two. I tried. I'm not sure it's worth it. (Though if anyone thinks it is I'd be happy to get some specific suggestions for the wording to use.) All I know is I thought hrm... Why cant you have SPI ? It seems useful and I dont immediately see why its a bad idea. So I dug up the old threads. Im just afraid say in a year or two we will forget why we disallow it. I was thinking something along the lines of: diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 6f577f0..a19f1da 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -422,6 +422,12 @@ plperl_init_interp(void) PERL_SET_CONTEXT(plperl); perl_construct(plperl); + + /* + * Allow things like SPI to happen *after* the plperl.*init functions have run + * this avoids nasty problems with security definer functions + * ...maybe some mail link here or the whole quote from Tom? + */ perl_parse(plperl, plperl_init_shared_libs, nargs, embedding, NULL); The only quibble I have with the docs is: + If the code fails with an error it will abort the initialization and + propagate out to the calling query, causing the current transaction or + subtransaction to be aborted. Any changes within the perl won't be + undone. If the literalplperl/ language is used again the + initialization will be repeated. I'd prefer to simplify the sentence further, so I've changed it to Any changes within perl won't be undone. Much better. Maybe we should have: plperl.on_plperl_safe_init (runs inside of the safe, PGC_USERSET) plperl.on_plperl_init (runs outside safe, PGC_SUSET) plperl.on_plpleru_init (PGC_SUSET) Which, except for the names, is essentially what the patches implement. Well not quite as with the above there is still no global on_init. If we're going to bikeshed the names, I'd suggest: plperl.on_init - run on interpreter creation plperl.on_plperl_init - run when then specialized for plperl plperl.on_plperlu_init - run when then specialized for plperlu Hrm, I think I agree with Tom that we should not have a global on_init. And instead of two separate GUCs (we still end up with 3 gucs total). Im still thinking through it... Since Tom also expressed a preference for .on_plperl_init/.on_plperlu_init Well I think Magnus and Robert did as well :) and you also suggested .on_init earlier, I'll rework the patch with those names, plus the docs and test fixes nted above. OK There does seem to be the risk that I may not have plperl GRANTed but I can make any plperl function elog(ERROR) as long as they have not loaded plperl via a plperl_safe_init. We can probably fix that if people think its a valid dos/attack vector. Interesting thought. If this is a valid concern (as it seems to be) then I could add some logic to check if the language has been GRANTed. (I.e. ignore on_plperl_init if the user can't write plperl code, and ignore on_plperlu_init if the user can't write plperlu code.) Well Im not sure. As a user I can probably cause havok just by passing interesting values to a function. It does seem inconsistent that you cant create plperl functions but you can potentially modify SHARED. In-fact if you have a security definer plperl function it seems quite scary that they could change values in SHARED. But any plperl function can do that anyway. (do we have/need documentation that SHARED in a plperl security definer function is unsafe?) So I dont think its *that* big of deal as long as the GRANT check is in place. Thoughts? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 10:18, Alex Hunsaker bada...@gmail.com wrote: On Wed, Feb 3, 2010 at 09:31, Tim Bunce tim.bu...@pobox.com wrote: On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: If we're going to bikeshed the names, I'd suggest: plperl.on_init - run on interpreter creation plperl.on_plperl_init - run when then specialized for plperl plperl.on_plperlu_init - run when then specialized for plperlu Hrm, I think I agree with Tom that we should not have a global on_init. And instead of two separate GUCs (we still end up with 3 gucs total). Im still thinking through it... Err And instead have two separate GUCs (3 in total) not And Instead of two -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] use of dblink_build_sql_insert() induces a server crash
On 02/03/2010 04:49 AM, Rushabh Lathia wrote:
Testcase:
create table foo (a int );
postgres=# SELECT dblink_build_sql_insert('foo','1 2',2,'{\0\,
\a\}','{\99\, \xyz\}');
HINT: Use the escape string syntax for escapes, e.g., E'\r\n'.
server closed the connection unexpectedly
Thanks for the report -- will have a look later today.
Joe
signature.asc
Description: OpenPGP digital signature
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Feb 3, 2010, at 9:21 AM, Alex Hunsaker wrote: plperl.on_init - run on interpreter creation plperl.on_plperl_init - run when then specialized for plperl plperl.on_plperlu_init - run when then specialized for plperlu Hrm, I think I agree with Tom that we should not have a global on_init. And instead of two separate GUCs (we still end up with 3 gucs total). Im still thinking through it... I completely agree on using plperl and plperlu instead of trusted and untrusted in the GUC names. The latter are just too confusing (even Tom mixed them up in a post last week). They are among the worst names in the system, IMHO. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby and VACUUM FULL
Simon Riggs si...@2ndquadrant.com writes: On Wed, 2010-02-03 at 11:50 -0500, Tom Lane wrote: I've concluded that that's too large a change to undertake for 9.0 The purpose of this was to make the big changes in 9.0. If we aren't going to do that it seems like we shouldn't bother at all. No, the purpose of this was to get rid of VACUUM FULL INPLACE in 9.0. I'm not interested in destabilizing the code (even more) just to avoid one small internal kluge. The proposed magic field in struct Relation is the only part of this that I'd foresee reverting later. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 The main problem of enabling standard_conforming_strings is that applications and/or programming language DB APIs are not prepared to support this. I don't see a change in DB APIs (that I know of -- Python, Perl, and PHP) to add support for producing a string according to standard_conforming_strings parameter. Perl (DBD::Pg anyway) has been compatible since May 2008. As one of the more vocal critics of the 8.3 implicit casting incident, I say +1 on making the change. Unlike casting, it's a simple GUC change to fix, and now (9.0) is the time to do it. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201002031233 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAktps4oACgkQvJuQZxSWSshAIgCg6wxvgVasOksQ8JQaFOeaSQEu zZwAn0UqIG7Oti6BJVeJYTEx6b7VsZjf =HJcB -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Greg Sabino Mullane g...@turnstep.com writes: As one of the more vocal critics of the 8.3 implicit casting incident, I say +1 on making the change. Unlike casting, it's a simple GUC change to fix, and now (9.0) is the time to do it. Unfortunately, no: six months ago was the time to do it. The argument for doing this now hinges solely on a marketing-driven choice of version name, and not on any actual evidence that applications are ready for it. We really need to do this at the start of a devel and alpha test cycle, not at the end. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 03, 2010 at 10:18:51AM -0700, Alex Hunsaker wrote: On Wed, Feb 3, 2010 at 09:31, Tim Bunce tim.bu...@pobox.com wrote: On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. I think this was the main concern? * We call a plperl function for the first time in a session, causing plperl.so to be loaded. This happens in the context of a superuser calling a non-superuser security definer function, or perhaps vice versa. Whose permissions apply to whatever the on_load code tries to do? (Hint: every answer is wrong.) It's hard to convey the underlying issues in a sentence or two. I tried. I'm not sure it's worth it. (Though if anyone thinks it is I'd be happy to get some specific suggestions for the wording to use.) All I know is I thought hrm... Why cant you have SPI ? It seems useful and I dont immediately see why its a bad idea. So I dug up the old threads. Im just afraid say in a year or two we will forget why we disallow it. Ah, yes, a comment is certainly easier to write up. I'll add one and include a link to the relevant thread in the archives. Thanks for the example. There does seem to be the risk that I may not have plperl GRANTed but I can make any plperl function elog(ERROR) as long as they have not loaded plperl via a plperl_safe_init. We can probably fix that if people think its a valid dos/attack vector. Interesting thought. If this is a valid concern (as it seems to be) then I could add some logic to check if the language has been GRANTed. (I.e. ignore on_plperl_init if the user can't write plperl code, and ignore on_plperlu_init if the user can't write plperlu code.) Well Im not sure. As a user I can probably cause havok just by passing interesting values to a function. It does seem inconsistent that you cant create plperl functions but you can potentially modify SHARED. In-fact if you have a security definer plperl function it seems quite scary that they could change values in SHARED. But any plperl function can do that anyway. (do we have/need documentation that SHARED in a plperl security definer function is unsafe?) So I dont think its *that* big of deal as long as the GRANT check is in place. I don't see a significant issue in security definer and %_SHARED from what you've said above. Authors of security definer functions should naturally take care in how they use their argument values and %_SHARED. I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
* Tom Lane t...@sss.pgh.pa.us [100203 12:39]: Greg Sabino Mullane g...@turnstep.com writes: As one of the more vocal critics of the 8.3 implicit casting incident, I say +1 on making the change. Unlike casting, it's a simple GUC change to fix, and now (9.0) is the time to do it. Unfortunately, no: six months ago was the time to do it. The argument for doing this now hinges solely on a marketing-driven choice of version name, and not on any actual evidence that applications are ready for it. We really need to do this at the start of a devel and alpha test cycle, not at the end. I'm not really worried about users using/testing PG from CVS or alphas - they are users following PG closely enough that the switch is easy for them to handle. *I* think beta1 is the when this *needs* to be done by. Sure, it would have been nicer if it was earlier, but beta1 is when users start actually using/testing (by users here, I mean ones who aren't closely following PG development, and changes). After beta1 comes out, it's absolutely a no-go, but if changing standard_conforming_strings is something the community wants to go towards, then I say do it now, before we're locked into another release and another year of it. a. -- Aidan Van Dyk Create like a god, ai...@highrise.ca command like a king, http://www.highrise.ca/ work like a slave. signature.asc Description: Digital signature
Re: [HACKERS] use of dblink_build_sql_insert() induces a server crash
On 02/03/2010 04:49 AM, Rushabh Lathia wrote:
create table foo (a int );
postgres=# SELECT dblink_build_sql_insert('foo','1 2',2,'{\0\,
\a\}','{\99\, \xyz\}');
HINT: Use the escape string syntax for escapes, e.g., E'\r\n'.
server closed the connection unexpectedly
The problem exists with all three dblink_build_sql_* functions. Here is
a more complete patch. If there are no objections I'll apply this to
HEAD and look at back-patching -- these functions have hardly been
touched since inception.
Joe
Index: dblink.c
===
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.87
diff -c -r1.87 dblink.c
*** dblink.c 24 Jan 2010 22:19:38 - 1.87
--- dblink.c 3 Feb 2010 17:45:26 -
***
*** 1255,1260
--- 1255,1262
int32 pknumatts_tmp = PG_GETARG_INT32(2);
ArrayType *src_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(3);
ArrayType *tgt_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(4);
+ TupleDesc tupdesc;
+ Relation rel;
Oid relid;
int16 pknumatts = 0;
char **src_pkattvals;
***
*** 1290,1295
--- 1292,1308
attributes too large)));
/*
+ * Open relation using relid, ensure we don't ask for
+ * more pk attributes than we have columns
+ */
+ rel = relation_open(relid, AccessShareLock);
+ tupdesc = CreateTupleDescCopy(rel-rd_att);
+ relation_close(rel, AccessShareLock);
+ if (pknumatts tupdesc-natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(number of primary key fields exceeds number of specified relation attributes)));
+
+ /*
* Source array is made up of key values that will be used to locate the
* tuple of interest from the local system.
*/
***
*** 1354,1359
--- 1367,1374
int2vector *pkattnums = (int2vector *) PG_GETARG_POINTER(1);
int32 pknumatts_tmp = PG_GETARG_INT32(2);
ArrayType *tgt_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(3);
+ TupleDesc tupdesc;
+ Relation rel;
Oid relid;
int16 pknumatts = 0;
char **tgt_pkattvals;
***
*** 1387,1392
--- 1402,1418
attributes too large)));
/*
+ * Open relation using relid, ensure we don't ask for
+ * more pk attributes than we have columns
+ */
+ rel = relation_open(relid, AccessShareLock);
+ tupdesc = CreateTupleDescCopy(rel-rd_att);
+ relation_close(rel, AccessShareLock);
+ if (pknumatts tupdesc-natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(number of primary key fields exceeds number of specified relation attributes)));
+
+ /*
* Target array is made up of key values that will be used to build the
* SQL string for use on the remote system.
*/
***
*** 1441,1446
--- 1467,1474
int32 pknumatts_tmp = PG_GETARG_INT32(2);
ArrayType *src_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(3);
ArrayType *tgt_pkattvals_arry = PG_GETARG_ARRAYTYPE_P(4);
+ TupleDesc tupdesc;
+ Relation rel;
Oid relid;
int16 pknumatts = 0;
char **src_pkattvals;
***
*** 1476,1481
--- 1504,1520
attributes too large)));
/*
+ * Open relation using relid, ensure we don't ask for
+ * more pk attributes than we have columns
+ */
+ rel = relation_open(relid, AccessShareLock);
+ tupdesc = CreateTupleDescCopy(rel-rd_att);
+ relation_close(rel, AccessShareLock);
+ if (pknumatts tupdesc-natts)
+ ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(number of primary key fields exceeds number of specified relation attributes)));
+
+ /*
* Source array is made up of key values that will be used to locate the
* tuple of interest from the local system.
*/
Index: expected/dblink.out
===
RCS file: /opt/src/cvs/pgsql/contrib/dblink/expected/dblink.out,v
retrieving revision 1.27
diff -c -r1.27 dblink.out
*** expected/dblink.out 22 Nov 2009 05:20:38 - 1.27
--- expected/dblink.out 3 Feb 2010 18:01:25 -
***
*** 39,44
--- 39,47
INSERT INTO foo(f1,f2,f3) VALUES('99','xyz','{a0,b0,c0}')
(1 row)
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_insert('foo','1 2 3 4',4,'{0, a, {a0,b0,c0}}','{99, xyz, {za0,zb0,zc0}}');
+ ERROR: number of primary key fields exceeds number of specified relation attributes
-- build an update statement based on a local tuple,
-- replacing the primary key values with new ones
SELECT dblink_build_sql_update('foo','1 2',2,'{0, a}','{99, xyz}');
***
*** 47,52
--- 50,58
UPDATE foo SET f1 = '99', f2 = 'xyz', f3 = '{a0,b0,c0}' WHERE f1 = '99' AND f2 = 'xyz'
(1 row)
+ -- too many pk fields, should fail
+ SELECT dblink_build_sql_update('foo','1 2 3 4',4,'{0, a, {a0,b0,c0}}','{99, xyz, {za0,zb0,zc0}}');
+ ERROR: number of primary key fields exceeds number of specified relation attributes
-- build a delete statement
Re: [HACKERS] use of dblink_build_sql_insert() induces a server crash
Joe Conway m...@joeconway.com writes: The problem exists with all three dblink_build_sql_* functions. Here is a more complete patch. If there are no objections I'll apply this to HEAD and look at back-patching -- these functions have hardly been touched since inception. Do you really need to copy the relation tupdesc when you only are going to make a one-time check of the number of attributes? This coding would make some sense if you intended to use the tupdesc again later in the functions, but it appears you don't. Also, what about cases where the relation contains dropped columns --- it's not obvious whether this test is correct in that case. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On Wed, Feb 3, 2010 at 12:34 PM, Greg Sabino Mullane g...@turnstep.com wrote: Perl (DBD::Pg anyway) has been compatible since May 2008. I would interpret that to mean that there is a significant possibility that a too-old DBD::Pg could get used with a new PostgreSQL, and therefore we shouldn't change anything for 9.0. May 2008 is not that long ago, especially for people running systems like RHEL with five-year major release cycles. I am not sure I really understand why anyone is a rush to make this change. What harm is being done by the status quo? What benefit do we get out of changing the default? The major argument that has been offered so far is that if we don't change it now, we never will, but I don't believe that the tenor of this discussion supports the contention that Tom or anyone else never wants to make this change. It also seems to overlook the fact that we are STILL dealing with the fallout from this change in the core code; Tom gave examples upthread of changes that are being released for the first time *in 9.0* to address problems created by this transition. And that is just the core code; we have to expect that third-party code will lag behind. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 10:56, Tim Bunce tim.bu...@pobox.com wrote: On Wed, Feb 03, 2010 at 10:18:51AM -0700, Alex Hunsaker wrote: On Wed, Feb 3, 2010 at 09:31, Tim Bunce tim.bu...@pobox.com wrote: On Tue, Feb 02, 2010 at 08:42:21PM -0700, Alex Hunsaker wrote: On Sat, Jan 30, 2010 at 08:49, Tim Bunce tim.bu...@pobox.com wrote: SPI functions are not available when the code is run. Hrm, we might want to stick why in the docs or as a comment somewhere. Ah, yes, a comment is certainly easier to write up. I'll add one and include a link to the relevant thread in the archives. Thanks for the example. BTW I (obviously?) stuck the example in the wrong spot in plperl.c. I did not have a tree with your patches applied handy :) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Tim Bunce tim.bu...@pobox.com writes: I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). What exactly are you proposing to check, and where, and what do you think that will fix? If the concern is that someone could sabotage the behavior of a plperl function by changing things around in the perl_init script, then I think we have to forget about making it USERSET. Whether someone has been granted permission to use plperl seems to me to have little to do with whether it's okay to mess up a function (possibly a SECURITY DEFINER one) belonging to someone else. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication and SSL
On Wed, Feb 3, 2010 at 08:23, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Fujii Masao wrote: On Thu, Jan 14, 2010 at 7:04 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: 1. Walsender calls pq_wait() which calls select(), waiting for timeout, or data to become available for reading in the underlying socket. 2. Client issues an SSL renegotiation by sending a message to the server 3. Server receives the message, and select() returns indicating that data has arrived 4. Walsender calls HandleEndOfRep() which calls pq_getbyte(). pq_readbyte() calls SSL_read(), which receives the renegotiation message and handles it. No application data has arrived, however, so SSL_read() blocks for some to arrive. It never does. What is the trigger of the renegotiation? The backend initiates it when the amount of data sent exceeds the RENEGOTIATION_LIMIT (which is defined in src/backend/libpq/be-secure.c). OTOH, I cannot find the code that the libpq explicitly does that. So I wonder if client (i.e., walreceiver in this case) really sends the SSL renegotiation message. Correct me if I'm wrong. I have no idea. I thought the SSL library can do so whenever it feels like it, but I'm not sure. It can only do it when we call the library. Which means at send or receive :-) But AFAIK either end (sender or receiver) can initiate it. The other problem scenario was that the server receive only the first half of an SSL packet. That doesn't produce any data available to read with SSL_read(), so SSL_read() will block, but it does wake up a select(). Yeah. It can be re-woken, because SSL_read() will eventually be calling back into our own functions, but that would require a second signal before it wakes up of course.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Tom Lane escribió: Michael Ledford mledf...@gmail.com writes: One might argue that the current method is already weakened as it is measured by the amount of data sent instead of of a length of time. A session could live a long time under the 512MB threshold depending on the queries that are being performed. Renegotiation after X amount of data is the recommended method AFAIK, because it limits the volume of data available to cryptanalysis. What makes you think that elapsed time is relevant at all? FWIW I think there's another problem with streaming replication here, which is that most data flows from client to server, so it would take quite some time for the threshold to be reached. Note that there's no size check in the libpq frontend code. Normally this is not an issue because the bulk of data is expected to flow in the other direction. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Tom Lane wrote: The argument for doing this now hinges solely on a marketing-driven choice of version name, and not on any actual evidence that applications are ready for it. We really need to do this at the start of a devel and alpha test cycle, not at the end. Application writers probably didn't bother all that much with alphas though. The bulk of them is going to start with the betas, which have not been delivered yet, so it seems a good time to try. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Robert Haas robertmh...@gmail.com wrote: What harm is being done by the status quo? What benefit do we get out of changing the default? I really think that the biggest harm is that people trying to convert to PostgreSQL, or testing PostgreSQL with their applications, can get bad behavior from use of standard string literals. If they post to a list and we point out the setting, that'll probably be the end of the trouble -- and I have seen a few such posts. Interestingly, the frequency of such posts dropped off after 8.2 was released with the GUC to configure it, which suggests that people are often reading documentation before making the attempt or at least doing web searches about the problem and fixing it without a post to the community. I do think we might be well-served to have such issues as this and the it's not a character string literal, it's a literal of UNKNOWN type covered in a page which is prominent enough to be likely to be read by those considering migration or compatibility testing. I'm not sure exactly where that would be, unless it's a couple more FAQ entries -- but a compatibility and migration page might be worth creating, with a reasonably prominent link from the home page. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 11:28, Tom Lane t...@sss.pgh.pa.us wrote: Tim Bunce tim.bu...@pobox.com writes: I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). What exactly are you proposing to check, and where, and what do you think that will fix? Non plperl GRANTed people could modify the global $_SHARED variable. Currently anyone that can make a plperl function can do anything they want with $_SHARED. So In my mind disallowing them to set plperl.plperl_safe_init would make the permission model of $_SHARED consistent. No? Now im not saying its a good permission model... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent vendor SSL renegotiation patches break PostgreSQL
Alvaro Herrera alvhe...@commandprompt.com writes: FWIW I think there's another problem with streaming replication here, which is that most data flows from client to server, so it would take quite some time for the threshold to be reached. Note that there's no size check in the libpq frontend code. Normally this is not an issue because the bulk of data is expected to flow in the other direction. Huh? I thought the slaves connect to the master, rather than the other way round? It's true that libpq doesn't contain any such code, but that seems like a fortunate thing right at the moment, as it limits the number of places we might have to hack something. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Perl (DBD::Pg anyway) has been compatible since May 2008. I would interpret that to mean that there is a significant possibility that a too-old DBD::Pg could get used with a new PostgreSQL, and therefore we shouldn't change anything for 9.0. May 2008 is not that long ago, especially for people running systems like RHEL with five-year major release cycles. That's a silly conclusion. Applications and drivers are always going to lag behind. If someone is having a problem, they either upgrade their DBD::Pg or flip the GUC. Are you really saying we should wait until 2008 +5 years (2013!) before making this change? Wouldn't we have to wait five years past the point when *all* drivers are compatible by your definition? I am not sure I really understand why anyone is a rush to make this change. What harm is being done by the status quo? What benefit do we get out of changing the default? The major argument that has been offered so far is that if we don't change it now, we never will, but I don't believe that the tenor of this discussion supports the contention that Tom or anyone else never wants to make this change. It's hardly a rush (the GUC has been around and is being used in production), and the benefit is standards compatibility, something we strive for around here. I personally don't agree with the now or never argument, but I do agree with the dot zero release is a good time for changes like this argument. It also seems to overlook the fact that we are STILL dealing with the fallout from this change in the core code; Tom gave examples upthread of changes that are being released for the first time *in 9.0* to address problems created by this transition. And that is just the core code; we have to expect that third-party code will lag behind. Which fallout are we still dealing with? Are you saying that the developers are not up to the challenge of handling this before 9.0 is released? (If this were anything more than a simple boolean GUC fix, I would be in your corner). Yes, third-party code will lag behind, but, again, that's the nature of the game. We didn't wait for every driver, app, and script to support schemas before we added them in 7.4, for example. We certainly didn't wait for applications to be implicit casting ready before 8.3, to (over?)use another example. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201002031342 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAktpxEUACgkQvJuQZxSWSsibFwCeJzeQzUTBFwqHQ451Y23cbLfT 4UUAoK/2Sg/pxq5ipdB2B2ekfzQgW0cT =5/gh -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Alvaro Herrera alvhe...@commandprompt.com writes: Tom Lane wrote: The argument for doing this now hinges solely on a marketing-driven choice of version name, and not on any actual evidence that applications are ready for it. We really need to do this at the start of a devel and alpha test cycle, not at the end. Application writers probably didn't bother all that much with alphas though. The bulk of them is going to start with the betas, which have not been delivered yet, so it seems a good time to try. I still think that changing it now is going to open a can of worms that we shouldn't be opening at this stage. We have got more than enough to worry about for 9.0 already. I think it is absolute folly to believe that this is only going to be a matter of flip the default and nothing else is going to pop up. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 11:43, Alex Hunsaker bada...@gmail.com wrote: On Wed, Feb 3, 2010 at 11:28, Tom Lane t...@sss.pgh.pa.us wrote: Tim Bunce tim.bu...@pobox.com writes: I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). What exactly are you proposing to check, and where, and what do you think that will fix? Non plperl ... Put another way: HEAD: only people with plperl granted can make functions to manipulate $_SHARED PATCH: anyone can set plperl.plperl_safe_init (but note not plperlu_init as its SUSER) and manipulate $_SHARED Proposed fix: only people with plperl granted can set plperl.plplerl_safe_init and hence restore HEAD behavior -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). What exactly are you proposing to check, and where, and what do you think that will fix? If the concern is that someone could sabotage the behavior of a plperl function by changing things around in the perl_init script, then I think we have to forget about making it USERSET. Whether someone has been granted permission to use plperl seems to me to have little to do with whether it's okay to mess up a function (possibly a SECURITY DEFINER one) belonging to someone else. If we are seriously worried about use of %_SHARED in security definer functions then ISTM the right thing might be to have more than one and swap in the one for the effective user in a security definer function. Or possibly even disable it altogether in security definer functions. %_SHARED has been around for several years now, and if there are genuine security concerns about it ISTM they would apply today, regardless of these patches. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Andrew Dunstan and...@dunslane.net writes: %_SHARED has been around for several years now, and if there are genuine security concerns about it ISTM they would apply today, regardless of these patches. Yes. I am not at all happy about inserting nonstandard permissions checks into GUC assign hooks --- they are not really meant for that and I think there could be unexpected consequences. Without a serious demonstration of a real problem that didn't exist before, I'm not in favor of it. I think a more reasonable answer is just to add a documentation note pointing out that %_SHARED should be considered insecure in a multi-user database. What I was actually wondering about, however, is the extent to which the semantics of Perl code could be changed from an on_init hook --- is there any equivalent of changing search_path or otherwise creating trojan-horse code that might be executed unexpectedly? And if so is there any point in trying to guard against it? AIUI there isn't anything that can be done in on_init that couldn't be done in somebody else's function anyhow. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On Wed, Feb 3, 2010 at 13:20, Robert Haas robertmh...@gmail.com wrote: On Wed, Feb 3, 2010 at 12:34 PM, Greg Sabino Mullane g...@turnstep.com wrote: Perl (DBD::Pg anyway) has been compatible since May 2008. I would interpret that to mean that there is a significant possibility that a too-old DBD::Pg could get used with a new PostgreSQL, and therefore we shouldn't change anything for 9.0. May 2008 is not that long ago, especially for people running systems like RHEL with five-year major release cycles. I fall into this camp with a few machines still running standard RHEL 4 which I believe has DBD::Pg 1.32 installed. We do keep up to date with PostgreSQL but the machines connecting to it include everything from brand new web servers through to ancient machines in accounting running reports. As much as I would like GUCs to disappear I think this one should proceed cautiously and probably be a 9.1 or even 9.2 item. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
I still think that changing it now is going to open a can of worms that we shouldn't be opening at this stage. We have got more than enough to worry about for 9.0 already. I think it is absolute folly to believe that this is only going to be a matter of flip the default and nothing else is going to pop up. I'll support Tom on this. I'm already worried about the timeline. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Making pg_config and pg_controldata output available via SQL
I'd really like to see the data from pg_config and pg_controldata available through SQL, such as by adding output to pg_show_all_settings(), or adding new SRFs named something like pg_config() and pg_controldata(). Does this make as much sense to the rest of the world as it does to me? In particular it's useful to be able to find $libdir without requiring pg_config, as some packagers tend not to include it in anything put the -dev packages, but all those settings seem useful to have on hand, and in at least most cases shouldn't be tough to expose via SQL. Comments? -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com signature.asc Description: Digital signature
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Feb 3, 2010, at 11:04 AM, Tom Lane wrote: What I was actually wondering about, however, is the extent to which the semantics of Perl code could be changed from an on_init hook --- is there any equivalent of changing search_path or otherwise creating trojan-horse code that might be executed unexpectedly? Yes. And if so is there any point in trying to guard against it? No. This is Perl we're talking about. The DBA should vet code before putting it into on_perl_init. AIUI there isn't anything that can be done in on_init that couldn't be done in somebody else's function anyhow. Correct. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On Wed, Feb 3, 2010 at 1:46 PM, Greg Sabino Mullane g...@turnstep.com wrote: Perl (DBD::Pg anyway) has been compatible since May 2008. I would interpret that to mean that there is a significant possibility that a too-old DBD::Pg could get used with a new PostgreSQL, and therefore we shouldn't change anything for 9.0. May 2008 is not that long ago, especially for people running systems like RHEL with five-year major release cycles. That's a silly conclusion. Applications and drivers are always going to lag behind. If someone is having a problem, they either upgrade their DBD::Pg or flip the GUC. Are you really saying we should wait until 2008 +5 years (2013!) before making this change? Wouldn't we have to wait five years past the point when *all* drivers are compatible by your definition? I don't think it's a silly conclusion at all, though it's possible that I am a silly person. The longer we wait before making an incompatible change, the more people will have adjusted their code to the new reality (or upgraded their drivers, etc.) and the fewer things will break. Taking this argument to its illogical extreme, we should never change anything at all, but I'm not proposing that. What I am saying is that I got all of the standard_conforming_strings problems in my own code (that I know about) fixed about a year ago, and it does not seem implausible to think that there could be people in the world who take longer to upgrade than I do. In fact, it seems overwhelmingly likely. Kevin Grittner made a good point upthread: the harm in NOT changing standard_conforming_strings is that we will endure for a longer period of time with strings that, well, don't conform to the standard, which may cause problems for people trying to migrate to PostgreSQL from other database systems. Conversely, the harm in changing it is that it may break existing PostgreSQL applications that run just fine on older releases. The second problem is something that we can expect to gradually decrease over time because of (1) the incredibly annoying escape_string_warning behavior and (2) software version upgrades. Exactly when the risk is low enough to make the change is a judgement call. It also seems to overlook the fact that we are STILL dealing with the fallout from this change in the core code; Tom gave examples upthread of changes that are being released for the first time *in 9.0* to address problems created by this transition. And that is just the core code; we have to expect that third-party code will lag behind. Which fallout are we still dealing with? Are you saying that the developers are not up to the challenge of handling this before 9.0 is released? (If this were anything more than a simple boolean GUC fix, I would be in your corner). http://archives.postgresql.org/pgsql-hackers/2010-01/msg02992.php Yes, third-party code will lag behind, but, again, that's the nature of the game. We didn't wait for every driver, app, and script to support schemas before we added them in 7.4, for example. We certainly didn't wait for applications to be implicit casting ready before 8.3, to (over?)use another example. Implicit casting was in some ways less of a big deal than this change, at least for me. It broke some things, but they all BROKE, and then I fixed them. When this standard_conforming_strings thing hit, all of my scripts that run out of cron started pouring out warning messages which I initially could not figure out how to get rid of. IIRC, whatever version of Fedora I was running at the time had a version of PostgreSQL that generated these stupid warnings, and a version of DBD::Pg that hadn't yet been updated. It was thoroughly miserable. Yeah, I probably could have gotten around it by writing my own custom escaping function or downloading DBD::Pg off of CPAN and compiling it, but at the time I didn't even understand whether that would actually fix the problem. Plus, I'm not sure anyone here would be willing to advocate the way that the implicit casting stuff went down as a model for future changes of similar type. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 1:49 PM, Alex Hunsaker bada...@gmail.com wrote: On Wed, Feb 3, 2010 at 11:43, Alex Hunsaker bada...@gmail.com wrote: On Wed, Feb 3, 2010 at 11:28, Tom Lane t...@sss.pgh.pa.us wrote: Tim Bunce tim.bu...@pobox.com writes: I do see a need for a GRANT check and I'm adding one now (based on the code in CreateFunction() in functioncmds.c - thanks to RhodiumToad on IRC for the pointer). What exactly are you proposing to check, and where, and what do you think that will fix? Non plperl ... Put another way: HEAD: only people with plperl granted can make functions to manipulate $_SHARED PATCH: anyone can set plperl.plperl_safe_init (but note not plperlu_init as its SUSER) and manipulate $_SHARED Proposed fix: only people with plperl granted can set plperl.plplerl_safe_init and hence restore HEAD behavior I think we should just not have any unprivileged-user-settable GUCs and then this problem goes away. Trying to test for GRANT privilege on the appropriate language seems like a big kludge, and I am doubtful that it's worth it. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Greg Sabino Mullane g...@turnstep.com writes: Which fallout are we still dealing with? Are you saying that the developers are not up to the challenge of handling this before 9.0 is released? (If this were anything more than a simple boolean GUC fix, I would be in your corner). I'm not certain that Robert is saying that, but *I* am. We have enough to do for 9.0; adding another work item of uncertain magnitude is not the thing to be doing right now. The notion that it's a simple boolean GUC fix and won't cause any followup work is unjustifiably optimistic. And that's just for the core code. I don't want to blindside driver writers and other third-party authors with a change like this made at the end of the cycle. If we do it at the beginning of the 9.1 devel cycle, no one will have room to argue that they didn't have adequate notice ... but they sure will be able to make that complaint if we do it now. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
On Wed, Feb 3, 2010 at 12:04, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: %_SHARED has been around for several years now, and if there are genuine security concerns about it ISTM they would apply today, regardless of these patches. Yes. I am not at all happy about inserting nonstandard permissions checks into GUC assign hooks I think Tims solution is just to check in plperl.c right before we eval it so not at SET time. I think a more reasonable answer is just to add a documentation note pointing out that %_SHARED should be considered insecure in a multi-user database. Works for me. We probably want to do that anyway. is there any equivalent of changing search_path or otherwise creating trojan-horse code that might be executed unexpectedly? Yes but not in the plperl variant. Only with plperlu or the plperl.init GUCs marked SUSER could you do any of the above. Which makes me think maybe the plperl.plperlu_init function could just have a similar permission check to plperl.plperl_safe_init and be USERSET ? Too gross? And if so is there any point in trying to guard against it? AIUI there isn't anything that can be done in on_init that couldn't be done in somebody else's function anyhow. Right, the point is you can only do that if you can make those functions (or if someone prepared a nice function for you to use). Maybe im just being paranoid. Leaving it the way it is now (USERSET) is certainly easier. =) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On 02/03/2010 01:20 PM, Robert Haas wrote: I am not sure I really understand why anyone is a rush to make this change. What harm is being done by the status quo? What benefit do we get out of changing the default? The major argument that has been offered so far is that if we don't change it now, we never will, but I don't believe that the tenor of this discussion supports the contention that Tom or anyone else never wants to make this change. For myself, it isn't so much a rush as a sense that the code out there that will break, will never change unless forced, and any time seems better than never. Correct me if I am wrong - but I think this issue represents an exploitable SQL injection security hole. I switched because I convinced myself that the ambiguity of \' represented actual danger. I'm concerned that if the web front end doing parameter checking and passing in code using either '' quoting or \' quoting can be exploited if the server happens to be configured the opposite way. To me, this ambiguity can only be addressed by everybody agreeing on the right way to do it, and '' quoting seems like the right way to do it to me. Cheers, mark -- Mark Mielkem...@mielke.cc -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl UPDATED [PATCH]
Alex Hunsaker bada...@gmail.com writes: On Wed, Feb 3, 2010 at 12:04, Tom Lane t...@sss.pgh.pa.us wrote: Yes. Â I am not at all happy about inserting nonstandard permissions checks into GUC assign hooks I think Tims solution is just to check in plperl.c right before we eval it so not at SET time. Well, that would be *completely* wrong/useless. What you would find out is the ID of the user who directly called the function, which would have nothing at all to do with the privileges of whoever set the GUC. I'm leaning in the same direction as Robert: let's just make all three of these SUSET and stop worrying. It's not real clear that there's much of a use-case for letting unprivileged users set on_plperl_init anyway. Also, we can always back it off later if we decide it's safer than it looks. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On 02/03/2010 02:15 PM, Robert Haas wrote: The longer we wait before making an incompatible change, the more people will have adjusted their code to the new reality (or upgraded their drivers, etc.) and the fewer things will break. In my experience, the opposite is true, although in this case, the damage may already be done. That is, the longer bad habits are allowed to form, the harder they are to break, and the more code is written that may be broken. People won't upgrade unless forced. At some point, the switch does have to be tripped. Is now the time? I have no comment. I just don't want to see never be the time, and if never is not the time, than now does not seem impratical. That said, if you say we'll tell people to prepare for a change in 9.0, and enforce the change in a later release, that is fine too. Cheers, mark -- Mark Mielkem...@mielke.cc -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Making pg_config and pg_controldata output available via SQL
Joshua Tolley eggyk...@gmail.com writes: I'd really like to see the data from pg_config and pg_controldata available through SQL, such as by adding output to pg_show_all_settings(), or adding new SRFs named something like pg_config() and pg_controldata(). Does this make as much sense to the rest of the world as it does to me? In particular it's useful to be able to find $libdir without requiring pg_config, as some packagers tend not to include it in anything put the -dev packages, but all those settings seem useful to have on hand, and in at least most cases shouldn't be tough to expose via SQL. Comments? I wonder whether there's a security issue there. Telling an attacker whether you've been built with feature X seems like possibly useful info that he couldn't otherwise get without local machine access. In particular, we already try to avoid exposing server filesystem path information. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
Rod Taylor escribió: As much as I would like GUCs to disappear I think this one should proceed cautiously and probably be a 9.1 or even 9.2 item. Note that this is *not* about removing the configuration setting, only about flipping its default value. There has been *no* talk of removing the setting. If you have old clients around, simply change the value from the default to off. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG 9.0 and standard_conforming_strings
On Wed, Feb 3, 2010 at 2:25 PM, Mark Mielke m...@mark.mielke.cc wrote: On 02/03/2010 01:20 PM, Robert Haas wrote: I am not sure I really understand why anyone is a rush to make this change. What harm is being done by the status quo? What benefit do we get out of changing the default? The major argument that has been offered so far is that if we don't change it now, we never will, but I don't believe that the tenor of this discussion supports the contention that Tom or anyone else never wants to make this change. For myself, it isn't so much a rush as a sense that the code out there that will break, will never change unless forced, and any time seems better than never. Correct me if I am wrong - but I think this issue represents an exploitable SQL injection security hole. I switched because I convinced myself that the ambiguity of \' represented actual danger. I'm concerned that if the web front end doing parameter checking and passing in code using either '' quoting or \' quoting can be exploited if the server happens to be configured the opposite way. To me, this ambiguity can only be addressed by everybody agreeing on the right way to do it, and '' quoting seems like the right way to do it to me. OK, you're wrong. :-) Yeah, there's a problem if the client and server are configured in opposite ways, but flipping the default setting of standard_conforming_strings is not going to make that problem go away. If anything it's going to make it worse. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
