Re: [HACKERS] Streaming replication and pg_xlogfile_name()

2010-01-28 Thread Heikki Linnakangas
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-01-28 Thread Pavel Stehule
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-01-28 Thread Pavel Stehule
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-01-28 Thread Pavel Stehule
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)

2010-01-28 Thread Guillaume Lelarge
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

2010-01-28 Thread Takahiro Itagaki

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]

2010-01-28 Thread Tim Bunce
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)

2010-01-28 Thread Takahiro Itagaki

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

2010-01-28 Thread Heikki Linnakangas
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]

2010-01-28 Thread Tim Bunce
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-01-28 Thread Pavel Stehule
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

2010-01-28 Thread Fujii Masao
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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Boszormenyi Zoltan
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

2010-01-28 Thread Fujii Masao
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

2010-01-28 Thread Leonardo F
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

2010-01-28 Thread Alvaro Herrera
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

2010-01-28 Thread Andrew Dunstan



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

2010-01-28 Thread Robert Haas
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-01-28 Thread Pavel Stehule
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?

2010-01-28 Thread Pavel Stehule
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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Robert Haas
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]

2010-01-28 Thread Andrew Dunstan



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

2010-01-28 Thread Alvaro Herrera
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

2010-01-28 Thread Joe Conway
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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Fujii Masao
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

2010-01-28 Thread Tim Bunce
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)

2010-01-28 Thread Tom Lane
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-01-28 Thread Pavel Stehule
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

2010-01-28 Thread Tom Lane
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]

2010-01-28 Thread Tom Lane
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]

2010-01-28 Thread Tim Bunce
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-01-28 Thread Robert Haas
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Robert Haas
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]

2010-01-28 Thread Tim Bunce
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Joe Conway
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]

2010-01-28 Thread Tim Bunce
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

2010-01-28 Thread Tom Lane
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]

2010-01-28 Thread Tom Lane
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-01-28 Thread Pavel Stehule
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]

2010-01-28 Thread Andrew Dunstan



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-01-28 Thread Hitoshi Harada
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

2010-01-28 Thread Robert Haas
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-01-28 Thread Pavel Stehule
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-01-28 Thread Pavel Stehule
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

2010-01-28 Thread Tom Lane
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-01-28 Thread Pavel Stehule
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]

2010-01-28 Thread Tom Lane
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-01-28 Thread Pavel Stehule
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Marko Tiikkaja
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

2010-01-28 Thread David E. Wheeler
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Greg Smith

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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Joshua D. Drake
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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Greg Stark
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Simon Riggs
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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Heikki Linnakangas
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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Andrew Dunstan



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

2010-01-28 Thread Tim Bunce
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

2010-01-28 Thread Peter Eisentraut
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]

2010-01-28 Thread Tim Bunce
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

2010-01-28 Thread Tom Lane
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]

2010-01-28 Thread Tim Bunce
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

2010-01-28 Thread Josh Berkus
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]

2010-01-28 Thread David E. Wheeler
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

2010-01-28 Thread Andrew Dunstan




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

2010-01-28 Thread Robert Haas
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

2010-01-28 Thread Andrew Dunstan


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

2010-01-28 Thread Mike Rylander
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

2010-01-28 Thread Simon Riggs

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

2010-01-28 Thread Merlin Moncure
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

2010-01-28 Thread Boszormenyi Zoltan
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

2010-01-28 Thread Tom Lane
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

2010-01-28 Thread Michael Glaesemann
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

2010-01-28 Thread Josh Berkus

 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

2010-01-28 Thread Andrew Dunstan



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

2010-01-28 Thread Peter Eisentraut
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

2010-01-28 Thread Greg Smith
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

2010-01-28 Thread Peter Eisentraut
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 Thread KaiGai Kohei
(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

2010-01-28 Thread Ivan Sergio Borgonovo
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


  1   2   >