Re: [HACKERS] Streaming replication and pg_xlogfile_name()
Fujii Masao wrote: In relation to the functions added recently, I found an annoying problem; pg_xlogfile_name(pg_last_xlog_receive/replay_location()) might report the wrong name because pg_xlogfile_name() always uses the current timeline, and a backend doesn't know the actual timeline related to the location which pg_last_xlog_receive/replay_location() reports. Even if a backend knows that, pg_xlogfile_name() would be unable to determine which timeline should be used. Hmm, I'm not sure what the use case for this is, but I agree it seems annoying that you can almost reconstruct the exact filename, but not quite because of the possible change in timeline ID. To solve this problem, I'm thiking to add the following functions: * pg_current_timeline() reports the current timeline ID. * pg_last_receive_timeline() reports the timeline ID which is related to the last WAL receive location. * pg_last_replay_timeline() reports the timeline ID which is related to the last WAL replay location. * pg_xlogfile_name(location text [, timeline bigint ]) reports the WAL file name using the given timeline. By default, the current timeline is used. * pg_xlogfile_name_offset(location text [, timeline bigint]) reports the WAL file name and offset using the given timeline. By default, the current timeline is used. That gets quite complicated to use. And there's a little race condition too: when you call pg_last_replay_timeline() and pg_last_xlog_replay_location() functions to get the timeline and XLogRecPtr of the last replayed record, the timeline might change in between the calls, so you end up with a combination that was never actually replayed. How about extending the format of the string returned by pg_last_xlog_receive/replay_location() to include the timeline ID? When it currently returns e.g '6/200016C', it could return '1/6/200016C', where 1 is the timeline ID. Then just teach pg_xlogfile_name[_offset]() to accept that format as well. -- 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] Review: listagg aggregate
2010/1/28 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp: Pavel Stehule pavel.steh...@gmail.com wrote: with actualised oids I'm checking the patch for commit, and have a couple of comments. * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. no I dislike it. This using is nonsense. Regards Pavel * Can we use StringInfo directly as the aggregate context instead of StringAggState? For the first reason, we need to drop 'delimiter' field from struct StringAggState. Now it has only StringInfo field. * We'd better avoiding to call text_to_cstring() for delimitors and elements for performance reason. We can use appendBinaryStringInfo() here. My proposal patch attached. Also, I've not changed it yet, but it might be considerable: * Do we need better names for string_agg1_transfn and string_agg2_transfn? They are almost internal names, but we could have more like string_agg_with_sep_transfn. Comments? 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: listagg aggregate
2010/1/28 David E. Wheeler da...@kineticode.com: On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. Ooh, nice. * Can we use StringInfo directly as the aggregate context instead of StringAggState? For the first reason, we need to drop 'delimiter' field from struct StringAggState. Now it has only StringInfo field. Makes sense. no, has not. Pavel * We'd better avoiding to call text_to_cstring() for delimitors and elements for performance reason. We can use appendBinaryStringInfo() here. My proposal patch attached. Also, I've not changed it yet, but it might be considerable: * Do we need better names for string_agg1_transfn and string_agg2_transfn? They are almost internal names, but we could have more like string_agg_with_sep_transfn. Yes please. Comments? Patch looks great, thank you! 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] Review: listagg aggregate
2010/1/28 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 David E. Wheeler da...@kineticode.com: On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. Ooh, nice. * Can we use StringInfo directly as the aggregate context instead of StringAggState? For the first reason, we need to drop 'delimiter' field from struct StringAggState. Now it has only StringInfo field. Makes sense. no, has not. What is use case for this behave?? Pavel Pavel * We'd better avoiding to call text_to_cstring() for delimitors and elements for performance reason. We can use appendBinaryStringInfo() here. My proposal patch attached. Also, I've not changed it yet, but it might be considerable: * Do we need better names for string_agg1_transfn and string_agg2_transfn? They are almost internal names, but we could have more like string_agg_with_sep_transfn. Yes please. Comments? Patch looks great, thank you! 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] Patch: libpq new connect function (PQconnectParams)
Le 28/01/2010 07:32, Joe Conway a écrit : On 01/26/2010 02:55 PM, Guillaume Lelarge wrote: Le 26/01/2010 19:43, Joe Conway a écrit : On 01/25/2010 03:21 PM, Guillaume Lelarge wrote: I didn't put any documentation before knowing which one will be choosen. So we still need to work on the manual. Please send the documentation as a separate patch. Once I have that I will commit the posted patch, barring any objections in the meantime. You'll find it attached with this mail. Please read it carefully, my written english is not that good. Final committed patch attached. One last code correction -- in psql/startup.c the original patch defines the keywords array in the body of the code, rather than at the top of the block. Minor improvements ( hopefully ;-)) to the documentation as well. Thanks a lot. -- Guillaume. http://www.postgresqlfr.org http://dalibo.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] Review: listagg aggregate
Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/28 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 David E. Wheeler da...@kineticode.com: On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. no, has not. What is use case for this behave?? I also think this usage is nonsense, but seems to be the most consistent behavior for me. I didn't say anything about use-cases, but just capability. Since we allow such kinds of usage for now, you need to verify the delimiter is not changed rather than ignoring it if you want disallow to change the delimiter during an aggregation. Of course you can cache the first delimiter at start, and check delimiters are not changed every calls -- but I think it is just a waste of cpu cycle. 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] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 06:33:19PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 11:28:02AM -0500, Tom Lane wrote: Really? We've found that gprof, for instance, doesn't exactly have zero interaction with the rest of the backend --- there's actually a couple of different bits in there to help it along, including a behavioral change during shutdown. I rather doubt that Perl profilers would turn out much different. Devel::NYTProf (http://blog.timbunce.org/tag/nytprof/) has zero interaction with the rest of the backend. I don't have to read any further than the place where it says doesn't work if you call both plperl and plperlu to realize that that's quite false. NYTProf is not, currently, multiplicity-safe. That's a limitation I intend to fix. Maybe we have different definitions of what a software interaction is... Doing _anything_ in the backend is an interaction of some kind, e.g., shifting later memory allocations to a different address. But that's not a very practical basis for a definition. From what you said, quoted above, it seemed that your definition of interaction with the rest of the backend was more much more direct. The specific example you gave related to the backend code needing to be modified to support the gprof profiler. Clearly that's not the case for NYTProf. We're splitting hairs now. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: The attached patch uses one TOC entry for each blob objects. When I'm testing the new patch, I found ALTER LARGE OBJECT command returns ALTER LARGEOBJECT tag. Should it be ALTER LARGE(space)OBJECT instead? As I remember, we had decided not to use LARGEOBJECT (without a space) in user-visible messages, right? 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: Typed Table
Peter Eisentraut wrote: Everyone, We could use some help. Anyone's got an idea what could be causing the behavior described below? On mån, 2010-01-25 at 21:45 +0200, Peter Eisentraut wrote: On tis, 2010-01-19 at 01:01 +0900, Hitoshi Harada wrote: * Conflict between transactions I'm not sure if this is related with the patch but I met this situation; A: regression=# create type persons_type as (name text, bdate date); A: CREATE TYPE A: regression=# begin; A: BEGIN A: regression=# drop type persons_type; A: DROP TYPE B: regression=# create table persons of persons_type; (LOCK) A: regression=# rollback; A: ROLLBACK B: CREATE TABLE B: regression=# drop table persons; B: DROP TABLE A: regression=# begin; A: BEGIN A: regression=# drop type persons_type; A: DROP TYPE B: regression=# create table persons of persons_type; (NO LOCK) B: CREATE TABLE A: regression=# commit; A: COMMIT B: regression=# select 'persons_type'::regtype; B: ERROR: type persons_type does not exist B: LINE 1: select 'persons_type'::regtype; I have at all no idea why the second create table doesn't lock. Well, if you try the same thing with CREATE FUNCTION foo() RETURNS persons_type AS $$ blah $$ LANGUAGE plpythonu; or some similar cases, there is also no lock. You will notice that (some/many?) DDL statements actually behave very poorly against concurrent other DDL. Against that background, however, the real question is why the first case *does* lock. I don't know. Types are cached in typcache. At the first CREATE TABLE, the type is not in cache, and lookup_type_cache() (by the call to lookup_rowtype_tupdesc() in transformOfType()) calls relation_open() which blocks. On the second call, however, it's already in the cache, and relation_open is not called. ISTM you should explicitly grab a lock on the of-type at some point, to make sure it doesn't get dropped while you're busy creating the table. How do we protect against that for the types used in columns? For example, if you do CREATE TABLE (foo mytype), and someone tries to drop mytype simultaneously? -- 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] Add on_perl_init and proper destruction to plperl [PATCH]
On Wed, Jan 27, 2010 at 06:27:50PM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: Okay. I could change the callback code to ignore calls if proc_exit_inprogress is false. So an abnormal shutdown via exit() wouldn't involve plperl at all. (Alternatively I could use use on_proc_exit() instead of atexit() to register the callback.) Use on_proc_exit please. I will continue to object to any attempt to hang arbitrary processing on atexit(). Ok. An advantage of on_proc_exit from your end is that it should allow you to not have to try to prevent the END blocks from using SPI, as that would still be perfectly functional when your callback gets called. (Starting a new transaction would be a good idea though, cf Async_UnlistenOnExit.) I'm surprised that you're suggesting that END block should be allowed to interact with the backend via SPI. It seems to go against what you've said previously about code running at shutdown. I've no use-case for that so I'm happy to leave it disabled. If someone does have a sane use-case, please let me know. It can always be enabled later. 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] quoting psql varible as identifier
2010/1/27 Robert Haas robertmh...@gmail.com: On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I hope, so this version is more readable and more clean. I removed some not necessary checks. This still seems overly complicated to me. I spent a few hours today working up the attached patch. Let me know your thoughts. There is serious issue. The psql_error only shows some message on output, but do nothing more - you don't set a result status for commands and for statements. So some potential error from parsing is pseudo quietly ignored - without respect to your setting ON_ERROR_STOP. This could be a problem for commands. Execution of broken SQL statements will raise syntax error. But for \set some variable will be a broken and the content can be used. I don't thing so it is good. It is limited. Your version is acceptable only when we don't enable escape syntax for commands. Then we don't need check it. On your version - I am not sure if it is fully compatible, and using a global variables isn't nice. I little bit modify my original code - it is more verbose (- useless using pqexpbuffer) - and more consistent with previous behave. Pavel ...Robert variable-escaping.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication, and walsender during recovery
On Thu, Jan 28, 2010 at 4:47 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I think there's a race condition at the end of recovery. When the shutdown checkpoint is written, with new TLI, doesn't a cascading walsender try to send that to the standby as soon as it's flushed to disk? But it won't find it in the WAL segment with the old TLI that it's reading. Right. But I don't think that such a shutdown checkpoint record is worth being sent by a cascading walsender. I think that such a walsender has only to exit without regard to the WAL segment with the new TLI. Also, when segments are restored from the archive, using restore_command, the cascading walsender won't find them because they're not written in pg_xlog like normal WAL segments. Yeah, I need to adjust my approach to the recent 'xlog-refactor' change. The archived file needs to be restored without a name change, and remain in pg_xlog until the bgwriter will have recycled it. But that change would make the xlog.c even more complicated. Should we postpone the 'cascading walsender' feature into v9.1, and, in v9.0, just forbid walsender to be started during recovery? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION 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] Streaming replication, and walsender during recovery
Fujii Masao wrote: On Thu, Jan 28, 2010 at 4:47 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I think there's a race condition at the end of recovery. When the shutdown checkpoint is written, with new TLI, doesn't a cascading walsender try to send that to the standby as soon as it's flushed to disk? But it won't find it in the WAL segment with the old TLI that it's reading. Right. But I don't think that such a shutdown checkpoint record is worth being sent by a cascading walsender. I think that such a walsender has only to exit without regard to the WAL segment with the new TLI. Also, when segments are restored from the archive, using restore_command, the cascading walsender won't find them because they're not written in pg_xlog like normal WAL segments. Yeah, I need to adjust my approach to the recent 'xlog-refactor' change. The archived file needs to be restored without a name change, and remain in pg_xlog until the bgwriter will have recycled it. I guess you could just say that it's working as designed, and WAL files restored from archive can't be streamed. Presumably the cascaded slave can find them in the archive too. But it is pretty weird, doesn't feel right. This reminds me of something I've been pondering anyway. Currently, restore_command copies the restored WAL segment as pg_xlog/RECOVERYXLOG instead of the usual 0... filename. That avoids overwriting any pre-existing WAL segments in pg_xlog, which may still contain useful data. Using the same filename over and over also means that we don't need to worry about deleting old log files during archive recovery. The downside in standby mode is that once standby has restored segment X from archive, and it's restarted, it must find X in the archive again or it won't be able to start up. The archive better be a directory on the same host. Streaming Replication, however, took another approach. It does overwrite any existing files in pg_xlog, we do need to worry about deleting old files, and if the master goes down, we can always find files we've already streamed in pg_xlog, so the standby can recover even if the master can't be contacted anymore. That's a bit inconsistent, and causes the problem that a cascading walsender won't find the files restored from archive. How about restoring/streaming files to a new directory, say pg_xlog/restored/, with the real filenames? At least in standby_mode, probably best to keep the current behavior in PITR. That would feel more clean, you could easily tell apart files originating from the server itself and those copied from the master. But that change would make the xlog.c even more complicated. Should we postpone the 'cascading walsender' feature into v9.1, and, in v9.0, just forbid walsender to be started during recovery? That's certainly the simplest solution... -- 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] CommitFest status summary 2010-01-27
Hi, Greg Smith írta: Provide rowcount for utility SELECTs I think I can write a review for this one right now based on the discussion that's already happened: -Multiple people think the feature is valuable and it seems worth exploring -The right way to handle the protocol change here is still not clear -There are any number of subtle ways clients might be impacted by this change, and nailing that down and determining who might break is an extremely wide ranging bit of QA work that's barely been exploring yet That last part screams returned with feedback for something submitted to the last CF before beta to me. As a candidate for 9.1-alpha1 where there's plenty of time to figure out what it breaks and revert if that turns out to be bad? That sounds like a much better time to be fiddling with something in this area. I understand your position, this is a subtle change that might turn out to break clients, indeed. I would like to see the following in order to make this patch have a shot at being comittable: 1) Provide some examples showing how the feature would work in practice and of use-cases for it. I'll try to explore all the things affected by this change and reflect them in a regression test. 2) To start talking about what's going to break, some notes about what this does to the regression tests would be nice. Is there a way to run a regression test under src/test/regress so the command status string is also written into the *.out file? It doesn't seem to me so because all the current *.out files only contain results for tuple-returning statements and src/test/regress/pg_regress_main.c runs psql in quiet mode. So, first I suggest dropping the -q option from the psql command line in psql_start_test() in pg_regress_main.c to be able to see the command strings. 3) Demonstrate with example sessions the claims that there are no backward compatibility issues here. I read when you mix old server with new clients or new server with old client, it will just work as before, but until I see a session proving that I have to assume that's based on code inspections rather than actual tests, and therefore not necessarily true. (Nothing personal, Zoltan--just done way too much QA in the last year to believe anything I'm told about code without a matching demo). 4) Investigate and be explicit about the potential breakage here both for libpq clients and at least one additional driver too. If I saw a demonstration that this didn't break the JDBC driver, for example, I'd feel a lot better about the patch. I thought the libpq change was obvious. strncmp(status, SELECT , 7) /* one space at the end */ doesn't match SELECT (no spaces). So: 1. old server sends SELECT, the code in the new libpq client gets a doesn't match, PQcmdTuples() returns . 2. new server sends SELECT N, old libpq client doesn't look for strings starting with SELECT, PQcmdTuples() returns I looked at the latest JDBC source (currently it's postgresql-jdbc-8.4-701) these are the places I found where the command status interpreted in core/v3/QueryExecutorImpl.java: private String receiveCommandStatus() throws IOException { //TODO: better handle the msg len int l_len = pgStream.ReceiveInteger4(); //read l_len -5 bytes (-4 for l_len and -1 for trailing \0) String status = pgStream.ReceiveString(l_len - 5); //now read and discard the trailing \0 pgStream.Receive(1); if (logger.logDebug()) logger.debug( =BE CommandStatus( + status + )); return status; } and private void interpretCommandStatus(String status, ResultHandler handler) { int update_count = 0; long insert_oid = 0; if (status.startsWith(INSERT) || status.startsWith(UPDATE) || status.startsWith(DELETE) || status.startsWith(MOVE)) { try { update_count = Integer.parseInt(status.substring(1 + status.lastIndexOf(' '))); if (status.startsWith(INSERT)) insert_oid = Long.parseLong(status.substring(1 + status.indexOf(' '), status.lastIndexOf(' '))); } catch (NumberFormatException nfe) { handler.handleError(new PSQLException(GT.tr(Unable to interpret the update count in command completion tag: {0}., status), PSQLState.CONNECTION_FAILURE)); return ; } } handler.handleCommandStatus(status, update_count, insert_oid); } receiveCommandStatus() reads everything up to and including the zero termination byte. interpretCommandStatus() handles only the old strings expected to carry the rowcount. This wouldn't break for the new SELECT N string as it falls into the latest (here nonexisting) else branch, effectively ignoring SELECT or SELECT N. The version of the same in ./core/v2/QueryExecutorImpl.java is: protected
Re: [HACKERS] Streaming replication, and walsender during recovery
On Thu, Jan 28, 2010 at 7:43 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: How about restoring/streaming files to a new directory, say pg_xlog/restored/, with the real filenames? At least in standby_mode, probably best to keep the current behavior in PITR. That would feel more clean, you could easily tell apart files originating from the server itself and those copied from the master. When the WAL file with the same name exists in the archive, pg_xlog and pg_xlog/restore/ which directory should we recover it from? I'm not sure that we can always make a right decision about that. How about just making a restore_command copy the WAL files as the normal one (e.g., ...) instead of a pg_xlog/RECOVERYXLOG? Though we need to worry about deleting them, we can easily leave the task to the bgwriter. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION 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] About Our CLUSTER implementation is pessimal patch
Hi all, attached a patch to do seq scan + sorting instead of index scan on CLUSTER (when that's supposed to be faster). As I've already said, the patch is based on: http://archives.postgresql.org/pgsql-hackers/2008-08/msg01371.php Of course, the code isn't supposed to be ready to be merged: I would like to write more comments and add some test cases to cluster.sql (plus change all the things you are going to tell me I have to change...) I would like your opinions on code correctness and the decisions I took, especially: 1) function names (cost_index_scan_vs_seqscansort I guess it's awful...) 2) the fact that I put in Tuplesortstate an EState variable, so that MakeSingleTupleTableSlot wouldn't have to be called for every row in the expression indexes case 3) the expression index case is not optimized: I preferred to call FormIndexDatum once for the first key value in copytup_rawheap and another time to get all the remaining values in comparetup_rawheap. I liked the idea of re-using FormIndexDatum in that case, instead of copyingpasting only the relevant code: but FormIndexDatum returns all the values, even when I might need only the first one 4) I refactored the code to deform and rewrite tuple into the function deform_and_rewrite_tuple, because now that part can be called by the regular index scan or by the new seq-scan + sort (but I could copypaste those lines instead of refactoring them into a new function) Suggestions and comments are not just welcome, but needed! Leonardo sorted_cluster.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make everything target
Andrew Dunstan wrote: Alvaro Herrera wrote: make world sounds reasonable and I've remember seeing it elsewhere. Here's a simple patch. Comments? Should the new targets be added to Makefile too? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make everything target
Alvaro Herrera wrote: Andrew Dunstan wrote: Alvaro Herrera wrote: make world sounds reasonable and I've remember seeing it elsewhere. Here's a simple patch. Comments? Should the new targets be added to Makefile too? Sure, good idea. 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] Review: listagg aggregate
On Thu, Jan 28, 2010 at 3:59 AM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/28 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 David E. Wheeler da...@kineticode.com: On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. no, has not. What is use case for this behave?? I also think this usage is nonsense, but seems to be the most consistent behavior for me. I didn't say anything about use-cases, but just capability. Since we allow such kinds of usage for now, you need to verify the delimiter is not changed rather than ignoring it if you want disallow to change the delimiter during an aggregation. Of course you can cache the first delimiter at start, and check delimiters are not changed every calls -- but I think it is just a waste of cpu cycle. Agreed. Not caching it seems the simplest solution. ...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: listagg aggregate
2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 3:59 AM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/28 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 David E. Wheeler da...@kineticode.com: On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote: * I think we cannot cache the delimiter at the first call. For example, SELECT string_agg(elem, delim) FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim); should return 'A+B*C' rather than 'A,B,C'. no, has not. What is use case for this behave?? I also think this usage is nonsense, but seems to be the most consistent behavior for me. I didn't say anything about use-cases, but just capability. Since we allow such kinds of usage for now, you need to verify the delimiter is not changed rather than ignoring it if you want disallow to change the delimiter during an aggregation. Of course you can cache the first delimiter at start, and check delimiters are not changed every calls -- but I think it is just a waste of cpu cycle. Agreed. Not caching it seems the simplest solution. simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Pavel ...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] can somebody execute this query on Oracle 11.2g and send result?
Hello, I can't to install Oracle, and need to know result. CREATE TABLE foo(a varchar(10), b varchar(10)); INSERT INTO foo VALUES('aaa',','); INSERT INTO foo VALUES('bbb',';'); INSERT INTO foo VALUES('ccc','+'); SELECT listagg(a,b) FROM foo; Thank you Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest status summary 2010-01-27
On Wed, Jan 27, 2010 at 11:13 PM, Greg Smith g...@2ndquadrant.com wrote: Robert Haas wrote: plpython3 - no reviewer yet This whole feature seems quite interesting, and I'm really impressed at how much work James has put into rethinking what a Python PL should look like. But isn't the fact that there isn't even a reviewer for it yet evidence it needs more time to develop a bit of a community first before being considered for core? I agree. I think this needs to live outside of core for the time being. I don't think we can commit to maintaining a second PL/python implementation in core because two or three people are excited about it. It may be really great, and if there are some small changes to core that are needed to support this living outside of core, then I think we should consider those. But committing the whole patch does not seem like a wise idea to me. We are then on the hook to maintain it, essentially forever, and it's not clear that there is enough community support for this for us to be certain of that. If the community around this grows, we can certainly revisit for 9.1. Provide rowcount for utility SELECTs I think I can write a review for this one right now based on the discussion that's already happened: -Multiple people think the feature is valuable and it seems worth exploring -The right way to handle the protocol change here is still not clear -There are any number of subtle ways clients might be impacted by this change, and nailing that down and determining who might break is an extremely wide ranging bit of QA work that's barely been exploring yet That last part screams returned with feedback for something submitted to the last CF before beta to me. As a candidate for 9.1-alpha1 where there's plenty of time to figure out what it breaks and revert if that turns out to be bad? That sounds like a much better time to be fiddling with something in this area. I feel a bit differently about this. No matter when we make a change like this, there will be some risk of breaking clients. Many of those clients may be proprietary, closed-source software, and we have no way of estimating how many such clients there are in total nor what percentage of them are likely to be broken by this change. Looking at a few of the open source clients and trying to judge whether they will break may be worthwhile, but I think the primary thing we need here is a better understanding of exactly which commands this change will affect and some thought about how plausible it is that someone could be depending on those tags. In particular, it seems to me that it's rather unlikely that anyone is depending on the command tag from an operation like CREATE TABLE AS or SELECT INTO, because isn't it always going to be SELECT? Furthermore, if we are going to ever change this in an incompatible way that may break clients, isn't 9.0 exactly the right time to do that? If that doesn't scream big changes coming, proceed with caution, I don't know what would. ...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: listagg aggregate
On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. ...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_perl_init and proper destruction to plperl [PATCH]
Tim Bunce wrote: I've no use-case for that so I'm happy to leave it disabled. If someone does have a sane use-case, please let me know. It can always be enabled later. As I noted upthread, there have been requests for user level session end handlers before. With SPI enabled as Tom suggests, this would just about buy us that for free. But if you're uncomfortable about it we can take that up at a later date. 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] CommitFest status summary 2010-01-27
Robert Haas escribió: Furthermore, if we are going to ever change this in an incompatible way that may break clients, isn't 9.0 exactly the right time to do that? If that doesn't scream big changes coming, proceed with caution, I don't know what would. I agree with this -- if we're ever going to change this, it must be now. -- 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
[HACKERS] plperl compiler warning
Last night I noted the following warning: plperl.c: In function ‘plperl_create_sub’: plperl.c:1117: warning: null argument where non-null required (argument 2) I'm not familiar enough with this code to propose a fix... Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] quoting psql varible as identifier
On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/27 Robert Haas robertmh...@gmail.com: On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I hope, so this version is more readable and more clean. I removed some not necessary checks. This still seems overly complicated to me. I spent a few hours today working up the attached patch. Let me know your thoughts. There is serious issue. The psql_error only shows some message on output, but do nothing more - you don't set a result status for commands and for statements. So some potential error from parsing is pseudo quietly ignored - without respect to your setting ON_ERROR_STOP. This could be a problem for commands. Execution of broken SQL statements will raise syntax error. But for \set some variable will be a broken and the content can be used. I don't thing so it is good. It is limited. Well, what you seem to be proposing to do is allow the command to execute (on the screwed-up data) and then afterwards pretend that it failed by overriding the return status. I think that's unacceptable. The root of the problem here is that the parsing and execution stages for backslash commands are not cleanly separated. There's no clean way for the lexer to return an error that allows the command to finish parsing normally but then doesn't execute it. Fixing that is going to require an extensive refactoring of commands.c which I don't think it makes sense to undertake at this point in the release cycle. Even if it did, it seems like material for a separate patch rather than something which has to be done before this goes in. Your version is acceptable only when we don't enable escape syntax for commands. Then we don't need check it. On your version - I am not sure if it is fully compatible, and using a global variables isn't nice. I'm not adding any new global variables - I'm just using the ones that are already there to avoid duplicating the same code four times. Referencing them from within the bodies of the lexer rules doesn't make the variables not global. ...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] Segmentation fault occurs when the standby becomes primary, in SR
Hi, When I created the trigger file to activate the standby server, I got the segmentation fault: sby [11342]: LOG: trigger file found: ../trigger sby [11343]: FATAL: terminating walreceiver process due to administrator command sby [11342]: LOG: redo done at 0/1E0 sby [11342]: LOG: last completed transaction was at log time 2000-01-01 09:21:04.685861+09 sby [11341]: LOG: startup process (PID 11342) was terminated by signal 11: Segmentation fault sby [11341]: LOG: terminating any other active server processes This happens in the following scenario: 0. The trigger file is found. 1. The variable StandbyMode is reset to FALSE before re-fetching the last applied record. 2. That record attempts to be read from the archive. 3. RestoreArchivedFile() goes through the following condition expression because the StandbyMode is off. if (StandbyMode recoveryRestoreCommand == NULL) goto not_available; 4. RestoreArchivedFile() wrongly constructs the command to be executed even though restore_command has not been supplied (this is possible in standby mode). --- Segmentation fault! The attached patch would fix the bug. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center *** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *** *** 2759,2766 RestoreArchivedFile(char *path, const char *xlogfname, uint32 restartLog; uint32 restartSeg; ! /* In standby mode, restore_command might not be supplied */ ! if (StandbyMode recoveryRestoreCommand == NULL) goto not_available; /* --- 2759,2769 uint32 restartLog; uint32 restartSeg; ! /* ! * Returns FALSE if restore_command has not been supplied. This is ! * possible in standby mode. ! */ ! if (recoveryRestoreCommand == NULL) goto not_available; /* -- 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] plperl compiler warning
On Thu, Jan 28, 2010 at 06:31:19AM -0800, Joe Conway wrote: Last night I noted the following warning: plperl.c: In function ‘plperl_create_sub’: plperl.c:1117: warning: null argument where non-null required (argument 2) The master branch of my git clone says line 1117 is: subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); Does that match yours? (If not, what is the text on the line?) What perl version are you using? What compiler version are you using? 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] Largeobject Access Controls (r2460)
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: When I'm testing the new patch, I found ALTER LARGE OBJECT command returns ALTER LARGEOBJECT tag. Should it be ALTER LARGE(space)OBJECT instead? As I remember, we had decided not to use LARGEOBJECT (without a space) in user-visible messages, right? The command tag should match the actual command. If the command name is ALTER LARGE OBJECT, the command tag should be too. This is independent of phraseology we might choose in error messages (though I agree I don't like largeobject in those either). 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: listagg aggregate
2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. It is only a few lines with zero complexity. The main issue of Takahiro proposal is unclean behave. we can have a content c1c2 --- c11, c12, c21, c22 and result of string_agg(c1, c2) have to be ?? c11 c12 c21 or c11 c22 c21 ?? What if some content of c2 will be NULL ?? I checked oracle. Oracle doesn't allow variable as delimiter. We can't check it. But we can fix first value and using it as constant. More - Takahiro proposal has one performance gain. It have to deTOAST separator value in every cycle. Takahiro has true - we can store length of separator and we can add separator to cumulative value as binary value. I prefer original behave with note in documentation - so as separator is used a value on first row, when is used expression and not constant. Regards Pavel ...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: Typed Table
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: ISTM you should explicitly grab a lock on the of-type at some point, to make sure it doesn't get dropped while you're busy creating the table. How do we protect against that for the types used in columns? We don't. There is no concept of a lock on a type. For scalar types this is more or less irrelevant anyway, since a scalar has no substructure that can be altered in any interesting way. I'm not sure how hard we ought to work on making composites behave differently. I think it's as likely to cause problems as solve them. 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_perl_init and proper destruction to plperl [PATCH]
Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 06:27:50PM -0500, Tom Lane wrote: An advantage of on_proc_exit from your end is that it should allow you to not have to try to prevent the END blocks from using SPI, as that would still be perfectly functional when your callback gets called. (Starting a new transaction would be a good idea though, cf Async_UnlistenOnExit.) I'm surprised that you're suggesting that END block should be allowed to interact with the backend via SPI. It seems to go against what you've said previously about code running at shutdown. I think you have completely misunderstood what I'm complaining about. What I'm not happy about is executing operations at a point where they're likely to be ill-defined because the code is in the wrong state. In an early on_proc_exit hook, the system is for all practical purposes still fully functional, and so I don't see a reason for an arbitrary restriction on what the END blocks should be able to do. (Or, to repeat myself in a different way: the no-SPI restriction is utterly useless to guard against my real concerns anyway. I see no point in it either here or elsewhere.) 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
[HACKERS] Add on_perl_init and proper destruction to plperl UPDATED [PATCH]
This is an updated version of the third of the patches to be split out from the former 'plperl feature patch 1'. It includes changes following discussions with Tom Lane and others. Changes in this patch: - Added plperl.on_perl_init GUC for DBA use (PGC_SIGHUP) SPI functions are not available when the code is run. - Added interpreter destruction behaviour Hooked via on_proc_exit(). Only has any effect for normal shutdown. END blocks, if any, are run then objects are destroyed, calling their DESTROY methods, if any. SPI functions will die if called at this time. Tim. diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 5fa7e3a..06c63df 100644 *** a/doc/src/sgml/plperl.sgml --- b/doc/src/sgml/plperl.sgml *** CREATE TRIGGER test_valid_id_trig *** 1028,1034 /para /sect1 ! sect1 id=plperl-missing titleLimitations and Missing Features/title para --- 1028,1098 /para /sect1 ! sect1 id=plperl-under-the-hood ! titlePL/Perl Under the Hood/title ! ! sect2 id=plperl-config ! titleConfiguration/title ! ! para ! This section lists configuration parameters that affect applicationPL/Perl/. ! To set any of these parameters before applicationPL/Perl/ has been loaded, ! it is necessary to have added quoteliteralplperl// to the ! xref linkend=guc-custom-variable-classes list in ! filenamepostgresql.conf/filename. ! /para ! ! variablelist ! ! varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init ! termvarnameplperl.on_perl_init/varname (typestring/type)/term ! indexterm !primaryvarnameplperl.on_perl_init/ configuration parameter/primary ! /indexterm ! listitem !para !Specifies perl code to be executed when a perl interpreter is first initialized. !The SPI functions are not available when this code is executed. !If the code fails with an error it will abort the initialization of the interpreter !and propagate out to the calling query, causing the current transaction !or subtransaction to be aborted. !/para !para ! The perl code is limited to a single string. Longer code can be placed ! into a module and loaded by the literalon_perl_init/ string. ! Examples: ! programlisting ! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl' ! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;' ! /programlisting !/para !para !Initialization will happen in the postmaster if the plperl library is included !in literalshared_preload_libraries/ (see xref linkend=shared_preload_libraries), !in which case extra consideration should be given to the risk of destabilizing the postmaster. !/para !para !This parameter can only be set in the postgresql.conf file or on the server command line. !/para ! /listitem ! /varlistentry ! ! varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict ! termvarnameplperl.use_strict/varname (typeboolean/type)/term ! indexterm !primaryvarnameplperl.use_strict/ configuration parameter/primary ! /indexterm ! listitem !para !When set true subsequent compilations of PL/Perl functions have the literalstrict/ pragma enabled. !This parameter does not affect functions already compiled in the current session. !/para ! /listitem ! /varlistentry ! ! /variablelist ! ! sect2 id=plperl-missing titleLimitations and Missing Features/title para *** CREATE TRIGGER test_valid_id_trig *** 1067,1072 --- 1131,1138 /listitem /itemizedlist /para + /sect2 + /sect1 /chapter diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl index 24e2487..5d2e962 100644 *** a/src/pl/plperl/plc_perlboot.pl --- b/src/pl/plperl/plc_perlboot.pl *** *** 2,8 # $PostgreSQL$ PostgreSQL::InServer::Util::bootstrap(); - PostgreSQL::InServer::SPI::bootstrap(); use strict; use warnings; --- 2,7 diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 9277072..2202b0f 100644 *** a/src/pl/plperl/plperl.c --- b/src/pl/plperl/plperl.c *** *** 27,32 --- 27,33 #include miscadmin.h #include nodes/makefuncs.h #include parser/parse_type.h + #include storage/ipc.h #include utils/builtins.h #include utils/fmgroids.h #include utils/guc.h *** static HTAB *plperl_proc_hash = NULL; *** 138,143 --- 139,146 static HTAB *plperl_query_hash = NULL; static bool plperl_use_strict = false; + static char *plperl_on_perl_init = NULL; + static bool plperl_ending = false; /* this is saved and restored by plperl_call_handler */ static plperl_call_data *current_call_data = NULL; *** Datum plperl_validator(PG_FUNCTION_ARGS *** 151,156
Re: [HACKERS] [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
2010/1/27 KaiGai Kohei kai...@ak.jp.nec.com: Hmm, indeed, this logic (V3/V5) is busted. The idea of V4 patch can also handle this case correctly, although it is lesser in performance. I wonder whether it is really unacceptable cost in performance, or not. Basically, I assume ALTER TABLE RENAME/TYPE is not frequent operations, and I don't think this bugfix will damage to the reputation of PostgreSQL. Where should we go on the next? Isn't the problem here just that the following comment is 100% wrong? /* * Unlike find_all_inheritors(), we need to walk on child relations * that have diamond inheritance tree, because this function has to * return correct expected inhecount to the caller. */ It seems to me that the right solution here is to just add one more argument to find_all_inheritors(), something like List **expected_inh_count. Am I missing something? ...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] Streaming replication, and walsender during recovery
Fujii Masao masao.fu...@gmail.com writes: How about just making a restore_command copy the WAL files as the normal one (e.g., ...) instead of a pg_xlog/RECOVERYXLOG? Though we need to worry about deleting them, we can easily leave the task to the bgwriter. The reason for doing it that way was to limit disk space usage during a long restore. I'm not convinced we can leave the task to the bgwriter --- it shouldn't be deleting anything at that point. 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: listagg aggregate
Robert Haas robertmh...@gmail.com writes: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. Yeah. The real issue here is that in some cases you'd like to have non-aggregated parameters to an aggregate, but SQL has no notation to express that. I think Pavel's underlying complaint is that if the delimiter argument isn't constant, then we're exposing an implementation dependency in terms of just which values get separated by which delimiters. The most practical implementation seems to be that the first-call delimiter isn't actually used at all, and on subsequent calls the delimiter *precedes* the associated value, which is a bit surprising given the order in which one writes them. Not sure if this is worth documenting though. Those two or three people who actually try it will figure it out soon enough. 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: listagg aggregate
On Thu, Jan 28, 2010 at 10:56 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. Yeah. The real issue here is that in some cases you'd like to have non-aggregated parameters to an aggregate, but SQL has no notation to express that. Right. I think Pavel's underlying complaint is that if the delimiter argument isn't constant, then we're exposing an implementation dependency in terms of just which values get separated by which delimiters. The most practical implementation seems to be that the first-call delimiter isn't actually used at all, and on subsequent calls the delimiter *precedes* the associated value, which is a bit surprising given the order in which one writes them. Not sure if this is worth documenting though. Those two or three people who actually try it will figure it out soon enough. Yeah, I'm thoroughly unworried about 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] Review: listagg aggregate
On Wed, Jan 27, 2010 at 9:47 PM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: * Do we need better names for string_agg1_transfn and string_agg2_transfn? They are almost internal names, but we could have more like string_agg_with_sep_transfn. Surely the names of the transition and final functions should match the name of the aggregate. But if there's a proposal on the table to call this string_app_with_sep() instead of just string_agg(), +1 from me. ...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_perl_init and proper destruction to plperl [PATCH]
On Thu, Jan 28, 2010 at 10:39:33AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: On Wed, Jan 27, 2010 at 06:27:50PM -0500, Tom Lane wrote: An advantage of on_proc_exit from your end is that it should allow you to not have to try to prevent the END blocks from using SPI, as that would still be perfectly functional when your callback gets called. (Starting a new transaction would be a good idea though, cf Async_UnlistenOnExit.) I'm surprised that you're suggesting that END block should be allowed to interact with the backend via SPI. It seems to go against what you've said previously about code running at shutdown. I think you have completely misunderstood what I'm complaining about. What I'm not happy about is executing operations at a point where they're likely to be ill-defined because the code is in the wrong state. In an early on_proc_exit hook, the system is for all practical purposes still fully functional, and so I don't see a reason for an arbitrary restriction on what the END blocks should be able to do. Ah, okay. I guess I missed your underlying concerns in: http://archives.postgresql.org/message-id/26766.1263149...@sss.pgh.pa.us For the record, [...] and I think it's a worse idea to run arbitrary user-defined code at backend shutdown (the END-blocks bit). (Or, to repeat myself in a different way: the no-SPI restriction is utterly useless to guard against my real concerns anyway. I see no point in it either here or elsewhere.) I've left it in the updated patch I've just posted. There are two more plperl patches in the current commitfest that I'd like to chaperone through to commit (in some form or other) first. Thanks for your help Tom. 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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 10:48 -0500, Tom Lane wrote: Fujii Masao masao.fu...@gmail.com writes: How about just making a restore_command copy the WAL files as the normal one (e.g., ...) instead of a pg_xlog/RECOVERYXLOG? Though we need to worry about deleting them, we can easily leave the task to the bgwriter. The reason for doing it that way was to limit disk space usage during a long restore. I'm not convinced we can leave the task to the bgwriter --- it shouldn't be deleting anything at that point. I think bgwriter means RemoveOldXlogFiles(), which would normally clear down files at checkpoint. If that was added to the end of RecoveryRestartPoint() to do roughly the same job then it could potentially work. However, since not every checkpoint is a restartpoint we might easily end up with significantly more WAL files on the standby than would normally be there when it would be a primary. Not sure if that is an issue in this case, but we can't just assume we can store all files needed to restart the standby on the standby itself, in all cases. That might be an argument to add a restartpoint_segments parameter, so we can trigger restartpoints on WAL volume as well as time. But even that would not put an absolute limit on the number of WAL files. I'm keen to allow cascading in 9.0. If you pull both synch rep and cascading you're not offering much that isn't already there. -- 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] plperl compiler warning
On 01/28/2010 07:30 AM, Tim Bunce wrote: On Thu, Jan 28, 2010 at 06:31:19AM -0800, Joe Conway wrote: Last night I noted the following warning: plperl.c: In function ‘plperl_create_sub’: plperl.c:1117: warning: null argument where non-null required (argument 2) The master branch of my git clone says line 1117 is: subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); Does that match yours? (If not, what is the text on the line?) I pull directly from CVS, not git, but in any case my line 1117 is subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); so it appears to be the same What perl version are you using? What compiler version are you using? I'm on stock Fedora 12: perl.x86_64 4:5.10.0-87.fc12 gcc.x86_64 4.4.2-20.fc12 HTH, Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Now the dust is settling on the on_perl_init patch I'd like to ask for clarification on this next patch. On Fri, Jan 15, 2010 at 12:35:06AM +, Tim Bunce wrote: This is the fourth of the patches to be split out from the former 'plperl feature patch 1'. Changes in this patch: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. Would changing plperl.on_trusted_init and plperl.on_untrusted_init to PGC_BACKEND, so the user can't change the value after the session has started, resolve those concerns? Any other concerns with this patch? Tim. - select_perl_context() state management improved An error during interpreter initialization will leave the state (interp_state etc) unchanged. - The utf8fix code has been greatly simplified. Tim. diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 0054f5a..f2c91a9 100644 *** a/doc/src/sgml/plperl.sgml --- b/doc/src/sgml/plperl.sgml *** plplerl.on_perl_init = 'use lib /my/app *** 1079,1084 --- 1079,1120 /listitem /varlistentry + varlistentry id=guc-plperl-on-trusted-init xreflabel=plperl.on_trusted_init + termvarnameplperl.on_trusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_trusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperl/ perl interpreter +is first initialized in a session. The perl code can only perform trusted operations. +The SPI functions are not available when this code is executed. +Changes made after a literalplperl/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + + varlistentry id=guc-plperl-on-untrusted-init xreflabel=plperl.on_untrusted_init + termvarnameplperl.on_untrusted_init/varname (typestring/type)/term + indexterm +primaryvarnameplperl.on_untrusted_init/ configuration parameter/primary + /indexterm + listitem +para +Specifies perl code to be executed when the literalplperlu/ perl interpreter +is first initialized in a session. +The SPI functions are not available when this code is executed. +Changes made after a literalplperlu/ perl interpreter has been initialized will have no effect. +If the code fails with an error it will abort the initialization of the interpreter +and propagate out to the calling query, causing the current transaction +or subtransaction to be aborted. +/para + /listitem + /varlistentry + varlistentry id=guc-plperl-use-strict xreflabel=plperl.use_strict termvarnameplperl.use_strict/varname (typeboolean/type)/term indexterm diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 7cd5721..f3cabad 100644 *** a/src/pl/plperl/GNUmakefile --- b/src/pl/plperl/GNUmakefile *** PERLCHUNKS = plc_perlboot.pl plc_safe_ba *** 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) --- 41,47 SHLIB_LINK = $(perl_embed_ldflags) REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu ! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_util plperl_init plperlu # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out index ...e69de29 . diff --git a/src/pl/plperl/expected/plperl_shared.out b/src/pl/plperl/expected/plperl_shared.out index 72ae1ba..c1c12c1 100644 *** a/src/pl/plperl/expected/plperl_shared.out --- b/src/pl/plperl/expected/plperl_shared.out *** *** 1,3 --- 1,7 + -- test plperl.on_plperl_init via the shared hash + -- (must be done before plperl is initialized) + -- testing on_trusted_init gets run, and that it can alter %_SHARED + SET
Re: [HACKERS] Streaming replication, and walsender during recovery
Simon Riggs si...@2ndquadrant.com writes: I'm keen to allow cascading in 9.0. If you pull both synch rep and cascading you're not offering much that isn't already there. FWIW, I don't agree with that prioritization in the least. Cascading is something we could leave till 9.1, or even later, and hardly anyone would care. We have much more important problems to be spending our effort on right 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 [PATCH]
Tim Bunce tim.bu...@pobox.com writes: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. IIRC, what I was unhappy about was exposing shared library load as a time when interesting-to-the-user things would happen. It looks like you have got rid of that and instead made it happen at the first call of a plperl or plperlu function (respectively). That seems like a fairly reasonable way to work, and if it's defined that way, there doesn't seem any reason not to allow them to be USERSET/SUSET. But it ought to be documented like that, not with vague phrases like when the interpreter is initialized --- that means nothing to users. One issue that ought to be mentioned is what about transaction rollback. One could argue that if the first plperl call is inside a transaction that later rolls back, then all the side effects of that should be undone. But we have no way to undo whatever happened inside perl, and at least in most likely uses of on_init there wouldn't be any advantage in trying to force a redo anyway. I think it's okay to just define it as happening once regardless of any transaction failure (but of course this had better be documented). 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] quoting psql varible as identifier
2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/27 Robert Haas robertmh...@gmail.com: On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I hope, so this version is more readable and more clean. I removed some not necessary checks. This still seems overly complicated to me. I spent a few hours today working up the attached patch. Let me know your thoughts. There is serious issue. The psql_error only shows some message on output, but do nothing more - you don't set a result status for commands and for statements. So some potential error from parsing is pseudo quietly ignored - without respect to your setting ON_ERROR_STOP. This could be a problem for commands. Execution of broken SQL statements will raise syntax error. But for \set some variable will be a broken and the content can be used. I don't thing so it is good. It is limited. Well, what you seem to be proposing to do is allow the command to execute (on the screwed-up data) and then afterwards pretend that it failed by overriding the return status. I think that's unacceptable. The root of the problem here is that the parsing and execution stages for backslash commands are not cleanly separated. There's no clean way for the lexer to return an error that allows the command to finish parsing normally but then doesn't execute it. Fixing that is going to require an extensive refactoring of commands.c which I don't think it makes sense to undertake at this point in the release cycle. Even if it did, it seems like material for a separate patch rather than something which has to be done before this goes in. so I removed support for escaping from backslah commands and refactorised code. I hope so this code is more verbose and clean than previous versions. Regards Pavel Your version is acceptable only when we don't enable escape syntax for commands. Then we don't need check it. On your version - I am not sure if it is fully compatible, and using a global variables isn't nice. I'm not adding any new global variables - I'm just using the ones that are already there to avoid duplicating the same code four times. Referencing them from within the bodies of the lexer rules doesn't make the variables not global. ...Robert *** ./doc/src/sgml/ref/psql-ref.sgml.orig 2009-12-25 00:36:39.0 +0100 --- ./doc/src/sgml/ref/psql-ref.sgml 2010-01-28 16:57:15.016331154 +0100 *** *** 658,664 para If an unquoted argument begins with a colon (literal:/literal), it is taken as a applicationpsql/ variable and the value of the ! variable is used as the argument instead. /para para --- 658,669 para If an unquoted argument begins with a colon (literal:/literal), it is taken as a applicationpsql/ variable and the value of the ! variable is used as the argument instead. If the variable name is ! surrounded by single quotes (e.g. literal:'var'/literal), it ! will be escaped as an SQL literal and the result will be used as ! the argument. If the variable name is surrounded by double quotes, ! it will be escaped as an SQL identifier and the result will be used ! as the argument. /para para *** *** 2711,2728 para An additional useful feature of applicationpsql/application variables is that you can substitute (quoteinterpolate/quote) ! them into regular acronymSQL/acronym statements. The syntax for ! this is again to prepend the variable name with a colon (literal:/literal): programlisting testdb=gt; userinput\set foo 'my_table'/userinput testdb=gt; userinputSELECT * FROM :foo;/userinput /programlisting ! would then query the table literalmy_table/literal. The value of ! the variable is copied literally, so it can even contain unbalanced ! quotes or backslash commands. You must make sure that it makes sense ! where you put it. Variable interpolation will not be performed into ! quoted acronymSQL/acronym entities. /para para --- 2716,2750 para An additional useful feature of applicationpsql/application variables is that you can substitute (quoteinterpolate/quote) ! them into regular acronymSQL/acronym statements. ! applicationpsql/application provides special facilities for ! ensuring that values used as SQL literals and identifiers are ! properly escaped. The syntax for interpolating a value without ! any special escaping is again to prepend the variable name with a colon (literal:/literal): programlisting testdb=gt; userinput\set foo 'my_table'/userinput testdb=gt; userinputSELECT * FROM :foo;/userinput /programlisting ! would then query the table literalmy_table/literal. Note that this ! may be unsafe: the value of the variable is copied literally, so
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. 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] Review: listagg aggregate
2010/1/29 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. It is only a few lines with zero complexity. The main issue of Takahiro proposal is unclean behave. we can have a content c1 c2 --- c11, c12, c21, c22 and result of string_agg(c1, c2) have to be ?? c11 c12 c21 or c11 c22 c21 ?? What if some content of c2 will be NULL ?? I checked oracle. Oracle doesn't allow variable as delimiter. We can't check it. But we can fix first value and using it as constant. What about get_fn_expr_arg_stable() to check if the argument is stable during aggregate? Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication, and walsender during recovery
On Thu, Jan 28, 2010 at 11:41 AM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: I'm keen to allow cascading in 9.0. If you pull both synch rep and cascading you're not offering much that isn't already there. FWIW, I don't agree with that prioritization in the least. Cascading is something we could leave till 9.1, or even later, and hardly anyone would care. We have much more important problems to be spending our effort on right now. I agree. According to http://wiki.postgresql.org/wiki/Hot_Standby_TODO , the only must-fix issues that remain prior to beta are (1) implementing the new VACUUM FULL for system relations, and (2) some documentation improvements. It's a little early to be worrying about docs, but shouldn't we be trying to get the VACUUM FULL problems cleaned up first, and then look at what else we have time to address? As regards the remaining items for streaming replication at: http://wiki.postgresql.org/wiki/Streaming_Replication#v9.0 ...ISTM the most important issues are (1) fixing win32 and (2) adding a message type header, followed by (3) fixing pg_xlogfile_name() and (4) redefining smart shutdown in standby mode. If we fix the must-fix issues first, we can still decide to delay the release to fix the would-like-to-fix issues, or not. The other way around doesn't work. ...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: listagg aggregate
2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 10:56 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. Yeah. The real issue here is that in some cases you'd like to have non-aggregated parameters to an aggregate, but SQL has no notation to express that. Right. I think Pavel's underlying complaint is that if the delimiter argument isn't constant, then we're exposing an implementation dependency in terms of just which values get separated by which delimiters. The most practical implementation seems to be that the first-call delimiter isn't actually used at all, and on subsequent calls the delimiter *precedes* the associated value, which is a bit surprising given the order in which one writes them. Not sure if this is worth documenting though. Those two or three people who actually try it will figure it out soon enough. ok - there is one query, in 99.99% the second argument will be a constant. Can we use this information and optimize function for this case? The detoast on every row can take some percent from a performance. Pavel Yeah, I'm thoroughly unworried about 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] Review: listagg aggregate
2010/1/28 Robert Haas robertmh...@gmail.com: On Wed, Jan 27, 2010 at 9:47 PM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: * Do we need better names for string_agg1_transfn and string_agg2_transfn? They are almost internal names, but we could have more like string_agg_with_sep_transfn. Surely the names of the transition and final functions should match the name of the aggregate. But if there's a proposal on the table to call this string_app_with_sep() instead of just string_agg(), +1 from me. no, string_app_with_sep is too long for common using. Pavel ...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: listagg aggregate
Pavel Stehule pavel.steh...@gmail.com writes: in 99.99% the second argument will be a constant. Can we use this information and optimize function for this case? The detoast on every row can take some percent from a performance. What detoast? There won't be one for a constant, nor even for a variable in any sane situation --- who's going to be using multi-kilobyte delimiter values? And if they do, aren't they likely to run out of memory for the result long before the repeated detoasts become an interesting problem? You're arguing about a case that seems quite irrelevant to the real world. 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: listagg aggregate
2010/1/28 Hitoshi Harada umi.tan...@gmail.com: 2010/1/29 Pavel Stehule pavel.steh...@gmail.com: 2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel.steh...@gmail.com wrote: simplest could not be a best. There have to be only a const expression. But we have not possibility to check it in pg. Well... that's an entirely arbitrary limitation. I admit that it doesn't seem likely that someone would want to have a variable delimiter, but putting extra effort and code complexity into preventing it seems pointless. It is only a few lines with zero complexity. The main issue of Takahiro proposal is unclean behave. we can have a content c1 c2 --- c11, c12, c21, c22 and result of string_agg(c1, c2) have to be ?? c11 c12 c21 or c11 c22 c21 ?? What if some content of c2 will be NULL ?? I checked oracle. Oracle doesn't allow variable as delimiter. We can't check it. But we can fix first value and using it as constant. What about get_fn_expr_arg_stable() to check if the argument is stable during aggregate? I newer know so this function exists. Now we can a) check and allow only stable params b) when second parameter is stable, then store it and use it as constant. I prefer a) Pavel Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. Right, sorry, got 'em backwards. 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: listagg aggregate
2010/1/28 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: in 99.99% the second argument will be a constant. Can we use this information and optimize function for this case? The detoast on every row can take some percent from a performance. What detoast? There won't be one for a constant, nor even for a variable in any sane situation --- who's going to be using multi-kilobyte delimiter values? And if they do, aren't they likely to run out of memory for the result long before the repeated detoasts become an interesting problem? You're arguing about a case that seems quite irrelevant to the real world. I asking with get_fn_expr_arg_stable() we are able to fix second parameter without some performance issues. Regards Pavel 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: listagg aggregate
Hitoshi Harada umi.tan...@gmail.com writes: What about get_fn_expr_arg_stable() to check if the argument is stable during aggregate? Seems quite expensive (possible catalog lookups) and there still isn't any strong argument why we should bother. 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: listagg aggregate
Pavel Stehule pavel.steh...@gmail.com writes: with get_fn_expr_arg_stable() we are able to fix second parameter without some performance issues. No, that will create its own performance issues --- get_fn_expr_arg_stable isn't especially cheap. If there were a really strong reason why we had to do it, then I'd agree, but frankly the argument for disallowing a variable delimiter is too thin. 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: listagg aggregate
On 2010-01-28 19:17, Pavel Stehule wrote: 2010/1/28 Hitoshi Harada umi.tan...@gmail.com: What about get_fn_expr_arg_stable() to check if the argument is stable during aggregate? I newer know so this function exists. Now we can a) check and allow only stable params b) when second parameter is stable, then store it and use it as constant. I prefer a) Someone might have a perfectly good use case for using different delimiters. I don't think it's a good idea to be artificially limiting what you can and can't do. 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: listagg aggregate
On Jan 28, 2010, at 9:29 AM, Marko Tiikkaja wrote: Someone might have a perfectly good use case for using different delimiters. I don't think it's a good idea to be artificially limiting what you can and can't do. +1 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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 12:09 -0500, Robert Haas wrote: I agree. According to http://wiki.postgresql.org/wiki/Hot_Standby_TODO , the only must-fix issues that remain prior to beta are (1) implementing the new VACUUM FULL for system relations, and (2) some documentation improvements. It's a little early to be worrying about docs, but shouldn't we be trying to get the VACUUM FULL problems cleaned up first, and then look at what else we have time to address? Please don't confuse different issues. The fact that I have work to do still is irrelevant to what other people should do on other features. -- 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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 11:41 -0500, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: I'm keen to allow cascading in 9.0. If you pull both synch rep and cascading you're not offering much that isn't already there. FWIW, I don't agree with that prioritization in the least. Cascading is something we could leave till 9.1, or even later, and Not what you said just a few days ago. hardly anyone would care. Unfortunately, I think you're very wrong on that specific point. We have much more important problems to be spending our effort on right now. I'm a little worried the feature set of streaming rep isn't any better than what we have already. If we're going to destabilise the code, we really should be adding some features as well. -- 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] plperl compiler warning
Joe Conway m...@joeconway.com writes: I pull directly from CVS, not git, but in any case my line 1117 is subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); so it appears to be the same What perl version are you using? What compiler version are you using? I'm on stock Fedora 12: I see the same on Fedora 11. The -E expansion of the line in question is subref = Perl_newRV(((PerlInterpreter *)pthread_getspecific((*Perl_Gthr_key_ptr(((void *)0), (SV*)GV*)sub_glob)-sv_u.svu_gp)-gp_cvgen ? ((void *)0) : (((GV*)sub_glob)-sv_u.svu_gp)-gp_cv)); so it's evidently unhappy about the fact that GvCVu can return null, while Perl_newRV is declared __attribute__((nonnull(2))). It looks to me like this is probably a live bug not just compiler hypersensitivity. 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 walsender during recovery
Simon Riggs si...@2ndquadrant.com writes: On Thu, 2010-01-28 at 11:41 -0500, Tom Lane wrote: FWIW, I don't agree with that prioritization in the least. Cascading is something we could leave till 9.1, or even later, and Not what you said just a few days ago. Me? I don't recall having said a word about cascading before. I'm a little worried the feature set of streaming rep isn't any better than what we have already. Nonsense. Getting rid of the WAL-segment-based shipping delays is a quantum improvement --- it means we actually have something approaching real-time replication, which was really impractical before. Whether you can feed slaves indirectly is just a minor administration detail. Yeah, I know in some situations it could be helpful for performance, but it's not even in the same ballpark of must-have-ness. (Anyway, the argument that it's important for performance is pure speculation AFAIK, untainted by any actual measurements. Given the lack of optimization of WAL replay, it seems entirely possible that the last thing you want to burden a slave with is sourcing data to more slaves.) 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] quoting psql varible as identifier
On Thu, Jan 28, 2010 at 11:59 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/28 Robert Haas robertmh...@gmail.com: On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2010/1/27 Robert Haas robertmh...@gmail.com: On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I hope, so this version is more readable and more clean. I removed some not necessary checks. This still seems overly complicated to me. I spent a few hours today working up the attached patch. Let me know your thoughts. There is serious issue. The psql_error only shows some message on output, but do nothing more - you don't set a result status for commands and for statements. So some potential error from parsing is pseudo quietly ignored - without respect to your setting ON_ERROR_STOP. This could be a problem for commands. Execution of broken SQL statements will raise syntax error. But for \set some variable will be a broken and the content can be used. I don't thing so it is good. It is limited. Well, what you seem to be proposing to do is allow the command to execute (on the screwed-up data) and then afterwards pretend that it failed by overriding the return status. I think that's unacceptable. The root of the problem here is that the parsing and execution stages for backslash commands are not cleanly separated. There's no clean way for the lexer to return an error that allows the command to finish parsing normally but then doesn't execute it. Fixing that is going to require an extensive refactoring of commands.c which I don't think it makes sense to undertake at this point in the release cycle. Even if it did, it seems like material for a separate patch rather than something which has to be done before this goes in. so I removed support for escaping from backslah commands and refactorised code. I hope so this code is more verbose and clean than previous versions. I don't see any advantage of this over the code that I wrote. First, you can't just remove support for the escape syntax from \d commands without some discussion of whether or not that's the right thing to do, and I don't think it is. The cases where this will potentially cause a problem are limited to those where the input is invalidly encoded, and I don't think that's important enough to justify the surprise factor of having backslash commands behave differently from everything else. Second, even if it were OK to remove support for the escape syntax from \d commands, you failed to update the documentation you cribbed from my patch to match the behavior you implemented. Third, you've reintroduced all of the code duplication that I eliminated in my version of this patch, as well as at least one bug - you've used free() where I believe you need PQfreemem(). I am also thinking that it doesn't make sense to push the result of PQescapeLiteral() or PQescapeIdentifier() as a new buffer, because we don't want to process variable expansions recursively. At first I thought this was a security hole, but on further reflection I don't think it is: it'll rescan as a quoted string anyway, so any colon-escapes will be ignored. But I believe it's unnecessary at any rate. I would like to go ahead and commit my version of this patch and will do so later today if no one else objects. ...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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 13:05 -0500, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: On Thu, 2010-01-28 at 11:41 -0500, Tom Lane wrote: FWIW, I don't agree with that prioritization in the least. Cascading is something we could leave till 9.1, or even later, and Not what you said just a few days ago. Me? I don't recall having said a word about cascading before. Top of this thread. I'm a little worried the feature set of streaming rep isn't any better than what we have already. Nonsense. Getting rid of the WAL-segment-based shipping delays is a quantum improvement --- it means we actually have something approaching real-time replication, which was really impractical before. Whether you can feed slaves indirectly is just a minor administration detail. Yeah, I know in some situations it could be helpful for performance, but it's not even in the same ballpark of must-have-ness. FWIW, streaming has been possible and actively used since 8.2. (Anyway, the argument that it's important for performance is pure speculation AFAIK, untainted by any actual measurements. Given the lack of optimization of WAL replay, it seems entirely possible that the last thing you want to burden a slave with is sourcing data to more slaves.) Separate processes, separate CPUs, no problem. If WAL replay used more CPUs you might be right, but it doesn't yet, so same argument opposite conclusion. -- 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] Streaming replication, and walsender during recovery
Tom Lane wrote: (Anyway, the argument that it's important for performance is pure speculation AFAIK, untainted by any actual measurements. Given the lack of optimization of WAL replay, it seems entirely possible that the last thing you want to burden a slave with is sourcing data to more slaves.) On any typical production hardware, the work of WAL replay is going to leave at least one (and probably more) CPUs idle, and have plenty of network resources to spare too because it's just shuffling WAL in/out rather than dealing with so many complicated client conversations. And the thing you want to redistribute--the WAL file--is practically guaranteed to be sitting in the OS cache at the point where you'd be doing it, so no disk use either. You'll disrupt a little bit of memory/CPU cache, sure, but that's about it as far as leeching resources from the main replay in order to support the secondary slave. I'll measure it fully the next time I have one setup to give some hard numbers, I've never seen it rise to the point where it was worth worrying about before to bother. Anyway, I think what Simon was trying to suggest was that it's possible right now to ship partial WAL files over as they advance, if you monitor pg_xlogfile_name_offset and are willing to coordinate copying chunks over. That basic idea is even built already--the Skytools walmgr deals with partial WALs for example. Having all that built-into the server with a nicer UI is awesome, but it's been possible to build something with the same basic feature set since 8.2. Getting that going with a chain of downstreams slaves is not so easy though, so there's something that I think would be unique to the 9.0 implementation. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming replication, and walsender during recovery
Tom Lane wrote: Fujii Masao masao.fu...@gmail.com writes: How about just making a restore_command copy the WAL files as the normal one (e.g., ...) instead of a pg_xlog/RECOVERYXLOG? Though we need to worry about deleting them, we can easily leave the task to the bgwriter. The reason for doing it that way was to limit disk space usage during a long restore. I'm not convinced we can leave the task to the bgwriter --- it shouldn't be deleting anything at that point. That has been changed already. In standby mode, bgwriter does delete old WAL files when it performs a restartpoint. Otherwise the streamed WAL files will keep accumulating and eventually fill the disk. It works as it is, but having a sandbox dedicated for restored/streamed files in pg_xlog/restored, instead of messing with pg_xlog directly, would make me feel a bit easier about it. There's less potential for damage in case of bugs if they're separate. -- 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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 13:05 -0500, Tom Lane wrote: I'm a little worried the feature set of streaming rep isn't any better than what we have already. Nonsense. Getting rid of the WAL-segment-based shipping delays is a quantum improvement --- it means we actually have something approaching real-time replication, which was really impractical before. SR does not give us anything like replication. Replication implies an ability to read from the Slave. That is HS only territory. From what I read on the wiki SR doesn't give us anything that PostgreSQL + PITRTools doesn't already give us. And PITR Tools works as far back as 8.1 (although I would suggest 8.2+). One thing I am unclear on, is if with SR the entire log must be written before it streams to the slaves. If the entire log does not need to be written, then that is one up on PITRTools in that we have to wait for archive_command to execute. (Anyway, the argument that it's important for performance is pure speculation AFAIK, untainted by any actual measurements. Given the lack of optimization of WAL replay, it seems entirely possible that the last thing you want to burden a slave with is sourcing data to more slaves.) I agree. WAL replay as a whole is a bottlekneck. As it stands now (I don't know about 8.5), replay is a large bottleneck on keeping the warm-standby up to date. Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- 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 walsender during recovery
Simon Riggs wrote: I'm a little worried the feature set of streaming rep isn't any better than what we have already. Huh? Are you thinking of the Record-based Log Shipping described in the manual, using a program to poll pg_xlogfile_name_offset() in a tight loop, as a replacement for streaming replication? First of all, that requires a big chunk of custom development, so it's a bit of a stretch to say we have it already. Secondly, with that method, the standby still still be replaying the WAL one file at a time, which makes a difference with Hot Standby. -- 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] Review: listagg aggregate
One situation where this could actually matter in the long term is if we want to have an optimization for aggregate functions whose state variables can be combined. this could be important if we ever want to do parallel processing someday. So we could have, for example two subjobs build two sublists and then combiner the two lists later. in that case the separator might not be the one the user expected - our put another way the one the user expected might not be available when we need it. We could say this isn't a problem because not all aggregate functions will be amenable to such an optimization and perhaps this will just be one of them. Alternately we could just have faith that a solution will be found - it doesn't seem like it should be an insoluble problem to me. greg On 28 Jan 2010 15:57, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jan 28, 2010 at 9:01 AM, Pavel Stehule pavel Yeah. The real issue here is that in some cases you'd like to have non-aggregated parameters to an aggregate, but SQL has no notation to express that. I think Pavel's underlying complaint is that if the delimiter argument isn't constant, then we're exposing an implementation dependency in terms of just which values get separated by which delimiters. The most practical implementation seems to be that the first-call delimiter isn't actually used at all, and on subsequent calls the delimiter *precedes* the associated value, which is a bit surprising given the order in which one writes them. Not sure if this is worth documenting though. Those two or three people who actually try it will figure it out soon enough. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subs...
Re: [HACKERS] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 20:49 +0200, Heikki Linnakangas wrote: Simon Riggs wrote: I'm a little worried the feature set of streaming rep isn't any better than what we have already. Huh? Are you thinking of the Record-based Log Shipping described in the manual, using a program to poll pg_xlogfile_name_offset() in a tight loop, as a replacement for streaming replication? First of all, that requires a big chunk of custom development, so it's a bit of a stretch to say we have it already. It's been part of Skytools for years now... Secondly, with that method, the standby still still be replaying the WAL one file at a time, which makes a difference with Hot Standby. I'm not attempting to diss Streaming Rep, or anyone involved. What has been done is good internal work. I am pointing out and requesting that we should have a little more added before we stop for this release. -- 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] Streaming replication, and walsender during recovery
Simon Riggs wrote: On Thu, 2010-01-28 at 10:48 -0500, Tom Lane wrote: Fujii Masao masao.fu...@gmail.com writes: How about just making a restore_command copy the WAL files as the normal one (e.g., ...) instead of a pg_xlog/RECOVERYXLOG? Though we need to worry about deleting them, we can easily leave the task to the bgwriter. The reason for doing it that way was to limit disk space usage during a long restore. I'm not convinced we can leave the task to the bgwriter --- it shouldn't be deleting anything at that point. I think bgwriter means RemoveOldXlogFiles(), which would normally clear down files at checkpoint. If that was added to the end of RecoveryRestartPoint() to do roughly the same job then it could potentially work. SR added a RemoveOldXLogFiles() call to CreateRestartPoint(). (Since 8.4, RecoveryRestartPoint() just writes the location of the checkpoint record in shared memory, but doesn't actually perform the restartpoint; bgwriter does that in CreateRestartPoint()). However, since not every checkpoint is a restartpoint we might easily end up with significantly more WAL files on the standby than would normally be there when it would be a primary. Not sure if that is an issue in this case, but we can't just assume we can store all files needed to restart the standby on the standby itself, in all cases. That might be an argument to add a restartpoint_segments parameter, so we can trigger restartpoints on WAL volume as well as time. But even that would not put an absolute limit on the number of WAL files. I think it is a pretty important safety feature that we keep all the WAL around that's needed to recover the standby. To avoid out-of-disk-space situation, it's probably enough in practice to set checkpoint_timeout small enough in the standby to trigger restartpoints often enough. At the moment, we do retain streamed WAL as long as it's needed, but not the WAL restored from archive. -- 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] Streaming replication, and walsender during recovery
On Thu, 2010-01-28 at 21:00 +0200, Heikki Linnakangas wrote: However, since not every checkpoint is a restartpoint we might easily end up with significantly more WAL files on the standby than would normally be there when it would be a primary. Not sure if that is an issue in this case, but we can't just assume we can store all files needed to restart the standby on the standby itself, in all cases. That might be an argument to add a restartpoint_segments parameter, so we can trigger restartpoints on WAL volume as well as time. But even that would not put an absolute limit on the number of WAL files. I think it is a pretty important safety feature that we keep all the WAL around that's needed to recover the standby. To avoid out-of-disk-space situation, it's probably enough in practice to set checkpoint_timeout small enough in the standby to trigger restartpoints often enough. Hmm, I'm sorry but that's bogus. Retaining so much WAL that we are strongly in danger of blowing disk space is not what I would call a safety feature. Since there is no way to control or restrain the number of files for certain, that approach seems fatally flawed. Reducing checkpoint_timeout is the opposite of what you would want to do for performance. -- 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] Streaming replication, and walsender during recovery
Joshua D. Drake wrote: ...if with SR the entire log must be written before it streams to the slaves. No. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Segmentation fault occurs when the standby becomes primary, in SR
Fujii Masao wrote: When I created the trigger file to activate the standby server, I got the segmentation fault: ... The attached patch would fix the bug. Thanks, committed. (I kept the old comment, though, I liked it better) Now, whether we should even allow setting up a standby without restore_command is another question. It's *possible*, but you need to enable archiving in the master anyway to take an on-line backup, and you need the archive to catch up if the standby ever falls behind too much. Then again, if the database is small, maybe you don't mind taking a new base backup if the standby falls behind. And you *can* take a base backup with a dummy archive_command (ie. archive_command='/bin/true'), if you trust that the WAL files stay in pg_xlog long enough for standby to stream them from there. Perhaps we should require a restore_command. If you know what you're doing, you can always use '/bin/false' as restore_command to hack around it. -- 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: Segmentation fault occurs when the standby becomes primary, in SR
On Thu, Jan 28, 2010 at 2:23 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Perhaps we should require a restore_command. If you know what you're doing, you can always use '/bin/false' as restore_command to hack around it. That seems kind of needlessly hacky (and it won't work on Windows). Seems like it doesn't cost anything to let it be omitted altogether. ...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] plperl compiler warning
Tom Lane wrote: Joe Conway m...@joeconway.com writes: I pull directly from CVS, not git, but in any case my line 1117 is subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); so it appears to be the same What perl version are you using? What compiler version are you using? I'm on stock Fedora 12: I see the same on Fedora 11. The -E expansion of the line in question is subref = Perl_newRV(((PerlInterpreter *)pthread_getspecific((*Perl_Gthr_key_ptr(((void *)0), (SV*)GV*)sub_glob)-sv_u.svu_gp)-gp_cvgen ? ((void *)0) : (((GV*)sub_glob)-sv_u.svu_gp)-gp_cv)); so it's evidently unhappy about the fact that GvCVu can return null, while Perl_newRV is declared __attribute__((nonnull(2))). It looks to me like this is probably a live bug not just compiler hypersensitivity. It's on my F11 box too ... I guess it's time to upgrade my work machine, doesn't get reported by my usual setup. Tim, any suggestions? 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] plperl compiler warning
On Thu, Jan 28, 2010 at 12:49:20PM -0500, Tom Lane wrote: Joe Conway m...@joeconway.com writes: I pull directly from CVS, not git, but in any case my line 1117 is subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); so it appears to be the same What perl version are you using? What compiler version are you using? I'm on stock Fedora 12: I see the same on Fedora 11. The -E expansion of the line in question is subref = Perl_newRV(((PerlInterpreter *)pthread_getspecific((*Perl_Gthr_key_ptr(((void *)0), (SV*)GV*)sub_glob)-sv_u.svu_gp)-gp_cvgen ? ((void *)0) : (((GV*)sub_glob)-sv_u.svu_gp)-gp_cv)); so it's evidently unhappy about the fact that GvCVu can return null, while Perl_newRV is declared __attribute__((nonnull(2))). It looks to me like this is probably a live bug not just compiler hypersensitivity. Yes. (ISTR there have been cases where the notnull attribute was misapplied to some perl functions, but that's not the case here.) I think I missed this because the Xcode compiler on Snow Leopard is fairly old (gcc 4.2.1). Patch attached. Tim. diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index 9277072..2dd3458 100644 *** a/src/pl/plperl/plperl.c --- b/src/pl/plperl/plperl.c *** plperl_create_sub(plperl_proc_desc *prod *** 1113,1120 if (count == 1) { GV *sub_glob = (GV*)POPs; ! if (sub_glob SvTYPE(sub_glob) == SVt_PVGV) ! subref = newRV_inc((SV*)GvCVu((GV*)sub_glob)); } PUTBACK; --- 1113,1123 if (count == 1) { GV *sub_glob = (GV*)POPs; ! if (sub_glob SvTYPE(sub_glob) == SVt_PVGV) { ! SV *sv = (SV*)GvCVu((GV*)sub_glob); ! if (sv) ! subref = newRV_inc(sv); ! } } PUTBACK; -- 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: Typed Table
On tor, 2010-01-28 at 10:34 -0500, Tom Lane wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: ISTM you should explicitly grab a lock on the of-type at some point, to make sure it doesn't get dropped while you're busy creating the table. How do we protect against that for the types used in columns? We don't. There is no concept of a lock on a type. For scalar types this is more or less irrelevant anyway, since a scalar has no substructure that can be altered in any interesting way. I'm not sure how hard we ought to work on making composites behave differently. I think it's as likely to cause problems as solve them. The right thing would probably be SELECT FOR SHARE on the pg_type row, but I don't see that sort of thing used anywhere else in system catalog changes. -- 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 [PATCH]
On Thu, Jan 28, 2010 at 12:12:58PM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. ITYM on_untrusted_init. Right, sorry, got 'em backwards. I've done that several times. The naming is tricky because it's very dependent on your point of view. The 'trusted' language is for running 'untrusted' code and the 'untrusted' language is for running 'trusted' code. The naming convention is unfortunate. Just an observation from a newbie. I imagine it's been pointed out before. 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] Review: Typed Table
Peter Eisentraut pete...@gmx.net writes: The right thing would probably be SELECT FOR SHARE on the pg_type row, but I don't see that sort of thing used anywhere else in system catalog changes. If we were to do it the right thing would just be to define a locktag for type OIDs and add appropriate locking calls all over the system. But that would be a large, invasive change that seems far outside the scope of this patch, and certainly much beyond what can get done for 9.0. (Actually, if memory serves there is some notion of locking on arbitrary catalog objects already in the DROP code, so there probably is a suitable locktag defined already. But getting ALTER and generic type references to play along is still a major project, and I'm not convinced about the cost/benefit ...) 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 [PATCH]
On Thu, Jan 28, 2010 at 11:54:08AM -0500, Tom Lane wrote: Tim Bunce tim.bu...@pobox.com writes: I think the only controversial change is this one: - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs Both are PGC_USERSET. SPI functions are not available when the code is run. Errors are detected and reported as ereport(ERROR, ...) + plperl.on_trusted_init runs inside the Safe compartment. Isn't it a security hole if on_trusted_init is USERSET? That means an unprivileged user can determine what will happen in plperlu. SUSET would be saner. Yes. Good point (though ITYM on_UNtrusted_init). I'll change that. As I recall, Tom had concerns over the combination of PGC_USERSET and before-first-use semantics. IIRC, what I was unhappy about was exposing shared library load as a time when interesting-to-the-user things would happen. It looks like you have got rid of that and instead made it happen at the first call of a plperl or plperlu function (respectively). That seems like a fairly reasonable way to work, and if it's defined that way, there doesn't seem any reason not to allow them to be USERSET/SUSET. Great. But it ought to be documented like that, not with vague phrases like when the interpreter is initialized --- that means nothing to users. I'll change that. One issue that ought to be mentioned is what about transaction rollback. One could argue that if the first plperl call is inside a transaction that later rolls back, then all the side effects of that should be undone. But we have no way to undo whatever happened inside perl, and at least in most likely uses of on_init there wouldn't be any advantage in trying to force a redo anyway. I think it's okay to just define it as happening once regardless of any transaction failure (but of course this had better be documented). Will do. Once the previous patch lands I'll post an update to this patch with those changes applied. Thanks. 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] Streaming replication, and walsender during recovery
Guys, Hmm, I'm sorry but that's bogus. Retaining so much WAL that we are strongly in danger of blowing disk space is not what I would call a safety feature. Since there is no way to control or restrain the number of files for certain, that approach seems fatally flawed. Reducing checkpoint_timeout is the opposite of what you would want to do for performance. Which WAL are we talking about here? There's 3 copies to worry about: 1) master WAL 2) the archive copy of WAL 3) slave WAL --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
Re: [HACKERS] Add on_trusted_init and on_untrusted_init to plperl [PATCH]
On Jan 28, 2010, at 12:01 PM, Tim Bunce wrote: Once the previous patch lands I'll post an update to this patch with those changes applied. Ds the Add on_perl_init and proper destruction to plperl patch the one you're waiting on, then? Best, David, who loses track of these things -- 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 wrote: There has been some more discussion lately of problems caused by contrib/xml2. http://archives.postgresql.org/pgsql-bugs/2010-01/msg00251.php http://archives.postgresql.org/pgsql-bugs/2010-01/msg00198.php I think we need to either (1) fix the bugs and update the documentation to remove the statement that this will be removed or (2) actually remove it. Nobody seems interested in #1, so PFA a patch to do #2. It also rips out all the libxslt stuff, which seems to exist only for the purpose of supporting contrib/xml2. The problem is that there are people who use the XSLT and xpath_table stuff on text data and so don't run into these bugs. I agree it's a mess but I don't think just abandoning the functionality is a good idea. 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] further explain changes
On Sun, Jan 24, 2010 at 12:30 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jan 24, 2010 at 12:06 AM, Jaime Casanova why not let it go in ANALYZE, just as the sort info It's kinda long-winded - it adds like 4 extra lines for each hash join. I don't think I want to add that much clutter to regular E-A output. Well, that would only happen if you're deliberately obtuse about the formatting. The sort code manages to fit all the extra on one line, and I don't see why hash couldn't. OK, here's a new version. ...Robert --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -20,6 +20,7 @@ #include commands/explain.h #include commands/prepare.h #include commands/trigger.h +#include executor/hashjoin.h #include executor/instrument.h #include optimizer/clauses.h #include optimizer/planner.h @@ -67,6 +68,7 @@ static void show_upper_qual(List *qual, const char *qlabel, Plan *plan, ExplainState *es); static void show_sort_keys(Plan *sortplan, ExplainState *es); static void show_sort_info(SortState *sortstate, ExplainState *es); +static void show_hash_info(HashState *hashstate, ExplainState *es); static const char *explain_get_index_name(Oid indexId); static void ExplainScanTarget(Scan *plan, ExplainState *es); static void ExplainMemberNodes(List *plans, PlanState **planstate, @@ -1052,6 +1054,9 @@ ExplainNode(Plan *plan, PlanState *planstate, One-Time Filter, plan, es); show_upper_qual(plan-qual, Filter, plan, es); break; + case T_Hash: + show_hash_info((HashState *) planstate, es); + break; default: break; } @@ -1405,6 +1410,47 @@ show_sort_info(SortState *sortstate, ExplainState *es) } /* + * Show information on hash buckets/batches. + */ +static void +show_hash_info(HashState *hashstate, ExplainState *es) +{ + HashJoinTable hashtable; + + Assert(IsA(hashstate, HashState)); + hashtable = hashstate-hashtable; + + if (hashtable) + { + long spacePeakKb = (hashtable-spacePeak + 1023) / 1024; + if (es-format != EXPLAIN_FORMAT_TEXT) + { + ExplainPropertyLong(Hash Buckets, hashtable-nbuckets, es); + ExplainPropertyLong(Hash Batches, hashtable-nbatch, es); + ExplainPropertyLong(Original Hash Batches, +hashtable-nbatch_original, es); + ExplainPropertyLong(Peak Memory Usage, spacePeakKb, es); + } + else if (hashtable-nbatch_original != hashtable-nbatch) + { + appendStringInfoSpaces(es-str, es-indent * 2); + appendStringInfo(es-str, + Buckets: %d Batches: %d (originally %d) Memory Usage: %ldkB\n, + hashtable-nbuckets, hashtable-nbatch, + hashtable-nbatch_original, spacePeakKb); + } + else + { + appendStringInfoSpaces(es-str, es-indent * 2); + appendStringInfo(es-str, + Buckets: %d Batches: %d Memory Usage: %ldkB\n, + hashtable-nbuckets, hashtable-nbatch, + spacePeakKb); + } + } +} + +/* * Fetch the name of an index in an EXPLAIN * * We allow plugins to get control here so that plans involving hypothetical --- a/src/backend/executor/nodeHash.c +++ b/src/backend/executor/nodeHash.c @@ -287,6 +287,7 @@ ExecHashTableCreate(Hash *node, List *hashOperators) hashtable-innerBatchFile = NULL; hashtable-outerBatchFile = NULL; hashtable-spaceUsed = 0; + hashtable-spacePeak = 0; hashtable-spaceAllowed = work_mem * 1024L; hashtable-spaceUsedSkew = 0; hashtable-spaceAllowedSkew = @@ -719,6 +720,8 @@ ExecHashTableInsert(HashJoinTable hashtable, hashTuple-next = hashtable-buckets[bucketno]; hashtable-buckets[bucketno] = hashTuple; hashtable-spaceUsed += hashTupleSize; + if (hashtable-spaceUsed hashtable-spacePeak) + hashtable-spacePeak = hashtable-spaceUsed; if (hashtable-spaceUsed hashtable-spaceAllowed) ExecHashIncreaseNumBatches(hashtable); } @@ -1071,6 +1074,8 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) + mcvsToUse * sizeof(int); hashtable-spaceUsedSkew += nbuckets * sizeof(HashSkewBucket *) + mcvsToUse * sizeof(int); + if (hashtable-spaceUsed hashtable-spacePeak) + hashtable-spacePeak = hashtable-spaceUsed; /* * Create a skew bucket for each MCV hash value. @@ -1119,6 +1124,8 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) hashtable-nSkewBuckets++; hashtable-spaceUsed += SKEW_BUCKET_OVERHEAD; hashtable-spaceUsedSkew += SKEW_BUCKET_OVERHEAD; + if (hashtable-spaceUsed hashtable-spacePeak) +hashtable-spacePeak = hashtable-spaceUsed; } free_attstatsslot(node-skewColType, @@ -1205,6 +1212,8 @@ ExecHashSkewTableInsert(HashJoinTable hashtable, /* Account for space used, and back off if we've used too much */ hashtable-spaceUsed += hashTupleSize; hashtable-spaceUsedSkew += hashTupleSize; + if (hashtable-spaceUsed hashtable-spacePeak) + hashtable-spacePeak = hashtable-spaceUsed; while (hashtable-spaceUsedSkew hashtable-spaceAllowedSkew) ExecHashRemoveNextSkewBucket(hashtable);
Re: [HACKERS] make everything target
I wrote: Alvaro Herrera wrote: Andrew Dunstan wrote: Alvaro Herrera wrote: make world sounds reasonable and I've remember seeing it elsewhere. Here's a simple patch. Comments? Should the new targets be added to Makefile too? Sure, good idea. One more thing: do we want the new targets world and install-world documented, or just left for developers? 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] remove contrib/xml2
On Thu, Jan 28, 2010 at 4:18 PM, Andrew Dunstan and...@dunslane.net wrote: Robert Haas wrote: There has been some more discussion lately of problems caused by contrib/xml2. http://archives.postgresql.org/pgsql-bugs/2010-01/msg00251.php http://archives.postgresql.org/pgsql-bugs/2010-01/msg00198.php I think we need to either (1) fix the bugs and update the documentation to remove the statement that this will be removed or (2) actually remove it. Nobody seems interested in #1, so PFA a patch to do #2. It also rips out all the libxslt stuff, which seems to exist only for the purpose of supporting contrib/xml2. The problem is that there are people who use the XSLT and xpath_table stuff on text data and so don't run into these bugs. I agree it's a mess but I don't think just abandoning the functionality is a good idea. I'm one of those people. :) Expecting to see contrib/xml2 go away at some point, possibly without replacements for xslt_process and xpath_table, I've been working on some plpgsql and plperlu work-alikes targeted at TEXT columns, as the xml2 versions do. I hope these (attached) will be of some help to others. Note, these are not the exact functions I use, they are lightly edited to remove the use of wrappers I've created to paper over the transition from xpath_nodeset() to core XPATH(). -- Mike Rylander | VP, Research and Design | Equinox Software, Inc. / The Evergreen Experts | phone: 1-877-OPEN-ILS (673-6457) | email: mi...@esilibrary.com | web: http://www.esilibrary.com xml2-replacements.sql Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Hot Standby: Relation-specific deferred conflict resolution
Conflict resolution improvements are important to include in this release, as discussed many times. Proposal given here http://archives.postgresql.org/pgsql-hackers/2009-12/msg01175.php presents a viable design to improve this. Following patch is a complete working implementation of that design. I'm still testing it, but its worth publishing as early as possible to allow discussion. Not for commit, just yet, but soon. standby.c changes are to decide whether to defer recovery based upon relfilenode of WAL record. If resolution deferred, re-check for conflict during LockAcquire() and fail with snapshot error, just as if resolution had never been deferred. Also, an optimisation of conflict processing to avoid continual re-evaluation of conflicts since some are now deferred. API changes in heapam and nbtxlog to pass thru RelFileNode API changes in indexcmds, no behaviour changes procarray changes to implement LatestRemovedXid cache backend/access/heap/heapam.c|6 - backend/access/nbtree/nbtxlog.c |2 backend/commands/indexcmds.c|4 - backend/storage/ipc/procarray.c | 55 +++- backend/storage/ipc/standby.c | 112 +++-- backend/storage/lmgr/lock.c | 124 ++-- include/storage/lock.h |8 ++ include/storage/proc.h |5 + include/storage/standby.h |9 ++ 9 files changed, 292 insertions(+), 33 deletions(-) -- Simon Riggs www.2ndQuadrant.com *** a/src/backend/access/heap/heapam.c --- b/src/backend/access/heap/heapam.c *** *** 4139,4145 heap_xlog_cleanup_info(XLogRecPtr lsn, XLogRecord *record) xl_heap_cleanup_info *xlrec = (xl_heap_cleanup_info *) XLogRecGetData(record); if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid); /* * Actual operation is a no-op. Record type exists to provide a means --- 4139,4145 xl_heap_cleanup_info *xlrec = (xl_heap_cleanup_info *) XLogRecGetData(record); if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid, xlrec-node); /* * Actual operation is a no-op. Record type exists to provide a means *** *** 4171,4177 heap_xlog_clean(XLogRecPtr lsn, XLogRecord *record, bool clean_move) * no queries running for which the removed tuples are still visible. */ if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid); RestoreBkpBlocks(lsn, record, true); --- 4171,4177 * no queries running for which the removed tuples are still visible. */ if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid, xlrec-node); RestoreBkpBlocks(lsn, record, true); *** *** 4241,4247 heap_xlog_freeze(XLogRecPtr lsn, XLogRecord *record) * consider the frozen xids as running. */ if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(cutoff_xid); RestoreBkpBlocks(lsn, record, false); --- 4241,4247 * consider the frozen xids as running. */ if (InHotStandby) ! ResolveRecoveryConflictWithSnapshot(cutoff_xid, xlrec-node); RestoreBkpBlocks(lsn, record, false); *** a/src/backend/access/nbtree/nbtxlog.c --- b/src/backend/access/nbtree/nbtxlog.c *** *** 833,839 btree_redo(XLogRecPtr lsn, XLogRecord *record) * here is worth some thought and possibly some effort to * improve. */ ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid); } /* --- 833,839 * here is worth some thought and possibly some effort to * improve. */ ! ResolveRecoveryConflictWithSnapshot(xlrec-latestRemovedXid, xlrec-node); } /* *** a/src/backend/commands/indexcmds.c --- b/src/backend/commands/indexcmds.c *** *** 540,546 DefineIndex(RangeVar *heapRelation, * check for that. Also, prepared xacts are not reported, which is fine * since they certainly aren't going to do anything more. */ ! old_lockholders = GetLockConflicts(heaplocktag, ShareLock); while (VirtualTransactionIdIsValid(*old_lockholders)) { --- 540,546 * check for that. Also, prepared xacts are not reported, which is fine * since they certainly aren't going to do anything more. */ ! old_lockholders = GetLockConflicts(heaplocktag, ShareLock, InvalidTransactionId); while (VirtualTransactionIdIsValid(*old_lockholders)) { *** *** 627,633 DefineIndex(RangeVar *heapRelation, * We once again wait until no transaction can have the table open with * the index marked as read-only for updates. */ ! old_lockholders = GetLockConflicts(heaplocktag, ShareLock); while (VirtualTransactionIdIsValid(*old_lockholders)) { --- 627,633 * We once again wait until no transaction can have the table open with * the index marked as read-only for updates. */ ! old_lockholders =
Re: [HACKERS] CommitFest status summary 2010-01-27
On Wed, Jan 27, 2010 at 4:05 PM, Robert Haas robertmh...@gmail.com wrote: Waiting for Re-Review (5) = Writeable CTEs Set this ready for commit. For such a small patch, it's a wonderful feature. I think it deserves a fair shot on this 'fest. insert/returning/subquery is far and away one of the most requested features, and this patch delivers the goods in a very elegant way. 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] ECPGset_var
Hi, Boszormenyi Zoltan írta: I think the best would be to delete the NAN test from outofscope.pgc and fix the double/numeric NaN/Inf/-Inf problem separately and have their own test case. the attached patch attempts to fix NaN/Infinity problems in ECPG for float/double/numeric/decimal. I had to introduce a new NUMERIC_NULL value for the numeric sign to still be able to use risnull()/rsetnull(). 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/ecpglib/data.c pgsql/src/interfaces/ecpg/ecpglib/data.c *** pgsql.orig/src/interfaces/ecpg/ecpglib/data.c 2010-01-01 14:11:38.0 +0100 --- pgsql/src/interfaces/ecpg/ecpglib/data.c 2010-01-28 21:36:16.0 +0100 *** *** 5,10 --- 5,11 #include stdlib.h #include string.h + #include math.h #include ecpgtype.h #include ecpglib.h *** garbage_left(enum ARRAY_TYPE isarray, ch *** 38,43 --- 39,96 return false; } + /* stolen code from src/backend/utils/adt/float.c */ + #if defined(WIN32) !defined(NAN) + static const uint32 nan[2] = {0x, 0x7fff}; + + #define NAN (*(const double *) nan) + #endif + + static double + get_float8_infinity(void) + { + #ifdef INFINITY + return (double) INFINITY; + #else + return (double) (HUGE_VAL * HUGE_VAL); + #endif + } + + static double + get_float8_nan(void) + { + #ifdef NAN + return (double) NAN; + #else + return (double) (0.0 / 0.0); + #endif + } + + static bool + check_special_value(char *ptr, double *retval, char **endptr) + { + if (!pg_strncasecmp(ptr, NaN, 3)) + { + *retval = get_float8_nan(); + *endptr = ptr + 3; + return true; + } + else if (!pg_strncasecmp(ptr, Infinity, 8)) + { + *retval = get_float8_infinity(); + *endptr = ptr + 8; + return true; + } + else if (!pg_strncasecmp(ptr, -Infinity, 9)) + { + *retval = -get_float8_infinity(); + *endptr = ptr + 9; + return true; + } + + return false; + } + bool ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno, enum ECPGttype type, enum ECPGttype ind_type, *** ecpg_get_data(const PGresult *results, i *** 300,307 case ECPGt_float: case ECPGt_double: if (isarray *pval == '') ! dres = strtod(pval + 1, scan_length); ! else dres = strtod(pval, scan_length); if (isarray *scan_length == '') --- 353,361 case ECPGt_float: case ECPGt_double: if (isarray *pval == '') ! pval++; ! ! if (!check_special_value(pval, dres, scan_length)) dres = strtod(pval, scan_length); if (isarray *scan_length == '') diff -dcrpN pgsql.orig/src/interfaces/ecpg/ecpglib/misc.c pgsql/src/interfaces/ecpg/ecpglib/misc.c *** pgsql.orig/src/interfaces/ecpg/ecpglib/misc.c 2010-01-26 10:09:40.0 +0100 --- pgsql/src/interfaces/ecpg/ecpglib/misc.c 2010-01-28 21:53:44.0 +0100 *** ECPGset_noind_null(enum ECPGttype type, *** 344,354 break; case ECPGt_decimal: memset((char *) ptr, 0, sizeof(decimal)); ! ((decimal *) ptr)-sign = NUMERIC_NAN; break; case ECPGt_numeric: memset((char *) ptr, 0, sizeof(numeric)); ! ((numeric *) ptr)-sign = NUMERIC_NAN; break; case ECPGt_interval: memset((char *) ptr, 0xff, sizeof(interval)); --- 344,354 break; case ECPGt_decimal: memset((char *) ptr, 0, sizeof(decimal)); ! ((decimal *) ptr)-sign = NUMERIC_NULL; break; case ECPGt_numeric: memset((char *) ptr, 0, sizeof(numeric)); ! ((numeric *) ptr)-sign = NUMERIC_NULL; break; case ECPGt_interval: memset((char *) ptr, 0xff, sizeof(interval)); *** ECPGis_noind_null(enum ECPGttype type, v *** 416,426 return true; break; case ECPGt_decimal: ! if (((decimal *) ptr)-sign == NUMERIC_NAN) return true; break; case ECPGt_numeric: ! if (((numeric *) ptr)-sign == NUMERIC_NAN) return true; break; case ECPGt_interval: --- 416,426 return true; break; case ECPGt_decimal: ! if (((decimal *) ptr)-sign == NUMERIC_NULL) return true; break; case ECPGt_numeric: ! if (((numeric *) ptr)-sign == NUMERIC_NULL) return true; break; case ECPGt_interval: diff -dcrpN pgsql.orig/src/interfaces/ecpg/include/pgtypes_numeric.h pgsql/src/interfaces/ecpg/include/pgtypes_numeric.h *** pgsql.orig/src/interfaces/ecpg/include/pgtypes_numeric.h 2010-01-06 08:43:28.0 +0100 --- pgsql/src/interfaces/ecpg/include/pgtypes_numeric.h 2010-01-28
Re: [HACKERS] remove contrib/xml2
Andrew Dunstan and...@dunslane.net writes: Robert Haas wrote: I think we need to either (1) fix the bugs and update the documentation to remove the statement that this will be removed or (2) actually remove it. I agree it's a mess but I don't think just abandoning the functionality is a good idea. Yeah, we can't really remove it until we have a plausible substitute for the xpath_table functionality. This is in the TODO list ... 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
[HACKERS] Pathological regexp match
We came across a regexp that takes very much longer than expected. PostgreSQL 8.4.1 on x86_64-unknown-linux-gnu, compiled by GCC gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-44), 64-bit SELECT 'ooo...' ~ $r$Z(Q)[^Q]*A.*?(\1)$r$; -- omitted for email brevity ?column? -- t (1 row) Time: 90525.148 ms The full query is below. The same match in perl takes less than 0.01 seconds on the same hardware. #!/bin/env perl use warnings; use strict; my $sample = 'ooo...'; # omitted for email brevity if ($sample =~ /Z(Q)[^Q]*A.*?(\1)/) { print 'matches'; } else { print 'does not match'; } This is a simplified version of a match that finally finished after 18 hours. Given the nearly 4 orders of magnitude difference between the Perl script and the Postgres version, is there something that could be improved in the Postgres regex engine? Cheers, Michael Glaesemann michael.glaesem...@myyearbook.com SELECT
Re: [HACKERS] remove contrib/xml2
Yeah, we can't really remove it until we have a plausible substitute for the xpath_table functionality. This is in the TODO list ... What about moving it to pgfoundry? I'm really not keen on shipping known-broken stuff in /contrib. --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
Re: [HACKERS] plperl compiler warning
Tim Bunce wrote: It looks to me like this is probably a live bug not just compiler hypersensitivity. Yes. (ISTR there have been cases where the notnull attribute was misapplied to some perl functions, but that's not the case here.) I think I missed this because the Xcode compiler on Snow Leopard is fairly old (gcc 4.2.1). Patch attached. Thanks. Applied. 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] make everything target
On tor, 2010-01-28 at 16:21 -0500, Andrew Dunstan wrote: One more thing: do we want the new targets world and install-world documented, or just left for developers? Document them. How else would new developers learn about them? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 64-bit size pgbench
Attached is a patch that fixes a long standing bug in pgbench: it won't handle scale factors above ~4000 (around 60GB) because it uses 32-bit integers for its computations related to the number of accounts, and it just crashes badly when you exceed that. This month I've run into two systems where that was barely enough to exceed physical RAM, so I'd expect this to be a significant limiting factor during 9.0's lifetime. A few people have complained about it already in 8.4. The index size on the big accounts table has to increase for this to work, it's a bigint now instead of an int. That's going to mean a drop in results for some tests, just because less index will fit in RAM. I'll quantify that better before submitting something final here. I still have some other testing left to do as well: making sure I didn't break the new \setshell feature (am suspicious of strtol()), confirming the random numbers are still as random as they should be (there was a little bug in the debugging code related to that, too). Was looking for general feedback on whether the way I've converted this to use 64 bit integers for the account numbers seems appropriate, and to see if there's any objection to fixing this in general given the potential downsides. Here's the patch in action on previously unreachable sizes (this is a system with 8GB of RAM, so I'm basically just testing seek speed here): $ ./pgbench -j 4 -c 8 -T 30 -S pgbench starting vacuum...end. transaction type: SELECT only scaling factor: 5000 query mode: simple number of clients: 8 number of threads: 4 duration: 30 s number of transactions actually processed: 2466 tps = 82.010509 (including connections establishing) tps = 82.042946 (excluding connections establishing) $ psql -x -c select relname,reltuples from pg_class where relname='pgbench_accounts' -d pgbench relname | pgbench_accounts reltuples | 5e+08 $ psql -x -c select pg_size_pretty(pg_table_size('pgbench_accounts')) -d pgbench pg_size_pretty | 63 GB $ psql -x -c select aid from pgbench_accounts order by aid limit 1 -d pgbench aid | 1 $ psql -x -c select aid from pgbench_accounts order by aid desc limit 1 -d pgbench aid | 5 -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c index 38086a5..8a7064a 100644 *** a/contrib/pgbench/pgbench.c --- b/contrib/pgbench/pgbench.c *** usage(const char *progname) *** 313,326 } /* random number generator: uniform distribution from min to max inclusive */ ! static int ! getrand(int min, int max) { /* * Odd coding is so that min and max have approximately the same chance of * being selected as do numbers between them. */ ! return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0)); } /* call PQexec() and exit() on failure */ --- 313,326 } /* random number generator: uniform distribution from min to max inclusive */ ! static int64 ! getrand(int64 min, int64 max) { /* * Odd coding is so that min and max have approximately the same chance of * being selected as do numbers between them. */ ! return min + (int64) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0)); } /* call PQexec() and exit() on failure */ *** runShellCommand(CState *st, char *variab *** 630,636 FILE *fp; char res[64]; char *endptr; ! int retval; /* * Join arguments with whilespace separaters. Arguments starting with --- 630,636 FILE *fp; char res[64]; char *endptr; ! int64 retval; /* * Join arguments with whilespace separaters. Arguments starting with *** runShellCommand(CState *st, char *variab *** 704,710 } /* Check whether the result is an integer and assign it to the variable */ ! retval = (int) strtol(res, endptr, 10); while (*endptr != '\0' isspace((unsigned char) *endptr)) endptr++; if (*res == '\0' || *endptr != '\0') --- 704,710 } /* Check whether the result is an integer and assign it to the variable */ ! retval = (int64) strtol(res, endptr, 19); while (*endptr != '\0' isspace((unsigned char) *endptr)) endptr++; if (*res == '\0' || *endptr != '\0') *** runShellCommand(CState *st, char *variab *** 712,718 fprintf(stderr, %s: must return an integer ('%s' returned)\n, argv[0], res); return false; } ! snprintf(res, sizeof(res), %d, retval); if (!putVariable(st, setshell, variable, res)) return false; --- 712,718 fprintf(stderr, %s: must return an integer ('%s' returned)\n, argv[0], res); return false; } ! snprintf(res, sizeof(res), INT64_FORMAT, retval); if (!putVariable(st, setshell, variable, res)) return false; *** top: *** 959,966 if (pg_strcasecmp(argv[0],
Re: [HACKERS] Review: Typed Table
On tor, 2010-01-28 at 00:43 +0900, Hitoshi Harada wrote: OK, I confirmed all the issues relevant to the patch were fixed. I'm not so familiar with transaction detail, so I leave it as a known issue. I have applied this now, because it appeared that the locking issue is a known more general problem. -- 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/01/28 18:21), Takahiro Itagaki wrote: KaiGai Koheikai...@ak.jp.nec.com wrote: The attached patch uses one TOC entry for each blob objects. When I'm testing the new patch, I found ALTER LARGE OBJECT command returns ALTER LARGEOBJECT tag. Should it be ALTER LARGE(space)OBJECT instead? As I remember, we had decided not to use LARGEOBJECT (without a space) in user-visible messages, right? Sorry, I left for fix this tag when I was pointed out LARGEOBJECT should be LARGE(space)OBJECT. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com pgsql-fix-large_object-tag.patch Description: application/octect-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] returning array in a field together with other types
I'm trying to return a set of record as: (text, int[]), (text, int[]), ... row from C and I really would like to avoid to use BuildTupleFromCString So this is how I think my function should end... char *curout; /* cstring */ int4 *pos; ... get_typlenbyvalalign(INT4OID, typlen, typbyval, typalign); v_pos = construct_array( pos, npos, INT4OID, typlen, typbyval, typalign ); values[0] = GET_TEXT(curout); /* correct? */ values[1] = PointerGetDatum(v_pos); /* */ resultTuple = heap_form_tuple(inter_call_data-tupd, values, nulls); result = HeapTupleGetDatum(resultTuple); SRF_RETURN_NEXT(fctx, result); ... I've seen that GET_TEXT(curout) just call textin. I wonder if other GET_ macro aren't really converting to text representation and then back to Datum... so there is no real difference with using BuildTupleFromCString. BTW could I pass static values to construct_array since they are basic type array so the size etc... should be known at compile time, inspite of having to call get_typlenbyvalalign? thanks -- Ivan Sergio Borgonovo http://www.webthatworks.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers