Re: [HACKERS] Replication logging

2011-01-17 Thread Fujii Masao
On Tue, Jan 18, 2011 at 4:15 PM, Heikki Linnakangas
 wrote:
> I also find it weird that incoming replication connections are logged by
> default. In the standby, we also log "streaming replication successfully
> connected to primary", which serves much of the same debugging purpose. That
> standby-side message is ok, we have a tradition of being more verbose during
> recovery, we also emit the "restored log file \"%s\" from archive" message
> for every WAL segment restored from archive for example.
>
> We could turn log_connections into an enum, like log_statement:
>
> log_connections = 'none'        # none, replication, regular, all

+1

We should treat log_disconnections the same?

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] ToDo List Item - System Table Index Clustering

2011-01-17 Thread Simone Aiken

Followup on System Table Index clustering ToDo -

It looks like to implement this I need to do the following:

1 - Add statements to indexing.h to cluster the selected indexes.
A do-nothing define at the top to suppress warnings and then
lines below for perl to parse out.

#define DECLARE_CLUSTER_INDEX(table,index) ...
( add the defines under the index declarations ).

2 - Alter genbki.pl to produce the appropriate statements in 
postgres.bki when it reads the new lines in indexing.h.
Will hold them in memory until the end of the file so they
will come in after 'Build Indices' is called.

CLUSTER tablename USING indexname

3 - Initdb will pipe the commands in postgres.bki to the
postgres executable running in --boot mode. Code
will need to be added to bootparse.y to recognize
this new command and resolve it into a call to

cluster_rel( tabOID, indOID, 0, 0, -1, -1 );


Speak now before I learn Bison ... actually I should probably
learn Bison anyway.  After ProC other pre-compilation languages
can't be that bad.

Sound all right?

Thanks,

-Simone Aiken


On Jan 15, 2011, at 10:11 PM, Simone Aiken wrote:

> 
> Hello Postgres Hackers,
> 
> In reference to this todo item about clustering system table indexes, 
>   
> ( http://archives.postgresql.org/pgsql-hackers/2004-05/msg00989.php ) 
> I have been studying the system tables to see which would benefit  from 
> clustering.  I have some index suggestions and a question if you have a 
> moment.
> 
> Cluster Candidates:
> 
>   pg_attribute:  Make the existing index ( attrelid, attnum ) clustered 
> to 
>   order it by table and column.
>   
>   pg_attrdef:  Existing index ( adrelid, adnum ) clustered to order it
>   by table and column.
> 
>   pg_constraint:  Existing index ( conrelid ) clustered to get table 
>   constraints contiguous.
> 
>   pg_depend: Existing Index (refclassid, refobjid, refobjsubid) clustered
>   to so that when the referenced object is changed its dependencies 
>   arevcontiguous.
> 
>   pg_description: Make the existing index ( Objoid, classoid, objsubid ) 
>   clustered to order it by entity, catalog, and optional column.  
>   * reversing the first two columns makes more sense to me ... 
>   catalog, object, column or since object implies catalog ( 
> right? ) 
>   just dispensing with catalog altogether, but that would mean 
>   creating a new index.
>   
>   pg_shdependent: Existing index (refclassid, refobjid) clustered for 
>   same reason as pg_depend.
> 
>   pg_statistic: Existing index (starelid, staattnum) clustered to order 
>   it by table and column.
> 
>   pg_trigger:  Make the existing index ( tgrelid, tgname ) clustered to 
>   order it by table then name getting all the triggers on a table 
> together.
> 
> Maybe Cluster:
> 
>   pg_rewrite: Not sure about this one ... The existing index ( ev_class,
>   rulename ) seems logical to cluster to get all the rewrite rules for a
>   given table contiguous but in the db's available to me virtually every
>   table only has one rewrite rule.  
> 
>   pg_auth_members:  We could order it by role or by member of
>   that role.  Not sure which would be more valuable.
> 
> 
> Stupid newbie question:
> 
> 
>   is there a way to make queries on the system tables show me what 
>   is actually there when I'm poking around?  So for example:
> 
>   Select * from pg_type limit 1;
> 
>   tells me that the typoutput is 'boolout'.  An english string rather 
> than 
>   a number.  So even though the documentation says that column
>   maps to pg_proc.oid I can't then write:
> 
>   Select * from pg_proc where oid = 'boolout';
> 
>   It would be very helpful if I wasn't learning the system but since I
>   am I'd like to turn it off for now.  Fewer layers of abstraction.
> 
> 
> Thanks,
> 
> Simone Aiken
> 
> 303-956-7188
> Quietly Competent Consulting
> 
> 
> 
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers



Re: [HACKERS] Replication logging

2011-01-17 Thread Heikki Linnakangas

On 17.01.2011 21:04, Robert Haas wrote:

On Mon, Jan 17, 2011 at 1:57 PM, Tom Lane  wrote:

I'm of the opinion that the correct way of "lowering in later releases"
is to make the messages obey Log_connections.  The "needed for debug"
argument seems mighty weak to me even for the time, and surely falls
down now.


On a busy system, you could have a pretty high rate of messages
spewing forth for regular connections - that's a lot to wade through
if all you really want to see are the replication connections, which
should be much lower volume.  But I guess now that we have
pg_stat_replication it's a little easier to see the status of
replication anyway.  On the whole I've found the default setting here
very pleasant, so I'm reluctant to change it, but it sounds like I
might be out-voted.


I also find it weird that incoming replication connections are logged by 
default. In the standby, we also log "streaming replication successfully 
connected to primary", which serves much of the same debugging purpose. 
That standby-side message is ok, we have a tradition of being more 
verbose during recovery, we also emit the "restored log file \"%s\" from 
archive" message for every WAL segment restored from archive for example.


We could turn log_connections into an enum, like log_statement:

log_connections = 'none'# none, replication, regular, all

--
  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] We need to log aborted autovacuums

2011-01-17 Thread Peter Eisentraut
On mån, 2011-01-17 at 17:26 -0800, Josh Berkus wrote:
> However, it's hard for me to imagine a real-world situation where a
> table would be under repeated full-table-locks from multiple
> connections.  Can anyone else?

If you want to do assertion-type checks at the end of transactions in
the current fake-serializable mode, you need to lock the table.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SQL/MED - file_fdw

2011-01-17 Thread Itagaki Takahiro
On Sat, Jan 15, 2011 at 08:35, Shigeru HANADA  wrote:
> Interface of NextCopyFrom() is fixed to return HeapTuple, to support
> tableoid without any change to TupleTableSlot.
>
> 3) 20110114-copy_export_HeapTupe.patch
> This patch fixes interface of NextCopyFrom() to return results as
> HeapTuple.

I think file_fdw can return tuples in virtual tuples forms,
and ForeignNext() calls ExecMaterializeSlot() to store tableoid.

-- 
Itagaki Takahiro

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] MingW + GCC 4.5.2 + Relocate libpq.dll = SegFault

2011-01-17 Thread Charlie Savage
I'm compiling postgresql 9.0.2 using msys + mingw + gcc 4.5.2 (latest 
official release from mingw).  This is on Windows 7 64-bit.


Unfortunately the built dlls, at least libpq.dll, crash if they need to 
be relocated.  This happens to me when loading libpq.dll into a project 
that has a number of other dll requirements.  Note this does NOT happen 
when building with gcc 3.4.5.


Using GDB to track down the problem, the error occurs in 
__gcc_register_frame and looks to be the same error described here:


http://old.nabble.com/Bad-DLL-relocation---reproducible-w--test-case-td18292380.html

Note a similar sounding error described, and fixed, in newer releases of 
binutils (which mingw provides and I am using) is described here:


http://lists-archives.org/mingw-users/11369-error-0xc005-is-now-fixed.html

Looking at the postgresql Makefiles, the dlls are built using dllwrap. 
In particular, see src/Makefile.shlib line 413 and src/Makefile.port 
line 25.


When I change these to not use dllwrap and instead just use gcc -shared 
the problem goes away.  Therefore I'd like to propose:


1.  Change line 413 in src/Makefile.shlib

$(DLLWRAP) -o $@ --dllname $(shlib) $(DLLWRAP_FLAGS) --def 
$(DLL_DEFFILE) $(OBJS) $(LDFLAGS) $(LDFLAGS_SL) $(SHLIB_LINK)


To:

$(CC) -shared -o $@ $(DLL_DEFFILE) $(OBJS) $(LDFLAGS) $(LDFLAGS_SL) 
$(SHLIB_LINK)


2.  Changle line 73 in src/Makefile.port:

$(DLLWRAP) -o $@ --def $*.def $< $(LDFLAGS) $(LDFLAGS_SL) $(BE_DLLLIBS)

To:

$(CC) -shared -o $@ $*.def $< $(LDFLAGS) $(LDFLAGS_SL) $(BE_DLLLIBS)

I tested this by intentionally compiling libpq.dll using the link flag 
-Wl,--image-base to make sure that its base address conflicted with 
another dll loaded by my program.  With the proposed changes, windows 
successfully relocated libpq.dll without causing a segmentation fault.


I don't claim to know why dllwrap is producing dlls that can't be 
relocated while gcc -shared is.  But it appears to have been deprecated 
back in 2006 according to the binutils mailing list:


http://www.mail-archive.com/bug-binutils@gnu.org/msg01470.html

So between being deprecated and not producing relocatable dlls, it seems 
like its best to stop using dllwrap.  If this seems like a reasonable 
change I can put together a quick patch if that helps.


Thanks,

Charlie

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Confusing comment in TransactionIdIsInProgress

2011-01-17 Thread Jim Nasby
Shouldn't the comment read "If first time through"?

/*
 * If not first time through, get workspace to remember main XIDs in. We
 * malloc it permanently to avoid repeated palloc/pfree overhead.
 */
if (xids == NULL)
{
...
xids = (TransactionId *) malloc(maxxids * 
sizeof(TransactionId));
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Tom Lane
"Kevin Grittner"  writes:
>> Dan Ports  wrote:
>  The biggest, in my mind, is whether
> MySerializableXact needs to be declared volatile.
 
> The problem is that I don't have a very clear sense of what it really
> does, which is not helped much by having done a few years of Java
> programming, where the same keyword seems to have a vaguely-similar-
> but-not-really-the-same meaning.

In C it means that the compiler must not try to optimize away loads or
stores of the variable, because the variable is subject to being read or
changed by outside forces (interrupt service routines, other processes
or threads, etc).

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_filedump moved to pgfoundry

2011-01-17 Thread Tom Lane
David Fetter  writes:
> On Mon, Jan 17, 2011 at 09:48:58PM -0500, Tom Lane wrote:
>> (Before someone suggests folding it into contrib/: we can't because
>> of license issues.  pg_filedump is GPL, per Red Hat company policy,
>> and that's not going to change.)

> Who's the copyright holder(s)?  If it's all individual contributors,
> Red Hat policy is not in play.

Sorry David, it was written on the company's dime.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_filedump moved to pgfoundry

2011-01-17 Thread David Fetter
On Mon, Jan 17, 2011 at 09:48:58PM -0500, Tom Lane wrote:
> I've gotten permission to move pg_filedump from its former home at
> sources.redhat.com to pgfoundry.  You can find the historical release
> tarballs as well as current sources at
>   http://pgfoundry.org/projects/pgfiledump/
> 
> One advantage of doing this is it will be a lot easier to let other
> folks join in the fun of hacking it.  If anyone has been harboring
> a yen to improve pg_filedump, please join that pgfoundry project.
> 
> (Before someone suggests folding it into contrib/: we can't because
> of license issues.  pg_filedump is GPL, per Red Hat company policy,
> and that's not going to change.)

Who's the copyright holder(s)?  If it's all individual contributors,
Red Hat policy is not in play.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Kevin Grittner
> Dan Ports  wrote:
> On Mon, Jan 17, 2011 at 07:20:20PM -0600, Kevin Grittner wrote:
>> OK. I may need to bounce some questions off the list to resolve
>> some of them. The biggest, in my mind, is whether
>> MySerializableXact needs to be declared volatile. I don't have my
>> head around the issues on that as well as I might. Any thoughts on
>> it?
> 
> I'd been meaning to ask about that too. I couldn't think of any
> reason it would need to be volatile. Why do you think it might need
> to be?
 
I honestly can't remember why I initially put that there -- possibly
because I had a notion that some modifications to the structure might
not need to be so closely synchronized with other processes as to
need have modifications covered by a LW lock, but not wanting them to
linger forever.  It may be completely bogus.
 
The problem is that I don't have a very clear sense of what it really
does, which is not helped much by having done a few years of Java
programming, where the same keyword seems to have a vaguely-similar-
but-not-really-the-same meaning.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Peter Eisentraut
On mån, 2011-01-17 at 15:33 -0500, Tom Lane wrote:
> Peter Eisentraut  writes:
> > On mån, 2011-01-17 at 07:35 +0100, Magnus Hagander wrote:
> >> In fact, aren't there cases where the *length test* also fails?
> 
> > Currently, two text values are only equal of strcoll() considers them
> > equal and the bits are the same.  So this patch is safe in that regard.
> 
> > There is, however, some desire to loosen this.
> 
> That isn't ever going to happen, unless you'd like to give up hash joins
> and hash aggregation on text values.

Since citext exists and supports hashing, it's obviously possible
nevertheless.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] limiting hint bit I/O

2011-01-17 Thread Jim Nasby
On Jan 14, 2011, at 7:24 PM, Josh Berkus wrote:
> On 1/14/11 11:51 AM, Tom Lane wrote:
>> The people whose tables are mostly insert-only complain about it, but
>> that's not the majority of our userbase IMO.  We just happen to have a
>> couple of particularly vocal ones, like Berkus.
> 
> It might or might not be the majority, but it's an extremely common case
> affecting a lot of users.  Many, if not most, software applications have
> a "log" table (or two, or three) which just accumulates rows, and when
> that log table gets a vacuum freeze it pretty much halts the database in
> its tracks.  Between my client practice and IRC, I run across complaints
> about this issue around 3 times a month.
> 
> And data warehousing is a significant portion of our user base, and
> *all* DW users are affected by this.  In some cases, vacuum issues are
> sufficient to prevent people from using PostgreSQL for data warehousing.

This also affects us every time we stand up a new londiste replica, and I 
expect Slony folks would suffer the same thing. When you copy everything over, 
that's going to happen in a relatively short range of XIDs, so when those XIDs 
start hitting freeze age suddenly *everything* needs to get frozen.

As for hint bits, you're generally not going to have anyone reading from a 
slave that's still being built, so you won't see any hint bit setting until you 
actually open up for users. So for the first X amount of time, performance 
takes a big hit because you have to write all the hints out. Granted, you can 
technically VACUUM FREEZE after the slave is built, but that means more time 
before you can start using the slave and it's something you have to remember to 
do.
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] We need to log aborted autovacuums

2011-01-17 Thread Tom Lane
Josh Berkus  writes:
> On 1/17/11 11:46 AM, Tom Lane wrote:
>> Do we actually need a lock timeout either?  The patch that was being
>> discussed just involved failing if you couldn't get it immediately.
>> I suspect that's sufficient for AV.  At least, nobody's made a
>> compelling argument why we need to expend a very substantially larger
>> amount of work to do something different.

> The argument is that a sufficiently busy table might never get
> autovacuumed *at all*, whereas a small lock wait would allow autovacuum
> to block incoming transactions and start work.

> However, it's hard for me to imagine a real-world situation where a
> table would be under repeated full-table-locks from multiple
> connections.  Can anyone else?

If that is happening, autovacuum is screwed anyway.  Even if it manages
to acquire the lock, it will get kicked off by the next lock request
before it can finish vacuuming the table.  So I don't see an argument
here for adding all the mechanism required to support that.

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] pg_filedump moved to pgfoundry

2011-01-17 Thread Tom Lane
I've gotten permission to move pg_filedump from its former home at
sources.redhat.com to pgfoundry.  You can find the historical release
tarballs as well as current sources at
http://pgfoundry.org/projects/pgfiledump/

One advantage of doing this is it will be a lot easier to let other
folks join in the fun of hacking it.  If anyone has been harboring
a yen to improve pg_filedump, please join that pgfoundry project.

(Before someone suggests folding it into contrib/: we can't because
of license issues.  pg_filedump is GPL, per Red Hat company policy,
and that's not going to change.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_basebackup for streaming base backups

2011-01-17 Thread Fujii Masao
On Mon, Jan 17, 2011 at 10:50 PM, Magnus Hagander  wrote:
>> +       printf(_("  -D, --pgdata=directory   receive base backup into 
>> directory\n"));
>> +       printf(_("  -T, --tardir=directory    receive base backup into tar 
>> files\n"
>> +                        "                            stored in specified 
>> directory\n"));
>> +       printf(_("  -Z, --compress=0-9        compress tar output\n"));
>> +       printf(_("  -l, --label=label         set backup label\n"));
>> +       printf(_("  -p, --progress            show progress information\n"));
>> +       printf(_("  -v, --verbose             output verbose messages\n"));
>>
>> Can we list those options in alphabetical order as other tools do?
>
> Certainly. But it makes more sense to have -D and -T next to each
> other, I think - they'd end up way elsewhere. Perhaps we need a group
> taht says "target"?

I agree with you if we end up choosing -D and -T for a target rather
than pg_dump-like options I proposed.

>> + * Verify that the given directory exists and is empty. If it does not
>> + * exist, it is created. If it exists but is not empty, an error will
>> + * be give and the process ended.
>> + */
>> +static void
>> +verify_dir_is_empty_or_create(char *dirname)
>>
>> Since verify_dir_is_empty_or_create can be called after the connection has
>> been established, it should call PQfinish before exit(1).
>
> Yeah, that's a general thing - do we need to actually bother doing
> that? At most exit() places I haven't bothered free:ing memory or
> closing the connection.
>
> It's not just the PQclear() that you refer to further down - it's also
> all the xstrdup()ed strings for example. Do we really need to care
> about those before we do exit(1)? I think not.

Probably true. The allocated memory would be free'd right after
exit.

> Requiring PQfinish() might be more reasonable since it will give you a
> log on the server if you don't, but I'm not convinced that's necessary
> either?

At least it's required for each password-retry. Otherwise, previous
connections remain during backup. You can see this problem easily
by repeating the password input in pg_basebackup.

LOG:  could not send data to client: Connection reset by peer
LOG:  could not send data to client: Broken pipe
FATAL:  base backup could not send data, aborting backup

As you said, if PQfinish is not called at exit(1), the above messages
would be output. Those messages look ugly and should be
suppressed whenever we *can*. Also I believe other tools would
do that.

>> +       keywords[2] = "fallback_application_name";
>> +       values[2] = "pg_basebackup";
>>
>> Using the progname variable seems better rather than the fixed word
>> "pg_basebackup".
>
> I don't think so - that turns into argv[0], which means that if you
> use the full path of the program (/usr/local/pgsql/bin/pg_basebackup
> for example), the entire path goes into fallback_application_name -
> not just the program name.

No. get_progname extracts the actual name.

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] We need to log aborted autovacuums

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 8:26 PM, Josh Berkus  wrote:
> On 1/17/11 11:46 AM, Tom Lane wrote:
>> Do we actually need a lock timeout either?  The patch that was being
>> discussed just involved failing if you couldn't get it immediately.
>> I suspect that's sufficient for AV.  At least, nobody's made a
>> compelling argument why we need to expend a very substantially larger
>> amount of work to do something different.
>
> The argument is that a sufficiently busy table might never get
> autovacuumed *at all*, whereas a small lock wait would allow autovacuum
> to block incoming transactions and start work.
>
> However, it's hard for me to imagine a real-world situation where a
> table would be under repeated full-table-locks from multiple
> connections.  Can anyone else?

I'm not convinced we need a lock timeout for autovacuum.  I think it'd
be useful to expose on a user-level, but that's a different can of
worms.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] estimating # of distinct values

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 7:56 PM, Jim Nasby  wrote:
> - Forks are very possibly a more efficient way to deal with TOAST than having 
> separate tables. There's a fair amount of overhead we pay for the current 
> setup.

That seems like an interesting idea, but I actually don't see why it
would be any more efficient, and it seems like you'd end up
reinventing things like vacuum and free space map management.

> - Dynamic forks would make it possible to do a column-store database, or at 
> least something approximating one.

I've been wondering whether we could do something like this by
treating a table t with columns pk, a1, a2, a3, b1, b2, b3 as two
tables t1 and t2, one with columns pk, a1, a2, a3 and the other with
columns pk, b1, b2, b3.  SELECT * FROM t would be translated into
SELECT * FROM t1, t2 WHERE t1.pk = t2.pk.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Spread checkpoint sync

2011-01-17 Thread Greg Smith

Bruce Momjian wrote:

Should we be writing until 2:30 then sleep 30 seconds and fsync at 3:00?
  


The idea of having a dead period doing no work at all between write 
phase and sync phase may have some merit.  I don't have enough test data 
yet on some more fundamental issues in this area to comment on whether 
that smaller optimization would be valuable.  It may be a worthwhile 
concept to throw into the sequencing.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] psql: Add \dL to show languages

2011-01-17 Thread Josh Kupershmidt
Hi all,

I've updated the patch to address the following points:
 * help string now says "list procedural languages" (no parentheses now)
 * the language name column is now titled "Name"
 * added another column in verbose mode for 9.0+ showing whether DO
blocks are possible with the language. I named this column "DO
Blocks?", though am open to suggestions
 * fixed the whitespace problems Andreas noticed with the SELECT query

Looking at the verbose output, which looks something like this:

 List of languages
   Name| Owner | Trusted |  Call Handler   |
Validator| System Language | DO Blocks?
 | Access privileges
---+---+-+-++-+---
-+---
 plpgsql   | josh  | t   | plpgsql_call_handler()  |
plpgsql_validator(oid) | f   | t
 |
 plpythonu | josh  | f   | plpython_call_handler() | -
 | f   | t
 |
(2 rows)

I have a hard time imagining users who would find "Call Handler" or
"Validator" useful. This was in Fernando's original patch, and I just
didn't bother to take it out. If others feel the same way, I'd be
happy to rip those columns out.

Few more comments below:

On Mon, Jan 17, 2011 at 3:51 PM, Andreas Karlsson  wrote:
> On Sun, 2011-01-16 at 22:32 -0500, Josh Kupershmidt wrote:
>> On Sat, Jan 15, 2011 at 8:26 PM, Andreas Karlsson  wrote:
>> > Should we include a column in \dL+ for the laninline function (DO
>> > blocks)?
>>
>> Hrm, I guess that could be useful for the verbose output at least.
>
> Magnus Hagander agreed with that idea and added that for that we need to
> check the version. The column was added in 9.0 if I recall.

Added.

[snip]

> * Missing indentation before ACL column, the other functions have it.
> * One space before FROM instead of one newline like the other queries.
> * The space before ORDER BY.

These should be fixed now.

> That's enough nitpickery for now. :)

I spend enough of my time nitpicking others. Turnabout is fair play :)

Thanks for all the review and feedback from everyone.

Josh
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..30d4507 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*** testdb=>
*** 1249,1254 
--- 1249,1269 
  

  
+   
+ \dL[S+] [ pattern ]
+ 
+ 
+ Lists all procedural languages. If pattern
+ is specified, only languages whose names match the pattern are listed.
+ By default, only user-created languages
+ are shown; supply the S modifier to include system
+ objects. If + is appended to the command name, each
+ language is listed with its call handler, validator, access privileges,
+ and whether it is a system object.
+ 
+ 
+   
  

  \dn[S+] [ pattern ]
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..301dc11 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** exec_command(const char *cmd,
*** 416,421 
--- 416,424 
  			case 'l':
  success = do_lo_list();
  break;
+ 			case 'L':
+ success = listLanguages(pattern, show_verbose, show_system);
+ break;
  			case 'n':
  success = listSchemas(pattern, show_verbose, show_system);
  break;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 205190f..5984748 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*** listTables(const char *tabtypes, const c
*** 2566,2571 
--- 2566,2638 
  }
  
  
+ /* \dL
+ *
+ * Describes Languages.
+ */
+ bool
+ listLanguages(const char *pattern, bool verbose, bool showSystem)
+ {
+ 	PQExpBufferData buf;
+ 	PGresult *res;
+ 	printQueryOpt myopt = pset.popt;
+ 
+ 	initPQExpBuffer(&buf);
+ 
+ 	printfPQExpBuffer(&buf,
+ 	  "SELECT l.lanname AS \"%s\",\n",
+ 	  gettext_noop("Name"));
+ 	if (pset.sversion >= 80300)
+ 			appendPQExpBuffer(&buf,
+ 			  "   pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n",
+ 			  gettext_noop("Owner"));
+ 
+ 	appendPQExpBuffer(&buf,
+ 	  "   l.lanpltrusted AS \"%s\"",
+ 	  gettext_noop("Trusted"));
+ 
+ 	if (verbose)
+ 	{
+ 			appendPQExpBuffer(&buf,
+ 			  ",\n   l.lanplcallfoid::regprocedure AS \"%s\",\n"
+ 			  "   l.lanvalidator::regprocedure AS \"%s\",\n"
+ 			  "   NOT l.lanispl AS \"%s\",\n   ",
+ 			  gettext_noop("Call Handler"),
+ 			  gettext_noop("Validator"),
+ 			  gettext_noop("System Language"));
+ 			if (pset.sversion >= 9)
+ appendPQExpBuffer(&buf, "l.laninline != 0 AS \"%s\",\n   ",
+   gettext_noop("DO Blocks?"));
+ 			printACLColumn(&buf, "l.lanacl");			
+ 	}
+ 
+ 	appendPQExpBuffer(&buf,
+ 	  "\nFROM pg_catalog.pg_language l");
+ 
+ 	processSQLNamePatt

Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Greg Smith

Alvaro Herrera wrote:

I don't understand why it would be "overkill".  Are you saying people
would complain because you installed a 25 kB executable that they might
not want to use?  Just for fun I checked /usr/bin and noticed that I
have a "pandoc" executable, weighing 17 MB, that I have never used and I
have no idea what is it for.
  


It's for converting between the various types of text-like markup, i.e. 
reST, LaTex, Markdown, etc.


Anyway, just because the rest of the world has no standards anymore 
doesn't mean we shouldn't.  The changes Bruce has made recently have 
gotten this tool to where its output is starting to be readable and 
reliable.  The sort of people who want to run this will certainly be 
fine with installing contrib to do it, because they may want to have 
things like pgbench too.  There's really not enough demand for this to 
pollute the default server install footprint with any overhead from this 
tool, either in bytes or increased tool name squatting.  And the fact 
that it's still a little rough around the edges nudges away from the 
standard server package too.


Install in contrib as pg_test_fsync and I think you'll achieve the 
optimal subset of people who can be made happy here.  Not having it 
packaged at all before wasn't a big deal, because it was so hard to 
collect good data from only developer-level people were doing it 
anyway.  Now that it is starting to be more useful in that role for less 
experienced users, we need to make it easier for more people to run it, 
to collect feedback toward further improving its quality.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] test_fsync open_sync test

2011-01-17 Thread Greg Smith

Bruce Momjian wrote:

Is there a value to this test_fsync test?

Compare open_sync with different sizes:
(This is designed to compare the cost of one large
sync'ed write and two smaller sync'ed writes.)
open_sync 16k write   242.563 ops/sec
2 open_sync 8k writes 752.752 ops/sec

It compares the cost of doing larger vs. two smaller open_sync writes.
  


Might be some value for determining things like what the optimal WAL 
block size to use is.  All these tests are kind of hard to use 
effectively still, I'm not sure if it's time to start trimming tests yet 
until we've made more progress on interpreting results first.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] We need to log aborted autovacuums

2011-01-17 Thread Josh Berkus
On 1/17/11 11:46 AM, Tom Lane wrote:
> Do we actually need a lock timeout either?  The patch that was being
> discussed just involved failing if you couldn't get it immediately.
> I suspect that's sufficient for AV.  At least, nobody's made a
> compelling argument why we need to expend a very substantially larger
> amount of work to do something different.

The argument is that a sufficiently busy table might never get
autovacuumed *at all*, whereas a small lock wait would allow autovacuum
to block incoming transactions and start work.

However, it's hard for me to imagine a real-world situation where a
table would be under repeated full-table-locks from multiple
connections.  Can anyone else?

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.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] SSI patch version 12

2011-01-17 Thread Dan Ports
On Mon, Jan 17, 2011 at 07:20:20PM -0600, Kevin Grittner wrote:
> OK.  I may need to bounce some questions off the list to resolve some
> of them.  The biggest, in my mind, is whether MySerializableXact
> needs to be declared volatile.  I don't have my head around the
> issues on that as well as I might.  Any thoughts on it?

I'd been meaning to ask about that too. I couldn't think of any reason
it would need to be volatile. Why do you think it might need to be?

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
Sent 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: compact fsync request queue on overflow

2011-01-17 Thread Greg Smith

Chris Browne wrote:

  It was a little troublesome inducing it.  I did so by cutting
  shared memory to minimum (128kB)...
  With higher shared memory, I couldn't readily induce compaction,
  which is probably a concurrency matter of not having enough volume
  of concurrent work going on.
  


Quite.  It's taken me 12 days of machine time running pgbench to find 
the spots where this problem occurs on a system with a reasonably sized 
shared_buffers (I'm testing against 256MB).  It's one of those things 
it's hard to reproduce with test data.


Thanks for the thorough code review.  I've got a clear test plan I'm 
progressing through this week to beat on the performance measurement 
aspects of the patch.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Kevin Grittner
> Heikki Linnakangas  wrote:
 
> Setting the high bit in OldSetXidAdd() seems a bit weird. How about
> just using UINT64_MAX instead of 0 to mean no conflicts? Or 1, and
> start the sequence counter from 2.
 
Sure.  I think I like reserving 1 and starting at 2 better.  Will do.
 
> ReleasePredicateLocks() reads ShmemVariableCache->nextXid without
> XidGenLock. Maybe it's safe, we assume that TransactionIds are
> atomic elsewhere, but at least there needs to be a comment
> explaining it. But it probably should use ReadNewTransactionId()
> instead.
 
Hmm...  We don't want to *assign* a new xid here -- we just want to
know what the next one *will be*.  I will review locking around this
area, but I think it's sound.  If that checks out OK on review, I'll
add comments.
 
> Attached is a patch for some trivial changes, mostly typos.
 
Wow.  I transpose letters a lot don't I?  Thanks for finding those.
Will fix.
 
> Overall, this is very neat and well-commented code. All the data
> structures make my head spin, but I don't see anything unnecessary
> or have any suggestions for simplification. There's a few remaining
> TODO comments in the code, which obviously need to be resolved one
> way or another, but as soon as you're finished with any outstanding
> issues that you know of, I think this is ready for commit.
 
OK.  I may need to bounce some questions off the list to resolve some
of them.  The biggest, in my mind, is whether MySerializableXact
needs to be declared volatile.  I don't have my head around the
issues on that as well as I might.  Any thoughts on it?
 
Thanks for the multiple reviews and all the suggestions.  The code is
clearly better for your attention.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Dan Ports
On Mon, Jan 17, 2011 at 06:52:09PM -0600, Kevin Grittner wrote:
> I think we still need the vxid.  It shows in the pg_locks view, and
> we might possibly need it to find the right process to cancel once we
> have some way to do that.  But there's no point with having the tag
> level anymore.

Oh, right, we do use it to generate pg_lock_status. I forgot about that
particular tendril of code that extends outside of predicate.c.

> Dan, if you're not already working on these, I'll take them, so you
> can focus on 2PC.

All yours! But if you feel like renaming SerializableXactHashLock,
please hold off to prevent conflicts with my 2PC changes.

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] estimating # of distinct values

2011-01-17 Thread Jim Nasby
On Jan 17, 2011, at 6:36 PM, Tomas Vondra wrote:
> 1) Forks are 'per relation' but the distinct estimators are 'per
>   column' (or 'per group of columns') so I'm not sure whether the file
>   should contain all the estimators for the table, or if there should
>   be one fork for each estimator. The former is a bit difficult to
>   manage, the latter somehow breaks the current fork naming convention.

Yeah, when I looked at the fork stuff I was disappointed to find out there's 
essentially no support for dynamically adding forks. There's two other possible 
uses for that I can think of:

- Forks are very possibly a more efficient way to deal with TOAST than having 
separate tables. There's a fair amount of overhead we pay for the current setup.
- Dynamic forks would make it possible to do a column-store database, or at 
least something approximating one.

Without some research, there's no way to know if either of the above makes 
sense; but without dynamic forks we're pretty much dead in the water.

So I wonder what it would take to support dynamically adding forks...
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent 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: PL/Python validator function

2011-01-17 Thread Jan Urbański
On 17/01/11 09:26, Jan Urbański wrote:
> On 17/01/11 01:02, Hitoshi Harada wrote:
>> This is a review for the patch sent as
>> https://commitfest.postgresql.org/action/patch_view?id=456
>> It includes adequate amount of test. I found regression test failure
>> in plpython_error.
> 
>> My environment is CentOS release 5.4 (Final) with python 2.4.3
>> installed default.

Seems that somewhere between Python 2.4 and Python 2.6 the whole module
that was providing SyntaxError got rewritten and the way a syntax error
from Py_CompileString is reported changed :( I tried some tricks but in
the end I don't think it's worth it: I just added an alternative
regression output file for older Pythons.

>> It looks fine overall. The only thing that I came up with is trigger
>> check logic in PLy_procedure_is_trigger. Although it seems following
>> plperl's corresponding function, the check of whether the prorettype
>> is pseudo type looks redundant since it checks prorettype is
>> TRIGGEROID or OPAQUEOID later. But it is not critical.
> 
> Yes, you're right, a check for prorettype only should be sufficient. Wil
> fix.

I removed the test for TYPTYPE_PSEUDO in the is_trigger function.

Updated patch attached.

Cheers,
Jan
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index d987ddc..c0578f9 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** DATA(insert ( "pltcl"		t t "pltcl_call_h
*** 72,79 
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plpythonu"	f f "plpython_call_handler" "plpython_inline_handler" _null_ "$libdir/plpython" _null_ ));
! DATA(insert ( "plpython2u"	f f "plpython_call_handler" "plpython_inline_handler" _null_ "$libdir/plpython2" _null_ ));
! DATA(insert ( "plpython3u"	f f "plpython3_call_handler" "plpython3_inline_handler" _null_ "$libdir/plpython3" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 72,79 
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plpythonu"	f f "plpython_call_handler" "plpython_inline_handler" "plpython_validator" "$libdir/plpython" _null_ ));
! DATA(insert ( "plpython2u"	f f "plpython_call_handler" "plpython_inline_handler" "plpython_validator" "$libdir/plpython2" _null_ ));
! DATA(insert ( "plpython3u"	f f "plpython3_call_handler" "plpython3_inline_handler" "plpython3_validator" "$libdir/plpython3" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plpython/expected/README b/src/pl/plpython/expected/README
index f011528..27c995d 100644
*** a/src/pl/plpython/expected/README
--- b/src/pl/plpython/expected/README
***
*** 1,5 
--- 1,7 
  Guide to alternative expected files:
  
+ plpython_error_0.out		Python 2.4 and older
+ 
  plpython_unicode.out		any version, when server encoding != SQL_ASCII and client encoding = UTF8; else ...
  plpython_unicode_0.out		any version, when server encoding != SQL_ASCII and client encoding != UTF8; else ...
  plpython_unicode_2.out		Python 2.2
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 70890a8..a9e1f15 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
***
*** 1,6 
--- 1,30 
  -- test error handling, i forgot to restore Warn_restart in
  -- the trigger handler once. the errors and subsequent core dump were
  -- interesting.
+ /* Flat out Python syntax error
+  */
+ CREATE FUNCTION python_syntax_error() RETURNS text
+ AS
+ '.syntaxerror'
+ LANGUAGE plpythonu;
+ ERROR:  could not compile PL/Python function "python_syntax_error"
+ DETAIL:  SyntaxError: invalid syntax (, line 2)
+ /* With check_function_bodies = false the function should get defined
+  * and the error reported when called
+  */
+ SET check_function_bodies = false;
+ CREATE FUNCTION python_syntax_error() RETURNS text
+ AS
+ '.syntaxerror'
+ LANGUAGE plpythonu;
+ SELECT python_syntax_error();
+ ERROR:  could not compile PL/Python function "python_syntax_error"
+ DETAIL:  SyntaxError: invalid syntax (, line 2)
+ /* Run the function twice to check if the hashtable entry gets cleaned up */
+ SELECT python_syntax_error();
+ ERROR:  could not compile PL/Python function "python_syntax_error"
+ DETAIL:  SyntaxError: invalid syntax (, line 2)
+ RESET check_function_bodies;
  /* Flat out

Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Kevin Grittner
> Dan Ports  wrote:
 
> Yes, that comment was supposed to be attached to
> possibleUnsafeConflicts.
 
> Actually, I think that "other" hash no longer exists
 
> The comment above SERIALIZABLEXACT also needs to be updated since
> it refers to said hash table. And if I'm not mistaken (Kevin?), we
> can eliminate SERIALIZABLEXACTTAG altogether and not bother putting
> the vxid in the sxact.
 
I think we still need the vxid.  It shows in the pg_locks view, and
we might possibly need it to find the right process to cancel once we
have some way to do that.  But there's no point with having the tag
level anymore.
 
Otherwise, I agree with Dan on all counts.
 
Dan, if you're not already working on these, I'll take them, so you
can focus on 2PC.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] estimating # of distinct values

2011-01-17 Thread Tomas Vondra
Dne 9.1.2011 13:58, Jim Nasby napsal(a):
>> A resource fork? Not sure what you mean, could you describe it in more
>> detail?
> 
> Ooops, resource forks are a filesystem thing; we call them relation forks. 
> >From src/backend/storage/smgr/README:

OK, I think using relation forks seems like a good solution. I've done
some basic research and I think these are the basic steps when adding a
new fork:

1) define a new item in the ForkNum enum (relfilenode.h) - this should
   be somethink like DISTINCT_FORK I guess

2) modify the ForkNames (catalog.c) and the relpathbackend so that the
   proper filename is assigned to the fork

And then it will be accessed through smgr (smgrcreate etc.). Am I right
or is there something else I need to do?

There are a few open questions though:

1) Forks are 'per relation' but the distinct estimators are 'per
   column' (or 'per group of columns') so I'm not sure whether the file
   should contain all the estimators for the table, or if there should
   be one fork for each estimator. The former is a bit difficult to
   manage, the latter somehow breaks the current fork naming convention.

2) Where to keep the info that there is an estimator for a column? I
   guess we could put this into pg_attribute (it's one boolean). But
   what about the estimators for groups of columns? Because that's why
   I'm building this - to get distinct estimates for groups of columns.

   I guess we'll need a new system catalog to track this? (The same
   will be true for multi-dimensional histograms anyway).

3) I still am not sure how to manage the updates, i.e. how to track the
   new values.

   One possibility might be to do that synchronously - whenever a new
   item is inserted into the table, check if there's an estimator and
   update it. Updating the estimators is quite efficient (e.g. the
   bitmap manages to do 10.000.000 inserts in 9 seconds on my ancient
   workstation) although there might be issues with locking etc.

   The other possibility is to update the estimator asynchronously, i.e.
   store the new values somewhere (or just ctid of the row), and then
   process it periodically. I'm not sure how to intercept the new rows
   and where to store them. In another fork? Somewhere else?

regards
Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Spread checkpoint sync

2011-01-17 Thread Greg Smith

Jim Nasby wrote:

Wow, that's the kind of thing that would be incredibly difficult to figure out, 
especially while your production system is in flames... Can we change ereport 
that happens in that case from DEBUG1 to WARNING? Or provide some other means 
to track it


That's why we already added pg_stat_bgwriter.buffers_backend_fsync to 
track the problem before trying to improve it.  It was driving me crazy 
on a production server not having any visibility into when it happened.  
I haven't seen that we need anything beyond that so far.  In the context 
of this new patch for example, if you get to where a backend does its 
own sync, you'll know it did a compaction as part of that.  The existing 
statistic would tell you enough.


There's now enough data in test set 3 at 
http://www.2ndquadrant.us/pgbench-results/index.htm to start to see how 
this breaks down on a moderately big system (well, by most people's 
standards, but not Jim for whom this is still a toy).  Note the 
backend_sync column on the right, very end of the page; that's the 
relevant counter I'm commenting on:


scale=175:  Some backend fsync with 64 clients, 2/3 runs.
scale=250:  Significant backend fsync with 32 and 64 clients, every run.
scale=500:  Moderate to large backend fsync at any client count >=16.  
This seems to be worst spot of those mapped.  Above here, I would guess 
the TPS numbers start slowing enough that the fsync request queue 
activity drops, too.

scale=1000:  Backend fsync starting at 8 clients
scale=2000:  Backend fsync starting at 16 clients.  By here I think the 
TPS volumes are getting low enough that clients are stuck significantly 
more often waiting for seeks rather than fsync.


Looks like the most effective spot for me to focus testing on with this 
server is scales of 500 and 1000, with 16 to 64 clients.  Now that I've 
got the scale fine tuned better, I may crank up the client counts too 
and see what that does.  I'm glad these are appearing in reasonable 
volume here though, was starting to get nervous about only having NDA 
restricted results to work against.  Some days you just have to cough up 
for your own hardware.


I just tagged pgbench-tools-0.6.0 and pushed to 
GitHub/git.postgresql.org with the changes that track and report on 
buffers_backend_fsync if anyone else wants to try this out.  It includes 
those numbers if you have a 9.1 with them, otherwise just reports 0 for 
it all the time; detection of the feature wasn't hard to add.  The end 
portion of a config file for the program (the first part specifies 
host/username info and the like) that would replicate the third test set 
here is:


MAX_WORKERS="4"
SCRIPT="tpc-b.sql"
SCALES="1 10 100 175 250 500 1000 2000"
SETCLIENTS="4 8 16 32 64"
SETTIMES=3
RUNTIME=600
TOTTRANS=""

--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SQL/MED - file_fdw

2011-01-17 Thread Kevin Grittner
Itagaki Takahiro  wrote:
> Shigeru HANADA  wrote:
 
>> Attached patch would avoid this leak by adding per-copy context to
>> CopyState. This would be overkill, and ResetCopyFrom() might be
>> reasonable though.
>
> Good catch. I merged your fix into the attached patch.
 
Review for CF:
 
While there is a post which suggests applying this patch in the
middle of a string of fdw patches, it stands alone without depending
on any of the other patches.  I chose to focus on testing it alone,
to isolate just issues with this particular patch.  In addition to
the base patch, there was a patch-on-patch which was applied to
change signature of NextCopyFrom to take fewer params and return
HeapTuple.
 
This patch is in context diff format, applies cleanly, compiles
without warning, passes all tests in `make check` and `make
installcheck-world`.  From that and the testing I did, I think that
this patch could be committed before any of the others, if desired.
 
A few minor comments on format, though -- some parts of the patch
came out much more readable (for me, at least) when I regenerated the
diff using the git diff --patience switch.  For example, see the end
of the CopyStateData structure definition each way.  Also, whitespace
has little differences from what pgindent uses.  Most are pretty
minor except for a section where the indentation wasn't changed based
on a change in depth of braces, to keep the patch size down.
 
It appears that the point of the patch is to provide a better API for
accessing the implementation of COPY, to support other patches.  This
whole FDW effort is not an area of expertise for me, but the API
looks reasonable to me, with a somewhat parallel structure to some of
the other APIs used by the executor.  From reading the FDW posts, it
appears that other patches are successfully using this new API, and
the reviewers of the other patches seem happy enough with it, which
would tend to indicate that it is on-target.  Other hackers seem to
want it and we didn't already have it.  From my reading of the posts
the idea of creating an API at this level was agreed upon by the
community.
 
The only two files touched by this patch are copy.h and copy.c.  The
copy.h changes consisted entirely of new function prototypes and one
declaration of a pointer type (CopyState) to a struct defined and
used only inside copy.c (CopyStateData).  That pointer is the return
value from BeginCopyFrom and required as a parameter to other new
functions.  So the old API is still present exactly as it was, with
additional functions added.
 
I tried to read through the code to look for problems, but there are
so many structures and techniques involved that I haven't had to deal
with yet, that it would take me days to get up to speed enough to
desk check this adequately.  Since this API is a prerequisite for
other patches, and already being used by them, I figured I should do
what I could quickly and then worry about how to cover that.
 
Since it doesn't appear to be intended to change any user-visible
behavior, I don't see any need for docs or changes to the regression
tests.  I didn't see any portability problems.  It seemed a little
light on comments to me, but perhaps that's because of my ignorance
of some of the techniques being used -- maybe things are obvious
enough to anyone who would be mucking about in copy.c.
 
I spent a few hours doing ad hoc tests of various sorts to try to
break things, without success.  Among those tests were checks for
correct behavior on temporary tables and read only transactions --
separately and in combination.  I ran millions of COPY statements
inside of a single database transaction (I tried both FROM and TO)
without seeing memory usage increase by more than a few kB.  No
assertion failures.  No segfaults.  Nothing unusual in the log.  I
plan to redo these tests with the full fdw patch set unless someone
has already covered that ground.
 
So far everything I've done has been with asserts enabled, so I
haven't tried to get serious benchmarks, but it seems fast.  I will
rebuild without asserts and do performance tests before I change the
status on the CF page.
 
I'm wondering if it would make more sense to do the benchmarking with
just this patch or the full fdw patch set?  Both?
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Spread checkpoint sync

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 6:07 PM, Jim Nasby  wrote:
> On Jan 15, 2011, at 8:15 AM, Robert Haas wrote:
>> Well, the point of this is not to save time in the bgwriter - I'm not
>> surprised to hear that wasn't noticeable.  The point is that when the
>> fsync request queue fills up, backends start performing an fsync *for
>> every block they write*, and that's about as bad for performance as
>> it's possible to be.  So it's worth going to a little bit of trouble
>> to try to make sure it doesn't happen.  It didn't happen *terribly*
>> frequently before, but it does seem to be common enough to worry about
>> - e.g. on one occasion, I was able to reproduce it just by running
>> pgbench -i -s 25 or something like that on a laptop.
>
> Wow, that's the kind of thing that would be incredibly difficult to figure 
> out, especially while your production system is in flames... Can we change 
> ereport that happens in that case from DEBUG1 to WARNING? Or provide some 
> other means to track it?

Something like this?

http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=3134d8863e8473e3ed791e27d484f9e548220411

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Josh Berkus

> Also, it's not going to get packaged at all unless it gets renamed to
> something less generic, maybe pg_test_fsync; I'm not going to risk the
> oppobrium of sticking something named "test_fsync" into /usr/bin.
> Moving to contrib would be a good opportunity to fix the name.

+1.

It would be a lot easier to tell people on -performance to use this if
it was in contrib, for that matter.  "Go to *what* directory?"


-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.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] WIP: RangeTypes

2011-01-17 Thread Josh Berkus
On 1/17/11 1:09 PM, Jeff Davis wrote:
> I feel like I'm making this too complicated. Should I just scope out
> NULL range boundaries for the first cut, and leave room in the
> representation so that it can be added when there is a more thorough
> proposal for NULL range boundaries?

Well, NULL range boundaries aren't usable with Temporal, and yet I wrote
a whole scheduling application around it.  So I think it's OK to have
them as a TODO and raise an error for now.  Heck, we had arrays which
didn't accept NULLs for years.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.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] WIP: RangeTypes

2011-01-17 Thread Tom Lane
Jeff Davis  writes:
> I feel like I'm making this too complicated. Should I just scope out
> NULL range boundaries for the first cut, and leave room in the
> representation so that it can be added when there is a more thorough
> proposal for NULL range boundaries?

+1.  I'm far from convinced that a null boundary is sane at all.
If you don't know the value, how do you know it's greater/less than the
other bound?

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] Visual Studio 2010/Windows SDK 7.1 support

2011-01-17 Thread Brar Piening

On Tue, 18 Jan 2011 00:06:10 +0100, Brar Piening  wrote:


So there is now a third version of this patch at 
http://www.piening.info/VS2010v3.patch




Forgot to run perltidy on it - fixed in 
http://www.piening.info/VS2010v4.patch


Sorry!

Brar

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Spread checkpoint sync

2011-01-17 Thread Jim Nasby
On Jan 15, 2011, at 8:15 AM, Robert Haas wrote:
> Well, the point of this is not to save time in the bgwriter - I'm not
> surprised to hear that wasn't noticeable.  The point is that when the
> fsync request queue fills up, backends start performing an fsync *for
> every block they write*, and that's about as bad for performance as
> it's possible to be.  So it's worth going to a little bit of trouble
> to try to make sure it doesn't happen.  It didn't happen *terribly*
> frequently before, but it does seem to be common enough to worry about
> - e.g. on one occasion, I was able to reproduce it just by running
> pgbench -i -s 25 or something like that on a laptop.

Wow, that's the kind of thing that would be incredibly difficult to figure out, 
especially while your production system is in flames... Can we change ereport 
that happens in that case from DEBUG1 to WARNING? Or provide some other means 
to track it?
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Visual Studio 2010/Windows SDK 7.1 support

2011-01-17 Thread Brar Piening

Hi,

I initially considered this patch as a primer to start off some basic 
VS2010 support and not as something to be commited within the next few 
commitfests.


As there seems to be at least some interest in this patch I refactored 
the code and did some more testing (actually found some weird issues 
with pgbison.bat and pgflex.bat and replaced them with perl variants).


So there is now a third version of this patch at 
http://www.piening.info/VS2010v3.patch


This time even with unix linefeeds ;-)

On Sat, 15 Jan 2011 19:42:06 +0100, Magnus Hagander 
 wrote:
Please make sure this goes on the commitfest page 
(https://commitfest.postgresql.org/action/commitfest_view?id=9), so 
it's not missed.


I'll add it.

Regards,

Brar

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_dump directory archive format / parallel pg_dump

2011-01-17 Thread Jaime Casanova
On Fri, Jan 7, 2011 at 3:18 PM, Joachim Wieland  wrote:
> Here's a new series of patches for the parallel dump/restore. They need to be
> applied on top of each other.
>

This one is the last version of this patch? if so, commitfest app
should be updated to reflect that

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Warning compiling pg_dump (MinGW, Windows XP)

2011-01-17 Thread Andrew Dunstan



On 01/17/2011 03:51 PM, Tom Lane wrote:

Andrew Dunstan  writes:

On 01/17/2011 07:18 AM, Pavel Golub wrote:

So you think I should just ignore these warnings? Because I can't
remember the same behaviour on 8.x branches...

We've had them all along, as I said. See

for the same thing in an 8.2 build.

I wonder why mingw's gcc is complaining about %m when other versions of
gcc do not?  If you can't get it to shut up about that, there's not
going to be much percentage in silencing warnings about %lld.




We could add -Wno-format to the flags. That makes it shut up, but maybe 
we don't want to use such a sledgehammer.


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] SSI patch version 12

2011-01-17 Thread Dan Ports
On Mon, Jan 17, 2011 at 09:58:44PM +0200, Heikki Linnakangas wrote:
> What does that comment about list of concurrent r/w transactions refer 
> to? I don't see any list there. Does it refer to 
> possibleUnsafeConflicts, which is above that comment?

Yes, that comment was supposed to be attached to
possibleUnsafeConflicts. It appears to have wandered down a couple
lines, maybe during some combination of git merges and pgindent runs.

> Above SERIALIZABLEXID struct:
> >  * A hash table of these objects is maintained in shared memory to provide a
> >  * quick way to find the top level transaction information for a 
> > serializable
> >  * transaction,  Because a serializable transaction can acquire a snapshot
> >  * and read information which requires a predicate lock before it has a
> >  * TransactionId, it must be keyed by VirtualTransactionId; this hashmap
> >  * allows a fast link from MVCC transaction IDs to the related serializable
> >  * transaction hash table entry.
> 
> I believe the comment is trying to say that there's some *other* hash 
> that is keyed by VirtualTransactionId, so we need this other one keyed 
> by TransactionId. It took me a while to understand that, it should be 
> rephrased.

Actually, I think that "other" hash no longer exists, it got replaced
with a list because we weren't actually using vxid -> sxact lookup. So
the comment appears to be both confusing and inaccurate and should be
removed entirely, other than to note somewhere that not every
SERIALIZABLEXACT will appear in SerializableXidHash because it might
not have a TransactionId. 

The comment above SERIALIZABLEXACT also needs to be updated since it
refers to said hash table. And if I'm not mistaken (Kevin?), we can
eliminate SERIALIZABLEXACTTAG altogether and not bother putting the
vxid in the sxact.

While we're at it, it probably wouldn't hurt to rename
SerializableXactHashLock to PredTranLock or something, since there's no
SerializableXactHash anymore (although the lock is still being used
correctly)

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] WIP: RangeTypes

2011-01-17 Thread David Fetter
On Mon, Jan 17, 2011 at 01:09:26PM -0800, Jeff Davis wrote:
> When defining generic range functions, there is quite a bit of extra
> complexity needed to handle special cases.
> 
> The special cases are due to:
>  * empty ranges
>  * ranges with infinite boundaries
>  * ranges with NULL boundaries
>  * ranges with exclusive bounds (e.g. "(" or ")").
> 
> Infinite bounds, and exclusive bounds can both be handled somewhat
> reasonably, and the complexity can be somewhat hidden.  Empty ranges
> are a special case, but can be handled at the top of the generic
> function in a straightforward way.
> 
> NULL bounds, however, have been causing me a little frustration.
> [Explanation and illustrations].

In that case, let's leave them out for this cut.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Noah Misch
On Mon, Jan 17, 2011 at 02:36:56PM -0600, Jim Nasby wrote:
> On Jan 17, 2011, at 9:22 AM, Noah Misch wrote:
> > Just to be clear, the code already has these length tests today.  This patch
> > just moves them before the detoast.
> 
> Any reason we can't do this for other varlena? I'm wondering if it makes more 
> sense to have some generic toast comparison functions that don't care what 
> the data in toast actually is...

We could not apply the optimization to varlenas generically.  For example,
bpchareq() ignores trailing white space during comparison, so "foo " = "foo  ".
It would work for biteq(), though I'm not sure how often large-scale varbits
come up.  numericeq() does not qualify, because you might have a NumericLong in
a binary-upgraded table that would now become a NumericShort.  So, there very
well may be other places where we should apply the same optimization, but each
one needs individual consideration.

Thanks,
nm

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] WIP: RangeTypes

2011-01-17 Thread Jeff Davis
When defining generic range functions, there is quite a bit of extra
complexity needed to handle special cases.

The special cases are due to:
 * empty ranges
 * ranges with infinite boundaries
 * ranges with NULL boundaries
 * ranges with exclusive bounds (e.g. "(" or ")").

Infinite bounds, and exclusive bounds can both be handled somewhat
reasonably, and the complexity can be somewhat hidden. Empty ranges are
a special case, but can be handled at the top of the generic function in
a straightforward way.

NULL bounds, however, have been causing me a little frustration. A
reasonable interpretation of boolean operators that operate on ranges
might be: "true or false if we can prove it from only the inputs; else
NULL". This gets a little interesting because a NULL value as a range
boundary isn't 100% unknown: it's known to be on one side of the other
bound (assuming that the other side is known). This is similar to how
AND and OR behave for NULL. For instance, take the simple definition of
"contains":

   r1.a <= r2.a AND r1.b >= r2.b

(where "a" is the lower bound and "b" is the upper)

Consider r1: [NULL, 10], r2: [20, NULL]. Contains should return "false"
according to our rule above, because no matter what the values of r1.a
and r2.b, the ranges can't possibly overlap.

So, now, more complexity needs to be added. We can be more redundant and
do:

  r1.a <= r2.a AND r1.b <= r2 AND r1.a <= r2.b AND r1.b >= r2.a

That seems a little error-prone and harder to understand.

Then, when we have functions that operate on ranges and return ranges,
we're not dealing with 3VL exactly, but some other intuition about what
NULL should do. The semantics get a lot more complicated and hard to
reason about. For instance, what about:
  (NULL, 5) INTERSECT (3, NULL)
Should that evaluate to NULL, (NULL, NULL), or throw an error? What
about:
  (NULL, 5) MINUS (NULL, 7) 
  (NULL, 5) MINUS (3, NULL)

I feel like I'm making this too complicated. Should I just scope out
NULL range boundaries for the first cut, and leave room in the
representation so that it can be added when there is a more thorough
proposal for NULL range boundaries?

Regards,
Jeff Davis


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Bug in pg_describe_object, patch v2

2011-01-17 Thread Joel Jacobson
2011/1/17 Tom Lane :
> Joel Jacobson  writes:
>> a) pg_describe_object should always include the schema in the name,
>> even for object in public and pg_catalog.
>
> I knew you were going to demand that next, as soon as you figured out
> that it was an obstacle for using pg_describe_object output as a
> globally unique identifier.  But I'm going to reply, once again, that
> pg_describe_object is not meant to guarantee that and we're not going to
> make it so.

I knew you were going to say that, but I'm going to ask, once again,
is it possible to introduce another function, possibly returning a
differently formatted string, perhaps in JSON, or any other structured
format, suitable to be parsed by an application and which keys are
sorted in a way which guarantees uniqueness? Perhaps it could be named
pg_get_object_identifier or something like that.

It's a huge limitation when designing the type of application I'm
designing, when you need to invent your own solution to the problem,
to avoid the dependency on oids, which are not possible to use across
different databases.

Perhaps I'm the only one working with a project where a unique
identifier for each object is an absolute requirement, but it sounds
unlikely, quite a lot of developers must have thought about these
things before.

-- 
Best regards,

Joel Jacobson
Glue Finance

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] We need to log aborted autovacuums

2011-01-17 Thread Tom Lane
Simon Riggs  writes:
> On Mon, 2011-01-17 at 14:46 -0500, Tom Lane wrote:
>> Do we actually need a lock timeout either?  The patch that was being
>> discussed just involved failing if you couldn't get it immediately.
>> I suspect that's sufficient for AV.  At least, nobody's made a
>> compelling argument why we need to expend a very substantially larger
>> amount of work to do something different.

> I have a patch to do this BUT the log message is harder. Until we grab
> the lock we can't confirm the table still exists, so we can't get the
> name for it. The whole point of this is logging the names of tables for
> which we have failed to AV. Suggestions?

As I said before, the correct place to fix this is in autovacuum.c,
which has the table name near at hand.  If you insist on fixing it
somewhere else it's going to be very invasive.

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] Bug in pg_describe_object, patch v2

2011-01-17 Thread Dimitri Fontaine
Tom Lane  writes:

> Joel Jacobson  writes:
>> a) pg_describe_object should always include the schema in the name,
>> even for object in public and pg_catalog.
>
> I knew you were going to demand that next, as soon as you figured out
> that it was an obstacle for using pg_describe_object output as a
> globally unique identifier.  But I'm going to reply, once again, that
> pg_describe_object is not meant to guarantee that and we're not going to
> make it so.

And it's easy to pretend it's already coded this way if you're motivated
enough.  Just find a schema name that's not already existing:

dim=# set search_path to public, utils;
SET
dim=# select pg_describe_object('pg_proc'::regclass, 16602, 0);
  pg_describe_object   
---
 function unaccent_lexize(internal,internal,internal,internal)
(1 row)

dim=# begin; create schema dummy; set local search_path to 'dummy'; select 
pg_describe_object('pg_proc'::regclass, 16602, 0); rollback;
BEGIN
CREATE SCHEMA
SET
 pg_describe_object  
-
 function utils.unaccent_lexize(internal,internal,internal,internal)
(1 row)

ROLLBACK

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Bug in pg_describe_object, patch v2

2011-01-17 Thread Andreas Karlsson
On Sun, 2011-01-16 at 14:28 -0500, Tom Lane wrote:
> One other point here is that I find messages like this a mite
> unreadable:
> 
> function 1 (oidvector[], oidvector[]) btoidvectorcmp(oidvector,oidvector) of 
> operator family array_ops for access method gin
> 
> If we were to go with this, I'd be strongly tempted to rearrange all
> four of the messages involved to put the operator or function name
> at the end, eg
> 
> function 1 (oidvector[], oidvector[]) of operator family array_ops for access 
> method gin: btoidvectorcmp(oidvector,oidvector)

Yes, I agree with you that the second is much more readable with out
without the lefttype and righttype.

function 1 of operator family array_ops for access method gin: 
btoidvectorcmp(oidvector,oidvector)

is more readable in my opinion than,

function 1 btoidvectorcmp(oidvector,oidvector) of operator family array_ops for 
access method gin

Regards,
Andreas



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Warning compiling pg_dump (MinGW, Windows XP)

2011-01-17 Thread Tom Lane
Andrew Dunstan  writes:
> On 01/17/2011 07:18 AM, Pavel Golub wrote:
>> So you think I should just ignore these warnings? Because I can't
>> remember the same behaviour on 8.x branches...

> We've had them all along, as I said. See 
> 
>  
> for the same thing in an 8.2 build.

I wonder why mingw's gcc is complaining about %m when other versions of
gcc do not?  If you can't get it to shut up about that, there's not
going to be much percentage in silencing warnings about %lld.

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] psql: Add \dL to show languages

2011-01-17 Thread Andreas Karlsson
On Sun, 2011-01-16 at 22:32 -0500, Josh Kupershmidt wrote:
> On Sat, Jan 15, 2011 at 8:26 PM, Andreas Karlsson  wrote:
> > Should we include a column in \dL+ for the laninline function (DO
> > blocks)?
> 
> Hrm, I guess that could be useful for the verbose output at least.

Magnus Hagander agreed with that idea and added that for that we need to
check the version. The column was added in 9.0 if I recall.

> > SELECT l.lanname AS "Procedural Language",
> >   pg_catalog.pg_get_userbyid(l.lanowner) as "Owner",
> >   l.lanpltrusted AS "Trusted",
> >   l.lanplcallfoid::regprocedure AS "Call Handler",
> >   l.lanvalidator::regprocedure AS "Validator",
> >   NOT l.lanispl AS "System Language",
> > pg_catalog.array_to_string(l.lanacl, E'\n') AS "Access privileges" FROM 
> > pg_catalog.pg_language l
> >  ORDER BY 1;
> 
> Sorry, I don't understand.. what's wrong with the formatting of this
> query? In terms of whitespace, it looks pretty similar to
> listDomains() directly below listLanguages() in describe.c.

It is quite similar, so the general style is correct. Just some errors
in the details. Here are the ones I see in the example above, but there
could be others in other code paths.

* Missing indentation before ACL column, the other functions have it.
* One space before FROM instead of one newline like the other queries.
* The space before ORDER BY.

That's enough nitpickery for now. :)

Regards,
Andreas



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Bug in pg_describe_object, patch v2

2011-01-17 Thread Tom Lane
Joel Jacobson  writes:
> a) pg_describe_object should always include the schema in the name,
> even for object in public and pg_catalog.

I knew you were going to demand that next, as soon as you figured out
that it was an obstacle for using pg_describe_object output as a
globally unique identifier.  But I'm going to reply, once again, that
pg_describe_object is not meant to guarantee that and we're not going to
make it so.

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] log_hostname and pg_stat_activity

2011-01-17 Thread Alvaro Herrera
Excerpts from Peter Eisentraut's message of sáb ene 15 13:15:45 -0300 2011:

> Here is a patch that adds a client_hostname field to pg_stat_activity.
> It takes the hostname if it is available either by having log_hostname
> set or if the pg_hba.conf processing resolved it.  So I think for most
> people who would care about this thing, it would "just work".
> 
> I'm not so sure about the pgstat.c internals.  Do we need the separate
> buffer in shared memory, as in this patch, or could we just copy the
> name directly into the PgBackendStatus struct?  I guess not the latter,
> since my compiler complains about copying a string into a volatile
> pointer.

I think you should do it like clientaddr is handled: fill a string with
the value from MyProcPort, then make the PgBackendStatus point to that.

BTW you need to touch BackendStatusShmemSize if you change the routine
below that (but AFAIU that should be taken out).

...

I'm confused ... why isn't this code broken?  I don't understand why
isn't clientaddr clobbered the next time someone uses the stack space,
given that it's not allocated anywhere.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Tom Lane
Magnus Hagander  writes:
> I wonder if we can trust the *equality* test, but not the inequality?
> E.g. if compressed(A) == compressed(B) we know they're the same, but
> if compressed(A) != compressed(B) we don't know they're not they still
> might be.

I haven't looked at this patch, but it seems to me that it would be
reasonable to conclude A != B if the va_extsize values in the toast
pointers don't agree.  Once you've fetched the toasted values, you've
spent enough cycles that there's not going to be much point in
trying to do any cute optimizations beyond that.  So if the patch is
doing a memcmp on the compressed data, I'd be inclined to get rid of
that part.

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] psql: Add \dL to show languages

2011-01-17 Thread Andreas Karlsson
On Mon, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
> Yeah. Procedural langauges may strictly be wrong, but people aren't
> likely to misunderstand it.

That was idea when suggesting we call it "procedural languages". It is
short and I do not think it can be misunderstood.

Regards,
Andreas



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Jim Nasby
On Jan 17, 2011, at 9:22 AM, Noah Misch wrote:
> On Mon, Jan 17, 2011 at 07:35:52AM +0100, Magnus Hagander wrote:
>> On Mon, Jan 17, 2011 at 06:51, Itagaki Takahiro
>>  wrote:
>>> On Mon, Jan 17, 2011 at 04:05, Andy Colson  wrote:
 This is a review of:
 https://commitfest.postgresql.org/action/patch_view?id=468
 
 Purpose:
 
 Equal and not-equal _may_ be quickly determined if their lengths are
 different. ? This _may_ be a huge speed up if we don't have to detoast.
>>> 
>>> We can skip detoast to compare lengths of two text/bytea values
>>> with the patch, but we still need detoast to compare the contents
>>> of the values.
>>> 
>>> If we always generate same toasted byte sequences from the same raw
>>> values, we don't need to detoast at all to compare the contents.
>>> Is it possible or not?
>> 
>> For bytea, it seems it would be possible.
>> 
>> For text, I think locales may make that impossible. Aren't there
>> locale rules where two different characters can "behave the same" when
>> comparing them? I know in Swedish at least w and v behave the same
>> when sorting (but not when comparing) in some variants of the locale.
>> 
>> In fact, aren't there cases where the *length test* also fails? I
>> don't know this for sure, but unless we know for certain that two
>> different length strings can never be the same *independent of
>> locale*, this whole patch has a big problem...
> 
> Just to be clear, the code already has these length tests today.  This patch
> just moves them before the detoast.

Any reason we can't do this for other varlena? I'm wondering if it makes more 
sense to have some generic toast comparison functions that don't care what the 
data in toast actually is...
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Tom Lane
Peter Eisentraut  writes:
> On mån, 2011-01-17 at 07:35 +0100, Magnus Hagander wrote:
>> In fact, aren't there cases where the *length test* also fails?

> Currently, two text values are only equal of strcoll() considers them
> equal and the bits are the same.  So this patch is safe in that regard.

> There is, however, some desire to loosen this.

That isn't ever going to happen, unless you'd like to give up hash joins
and hash aggregation on text values.

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] We need to log aborted autovacuums

2011-01-17 Thread Simon Riggs
On Mon, 2011-01-17 at 14:46 -0500, Tom Lane wrote:
> Josh Berkus  writes:
> > However, we'd want a separate lock timeout for autovac, of course.  I'm
> > not at all keen on a *statement* timeout on autovacuum; as long as
> > autovacuum is doing work, I don't want to cancel it.  Also, WTF would we
> > set it to?
> 
> Yeah --- in the presence of vacuum cost delay, in particular, a
> statement timeout seems about useless for AV.
> 
> > Going the statement timeout route seems like a way to create a LOT of
> > extra work, troubleshooting, getting it wrong, and releasing patch
> > updates.  Please let's just create a lock timeout.
> 
> Do we actually need a lock timeout either?  The patch that was being
> discussed just involved failing if you couldn't get it immediately.
> I suspect that's sufficient for AV.  At least, nobody's made a
> compelling argument why we need to expend a very substantially larger
> amount of work to do something different.

I have a patch to do this BUT the log message is harder. Until we grab
the lock we can't confirm the table still exists, so we can't get the
name for it. The whole point of this is logging the names of tables for
which we have failed to AV. Suggestions?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Warning compiling pg_dump (MinGW, Windows XP)

2011-01-17 Thread Tom Lane
Robert Haas  writes:
> 2011/1/13 Pavel Golub :
>> pg_dump.c: In function 'dumpSequence':
>> pg_dump.c:11449:2: warning: unknown conversion type character 'l' in format
>> pg_dump.c:11449:2: warning: too many arguments for format

> It seems like PGAC_FUNC_SNPRINTF_LONG_LONG_INT_FORMAT is getting the
> wrong answer on your machine, though I'm not sure why.

That configure check tests whether snprintf produces the right result at
runtime.  It doesn't check whether the compiler will generate a warning
about it.  It looks to me like Pavel has a compiler that is out of sync
with his libc; which is a platform configuration mistake that he needs
to fix.

Another possibility is that configure chose to not use the system
snprintf at all, in which case %lld is the correct syntax to use but gcc
might well have some different expectation.  Without seeing the
config.log results it's hard to be sure about that one; but in any case
it's hard to credit that there are any modern machines where snprintf
can't handle long long int, so this still suggests a platform problem.

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] SSI patch version 12

2011-01-17 Thread Heikki Linnakangas

On 15.01.2011 01:54, Kevin Grittner wrote:

/*
 * for r/o transactions: list of concurrent r/w transactions that we 
could
 * potentially have conflicts with, and vice versa for r/w transactions
 */
TransactionId topXid;   /* top level xid for the transaction, 
if one
 * exists; else 
invalid */
TransactionId finishedBefore;   /* invalid means still running; 
else

 * the struct expires when no

 * serializable xids are before this. */
TransactionId xmin; /* the transaction's snapshot 
xmin */
uint32  flags;  /* OR'd combination of values 
defined below */
int pid;/* pid of associated 
process */
} SERIALIZABLEXACT;


What does that comment about list of concurrent r/w transactions refer 
to? I don't see any list there. Does it refer to 
possibleUnsafeConflicts, which is above that comment?


Above SERIALIZABLEXID struct:

 * A hash table of these objects is maintained in shared memory to provide a
 * quick way to find the top level transaction information for a serializable
 * transaction,  Because a serializable transaction can acquire a snapshot
 * and read information which requires a predicate lock before it has a
 * TransactionId, it must be keyed by VirtualTransactionId; this hashmap
 * allows a fast link from MVCC transaction IDs to the related serializable
 * transaction hash table entry.


I believe the comment is trying to say that there's some *other* hash 
that is keyed by VirtualTransactionId, so we need this other one keyed 
by TransactionId. It took me a while to understand that, it should be 
rephrased.


Setting the high bit in OldSetXidAdd() seems a bit weird. How about just 
using UINT64_MAX instead of 0 to mean no conflicts? Or 1, and start the 
sequence counter from 2.


ReleasePredicateLocks() reads ShmemVariableCache->nextXid without 
XidGenLock. Maybe it's safe, we assume that TransactionIds are atomic 
elsewhere, but at least there needs to be a comment explaining it. But 
it probably should use ReadNewTransactionId() instead.


Attached is a patch for some trivial changes, mostly typos.


Overall, this is very neat and well-commented code. All the data 
structures make my head spin, but I don't see anything unnecessary or 
have any suggestions for simplification. There's a few remaining TODO 
comments in the code, which obviously need to be resolved one way or 
another, but as soon as you're finished with any outstanding issues that 
you know of, I think this is ready for commit.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 2024b48..c8078fe 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -29,7 +29,7 @@

 

-Serializable Snapshot Isloation
+Serializable Snapshot Isolation

 

@@ -61,9 +61,9 @@
 do not conflict with locks acquired for writing data, and so
 reading never blocks writing and writing never blocks reading.
 PostgreSQL maintains this guarantee
-even when providing the the strictest level of transaction
+even when providing the strictest level of transaction
 isolation through the use of an innovative Serializable
-Snapshot Isloation (SSI) level.
+Snapshot Isolation (SSI) level.

 

@@ -647,7 +647,7 @@ ERROR:  could not serialize access due to read/write dependencies among transact
  
  
   
-   Don't leave connections dangling "idle in transction" longer than
+   Don't leave connections dangling "idle in transaction" longer than
necessary.
   
  
diff --git a/src/backend/storage/lmgr/README-SSI b/src/backend/storage/lmgr/README-SSI
index 7eb82be..2c4de29 100644
--- a/src/backend/storage/lmgr/README-SSI
+++ b/src/backend/storage/lmgr/README-SSI
@@ -207,7 +207,7 @@ until one of the other transactions in the set of conflicts which
 could generate an anomaly has successfully committed.  This is
 conceptually similar to how write conflicts are handled.  To fully
 implement this guarantee there needs to be a way to roll back the
-active transaciton for another process with a serialization failure
+active transaction for another process with a serialization failure
 SQLSTATE, even if it is "idle in transaction".
 
 
@@ -286,7 +286,7 @@ index which would have been included in the scan had they existed at
 the time.  Conceptually, we want to lock the gaps between and
 surrounding index entries within the scanned range.
 
-Correctness requires that any insert into an index generate a
+Correctness requires that any insert into an index generates a
 rw-conflict with a concurrent seria

Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 2:25 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut  wrote:
>>> Maybe instead of the proposed patch, a notice could be added:
>>> NOTICE: existing object was replaced
>
>> Well, that would eliminate the backward-compatibility hazard, pretty
>> much, but it seems noisy.  I already find some of these notices to be
>> unduly informative.
>
> ROTFL ...
>
> There has been some previous banter about reorganizing or reclassifying
> the various NOTICE messages to make them more useful and/or less noisy;
> but I don't think we've ever had a really concrete proposal for better
> behavior.  Maybe it's time to reopen that discussion.
>
> I do agree with Peter's underlying point: it would be pretty
> inconsistent for CREATE OR REPLACE to report this bit of info via
> command tag when CREATE IF NOT EXISTS is reporting an absolutely
> equivalent bit of info via elog(NOTICE).

There's a fine line between serious discussion, humor, and outright
mockery here, and I'm not too sure which one Peter's currently engaged
in.  I guess the point here for me is that commands tags for SELECT,
INSERT, UPDATE, and DELETE all return some useful information about
what actually happened - especially, a row count.  If it's reasonable
for those commands to return a row count in the command tag, then
there's no reason why utility commands shouldn't also be allowed to
return high-level status information as part of the command tag.  On
the flip side we could just rip out command tags altogether and have
psql print out the first two words of the input string.

The asymmetry between DROP-IF-EXISTS and CREATE-IF-NOT-EXISTS and the
proposed CREATE-OR-REPLACE behavior doesn't bother me very much,
because it's already asymmetric: the first two currently report what
happened, and the third one currently doesn't.  If you want to propose
to make all of them consistent, how?  I don't particularly like the
idea of adding a NOTICE here; we've got too many of those already[1].
Making DIE report that it didn't do anything via a command tag clearly
won't work, because you can say "DROP IF EXISTS foo, bar, baz" and the
answer might not be the same in all three cases, but COR has no such
issue.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] rhaas=# create table foo (a serial primary key);
NOTICE:  CREATE TABLE will create implicit sequence "foo_a_seq" for
serial column "foo.a"
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index
"foo_pkey" for table "foo"
CREATE TABLE

Well, yeah, why did I say primary key if I didn't want a primary key
index to be created, and why did I say serial if I didn't want a
sequence?

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pl/python refactoring

2011-01-17 Thread Peter Eisentraut
On sön, 2011-01-02 at 12:41 +0100, Jan Urbański wrote:
> Here they are. There are 16 patches in total. They amount to what's
> currently in my refactor branch (and almost to what I've sent as the
> complete refactor patch, while doing the splitting I discovered a few
> useless hunks that I've discarded).

Committed 0001, rest later.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] We need to log aborted autovacuums

2011-01-17 Thread Tom Lane
Josh Berkus  writes:
> However, we'd want a separate lock timeout for autovac, of course.  I'm
> not at all keen on a *statement* timeout on autovacuum; as long as
> autovacuum is doing work, I don't want to cancel it.  Also, WTF would we
> set it to?

Yeah --- in the presence of vacuum cost delay, in particular, a
statement timeout seems about useless for AV.

> Going the statement timeout route seems like a way to create a LOT of
> extra work, troubleshooting, getting it wrong, and releasing patch
> updates.  Please let's just create a lock timeout.

Do we actually need a lock timeout either?  The patch that was being
discussed just involved failing if you couldn't get it immediately.
I suspect that's sufficient for AV.  At least, nobody's made a
compelling argument why we need to expend a very substantially larger
amount of work to do something different.

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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-17 Thread David Fetter
On Mon, Jan 17, 2011 at 09:23:07PM +0200, Peter Eisentraut wrote:
> On mån, 2011-01-17 at 10:05 -0500, Robert Haas wrote:
> > On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut  wrote:
> > > On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
> > >> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp 
> > >> wrote:
> > >> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
> > >> is
> > >> > worth covering in an updated patch too?
> > >> > And if I change that, people might expect the same from DROP X IF
> > >> EXISTS too?
> > >>
> > >> It's far less clear what you'd change those cases to say, and they
> > >> already emit a NOTICE, so it seems unnecessary.
> > >
> > > Maybe instead of the proposed patch, a notice could be added:
> > >
> > > NOTICE: existing object was replaced
> > 
> > Well, that would eliminate the backward-compatibility hazard, pretty
> > much, but it seems noisy.  I already find some of these notices to be
> > unduly informative.
> 
> I'm also anti-NOTICE.
> 
> I'm just saying, we propose that CREATE OR REPLACE should return a tag
> of CREATE or REPLACE depending on what it did, then DROP IF NOT EXISTS
> should also return a tag of DROP or ??? depending on what it did.  Since
> the latter question was settled by a notice, that would also be the
> proper answer for the former.
> 
> Perhaps the next thing is that MERGE should return INSERT or UPDATE
> depending on the outcome?

Given that it can do both in a single statement, I'm guessing that
this is intended to be facetious.  Or are you suggesting that the
command tags become an array?  This has all kinds of interesting
possibilities, but would of course break all kinds of stuff in the
process.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_basebackup for streaming base backups

2011-01-17 Thread Magnus Hagander
On Mon, Jan 17, 2011 at 20:31, Tom Lane  wrote:
> Magnus Hagander  writes:
>> Actually, after some IM chats, I think pg_streamrecv should be
>> renamed, probably to pg_walstream (or pg_logstream, but pg_walstream
>> is a lot more specific than that)
>
> What I like about streamrecv is it's fairly clear which end of the
> connection it's supposed to be used on.  I find "pg_basebackup"
> quite lacking from that perspective, and the same for the names
> above.  Or are you proposing to merge the send and receive sides
> into one executable?

No, the sending side is in walsender.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pg_basebackup for streaming base backups

2011-01-17 Thread Tom Lane
Magnus Hagander  writes:
> Actually, after some IM chats, I think pg_streamrecv should be
> renamed, probably to pg_walstream (or pg_logstream, but pg_walstream
> is a lot more specific than that)

What I like about streamrecv is it's fairly clear which end of the
connection it's supposed to be used on.  I find "pg_basebackup"
quite lacking from that perspective, and the same for the names
above.  Or are you proposing to merge the send and receive sides
into one executable?

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 base backups

2011-01-17 Thread Dimitri Fontaine
Magnus Hagander  writes:
>> Until we get integrated WAL streaming while the base backup is ongoing.
>> We don't know when that is (9.1 or future), but that's what we're aiming
>> to now, right?
>
> Yeah, it does sound like a plan. But to still allow both - streaming
> it in parallell will eat two connections, and I'm sure some people
> might consider that a higher cost.

Sure.  Ah, tradeoffs :)

>> Well I still think that the easier setup we can offer here is to ship
>> with integrated libpq based archive and restore commands.  Those could
>> be bin/pg_walsender and bin/pg_walreceiver.  They would have some
>> switches to make them suitable for running in subprocesses of either the
>> base backup utility or the default libpq based archive daemon.
>
> Not sure why they'd run as an archive command and not like now as a
> replication client - but let's keep that out of this thread and in a
> new one :)

On the archive side you're right that it's not necessary, but it would
be to cater for the restore side.  Sure enough, thinking about it some
more, what we would like here is for the standby to be able to talk to
the archive server (pg_streamsendrecv) rather than the primary, in order
to offload it.  Ok scratch all that and get cascading support instead :)

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-17 Thread Tom Lane
Robert Haas  writes:
> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut  wrote:
>> Maybe instead of the proposed patch, a notice could be added:
>> NOTICE: existing object was replaced

> Well, that would eliminate the backward-compatibility hazard, pretty
> much, but it seems noisy.  I already find some of these notices to be
> unduly informative.

ROTFL ...

There has been some previous banter about reorganizing or reclassifying
the various NOTICE messages to make them more useful and/or less noisy;
but I don't think we've ever had a really concrete proposal for better
behavior.  Maybe it's time to reopen that discussion.

I do agree with Peter's underlying point: it would be pretty
inconsistent for CREATE OR REPLACE to report this bit of info via
command tag when CREATE IF NOT EXISTS is reporting an absolutely
equivalent bit of info via elog(NOTICE).

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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-17 Thread Peter Eisentraut
On mån, 2011-01-17 at 10:05 -0500, Robert Haas wrote:
> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut  wrote:
> > On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
> >> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp 
> >> wrote:
> >> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
> >> is
> >> > worth covering in an updated patch too?
> >> > And if I change that, people might expect the same from DROP X IF
> >> EXISTS too?
> >>
> >> It's far less clear what you'd change those cases to say, and they
> >> already emit a NOTICE, so it seems unnecessary.
> >
> > Maybe instead of the proposed patch, a notice could be added:
> >
> > NOTICE: existing object was replaced
> 
> Well, that would eliminate the backward-compatibility hazard, pretty
> much, but it seems noisy.  I already find some of these notices to be
> unduly informative.

I'm also anti-NOTICE.

I'm just saying, we propose that CREATE OR REPLACE should return a tag
of CREATE or REPLACE depending on what it did, then DROP IF NOT EXISTS
should also return a tag of DROP or ??? depending on what it did.  Since
the latter question was settled by a notice, that would also be the
proper answer for the former.

Perhaps the next thing is that MERGE should return INSERT or UPDATE
depending on the outcome?




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Determining client_encoding from client locale

2011-01-17 Thread Peter Eisentraut
On mån, 2011-01-17 at 14:59 +0100, Susanne Ebrecht wrote:
> I didn't thought about pg_dump dump files here.
> I more thought about files that came out of editors using mad encoding
> and maybe then also were created on Windows and then copied to
> Unix for import.
> 
> Written on little endian, copied to big endian and so on.

That may be worth investigating, but I don't think it's related to the
present patch.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] texteq/byteaeq: avoid detoast [REVIEW]

2011-01-17 Thread Peter Eisentraut
On mån, 2011-01-17 at 07:55 -0500, Robert Haas wrote:
> > There is, however, some desire to loosen this.  Possible
> applications
> > are case-insensitive comparison and Unicode normalization.  It's not
> > going to happen soon, but it may be worth considering not putting in
> an
> > optimization that we'll end up having to rip out again in a year
> > perhaps.
> 
> Hmm.  I hate to give up on this - it's a nice optimization for the
> cases to which it applies.   Would it be possible to jigger things so
> that we can still do it byte-for-byte when case-insensitive comparison
> or Unicode normalization AREN'T in use?

Well, at the moment it's all theoretical anyway.  These features aren't
on the table, and no one has implemented them.

It's quite possible, however, that if we go that way and investigate
which locales are safe for this, we might end up with the same answer as
for which locales are safe for LIKE optimization, namely none.



-- 
Sent 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: compact fsync request queue on overflow

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 1:43 PM, Chris Browne  wrote:
>      (I observe that it wasn't all that obvious that "hash_search()"
>      *adds* elements that are missing.  I got confused and went
>      looking for "hash_add() or similar.  It's permissible to say "dumb
>      Chris".)

I didn't invent that API.  It is a little strange, but you get the hang of it.

>      Question: Is there any further cleanup needed for the entries
>      that got "dropped" out of BGWriterShmem->requests?  It seems
>      not, but a leak seems conceivable.

They're just holding integers, so what's to clean up?

>      - Concurrent access...
>
>        Is there anything that can write extra elements to
>        BGWriterShmem->requests while this is running?
>
>        I wonder if we need to have any sort of lock surrounding this?

It's called with the BgWriterCommLock already held, and there's an
assertion to this effect as well.

>      With higher shared memory, I couldn't readily induce compaction,
>      which is probably a concurrency matter of not having enough volume
>      of concurrent work going on.

You can do it artificially by attaching gdb to the bgwriter.

>      In principle, the only case where it should worsen performance
>      is if the amount of time required to:
>        - Set up a hash table
>        - Insert an entry for each buffer
>        - Walk the skip_slot array, shortening the request queue
>          for each duplicate found
>      exceeded the amount of time required to do the duplicate fsync()
>      requests.
>
>      That cost should be mighty low.  It would be interesting to
>      instrument CompactBgwriterRequestQueue() to see how long it runs.
>
>      But note that this cost is also spread into a direction where it
>      likely wouldn't matter, as it is typically invoked by the
>      background writer process, so this would frequently not be paid
>      by "on-line" active processes.

This is not correct.  The background writer only ever does
AbsorbFsyncRequests(); this would substitute (to some degree) in cases
where the background writer fell down on the job.

> bgwriter.c: In function 'CompactBgwriterRequestQueue':
> bgwriter.c:1142: warning: 'num_skipped' may be used uninitialized in this 
> function

OK, I can fix that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Replication logging

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 1:57 PM, Tom Lane  wrote:
> Magnus Hagander  writes:
>> On Mon, Jan 17, 2011 at 17:46, Tom Lane  wrote:
>>> I think it'd make more sense just to say that replication connections
>>> are subject to the same log_connections rule as others.  An extra GUC
>>> for this is surely overkill.
>
>> I thought so, but Robert didn't agree. And given that things are the
>> way they are, clearly somebody else didn't agree as well - though I've
>> been unable to locate the original discussion if there was one.
>
> The existing behavior dates from here:
> http://archives.postgresql.org/pgsql-committers/2010-03/msg00245.php
>
> As best I can tell there was no preceding discussion, just Simon
> unilaterally deciding that this logging was required for debugging
> purposes.  (There is a followup thread in -hackers arguing about the
> message wording, but nobody questioned whether it should come out
> unconditionally.)
>
> I'm of the opinion that the correct way of "lowering in later releases"
> is to make the messages obey Log_connections.  The "needed for debug"
> argument seems mighty weak to me even for the time, and surely falls
> down now.

On a busy system, you could have a pretty high rate of messages
spewing forth for regular connections - that's a lot to wade through
if all you really want to see are the replication connections, which
should be much lower volume.  But I guess now that we have
pg_stat_replication it's a little easier to see the status of
replication anyway.  On the whole I've found the default setting here
very pleasant, so I'm reluctant to change it, but it sounds like I
might be out-voted.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Replication logging

2011-01-17 Thread Tom Lane
Magnus Hagander  writes:
> On Mon, Jan 17, 2011 at 17:46, Tom Lane  wrote:
>> I think it'd make more sense just to say that replication connections
>> are subject to the same log_connections rule as others.  An extra GUC
>> for this is surely overkill.

> I thought so, but Robert didn't agree. And given that things are the
> way they are, clearly somebody else didn't agree as well - though I've
> been unable to locate the original discussion if there was one.

The existing behavior dates from here:
http://archives.postgresql.org/pgsql-committers/2010-03/msg00245.php

As best I can tell there was no preceding discussion, just Simon
unilaterally deciding that this logging was required for debugging
purposes.  (There is a followup thread in -hackers arguing about the
message wording, but nobody questioned whether it should come out
unconditionally.)

I'm of the opinion that the correct way of "lowering in later releases"
is to make the messages obey Log_connections.  The "needed for debug"
argument seems mighty weak to me even for the time, and surely falls
down 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


[HACKERS] Review: compact fsync request queue on overflow

2011-01-17 Thread Chris Browne
I have been taking a peek at the following commitfest item:
   https://commitfest.postgresql.org/action/patch_view?id=497

Submission:

- I had to trim a little off the end of the patch to apply it, but
  that's likely the fault of how I cut'n'pasted it. It applied cleanly
  against HEAD.

- I observe that it doesn't include any alterations to documentation
  or to regression tests.

Both aspects seem apropos, as the behaviour is entirely internal to
the backend. I wouldn't expect a GUC variable for this, or SQL
commands to control it.

Usability Review:

Does the patch actually implement that?

  - Establishes a hash table
  - Establishes skip slot array
  - Walks through all BGWriter requests
- Adds to hash table.

  (I observe that it wasn't all that obvious that "hash_search()"
  *adds* elements that are missing.  I got confused and went
  looking for "hash_add() or similar.  It's permissible to say "dumb
  Chris".)

- If it's a collision, then mark collision in skip slot array, and
  add to count

  - After the walk
- Clean up hash table
- If nothing found, clean up skip slot array, and return

- If collisions found, then clear them out.

  Question: Is there any further cleanup needed for the entries
  that got "dropped" out of BGWriterShmem->requests?  It seems
  not, but a leak seems conceivable.

Do we want that?

  Eliminating a bunch of fsync() calls that are already being
  induced by other backends seems like a good thing, yep.

Do we already have it?

  Evidently not!

Does it follow SQL spec, or the community-agreed behavior?

  That doesn't seem relevant; this is well outside the scope of
  what SQL spec should have to say.

Does it include pg_dump support (if applicable)?

  Definitely not applicable.

Are there dangers?

  Possibilities...

  - Mentioned in the patch is the possibility of processing the
set of requests in reverse order, which might in principle
reduce work.  But there is some danger of this changing
semantics, so that reversal is not done.

  - Concurrent access...

Is there anything that can write extra elements to
BGWriterShmem->requests while this is running?

I wonder if we need to have any sort of lock surrounding this?

Have all the bases been covered?

  It is a comparatively simple change, so I wouldn't think things
  are missing.

Feature test:

   - Compiled and ran regression test; no problems found.

   Need to do...
- Validate it works as advertised
  - Hook up pgbench
  - Turn on DEBUG1 level
  - Watch that "compacted fsync request queue from %d entries to %d 
entries" come up

  It was a little troublesome inducing it.  I did so by cutting
  shared memory to minimum (128kB).

  I'd regularly get entries like the following:  (Note that I
  changed the error level to WARNING to induce logging this without
  getting all sorts of other stuff).

CONTEXT:  writing block 1735 of relation base/11933/16396
WARNING:  compacted fsync request queue from 16 entries to 3 entries - lost 
[13] entries
CONTEXT:  writing block 14 of relation base/11933/16387
WARNING:  compacted fsync request queue from 16 entries to 3 entries - lost 
[13] entries
CONTEXT:  writing block 4 of relation base/11933/16387
WARNING:  compacted fsync request queue from 16 entries to 3 entries - lost 
[13] entries
CONTEXT:  writing block 6 of relation base/11933/16387
WARNING:  compacted fsync request queue from 16 entries to 3 entries - lost 
[13] entries
CONTEXT:  writing block 1625 of relation base/11933/16396
WARNING:  compacted fsync request queue from 16 entries to 4 entries - lost 
[12] entries
CONTEXT:  writing block 880 of relation base/11933/16396
WARNING:  compacted fsync request queue from 16 entries to 4 entries - lost 
[12] entries
CONTEXT:  writing block 133 of relation base/11933/16396

  With higher shared memory, I couldn't readily induce compaction,
  which is probably a concurrency matter of not having enough volume
  of concurrent work going on.

- Corner cases?

  It's already a corner case ;-).

- Assertion failures?

  None seen thus far.

Performance test

- Does it slow down simple cases?

  It surely shouldn't; compaction is only considered if the fsync
  queue is larger than the number of shared buffers.  That doesn't
  seem like a "simple case" to me!

- Does it improve performance?

  I haven't been able to induce it at a level that would make the
  improvement visible.  But a database that is busy enough to have a
  'full' fsync queue should surely be helped by reducing the number
  of fsync requests.

- Does it slow down other things?

  In principle, the only case where it should worsen performance
  is if the amount of time required to:
- Set up a hash table
- Insert an entry for each buffer

Re: [HACKERS] Spread checkpoint sync

2011-01-17 Thread Greg Smith

Jeff Janes wrote:

Have you ever tested Robert's other idea of having a metronome process
do a periodic fsync on a dummy file which is located on the same ext3fs
as the table files?  I think that that would be interesting to see.
  


To be frank, I really don't care about fixing this behavior on ext3, 
especially in the context of that sort of hack.  That filesystem is not 
the future, it's not possible to ever really make it work right, and 
every minute spent on pandering to its limitations would be better spent 
elsewhere IMHO.  I'm starting with the ext3 benchmarks just to provide 
some proper context for the worst-case behavior people can see right 
now, and to make sure refactoring here doesn't make things worse on it.  
My target is same or slightly better on ext3, much better on XFS and ext4.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] What happened to open_sync_without_odirect?

2011-01-17 Thread Bruce Momjian
Josh Berkus wrote:
> On 1/15/11 4:30 PM, Bruce Momjian wrote:
> > Josh Berkus wrote:
> >> Last I remember, we were going to add this as an option.  But I don't
> >> see a patch in the queue.  Am I missing it?  Was I supposed to write it?
> > 
> > I don't know, but let me add that I am confused how this would look to
> > users.  In many cases, kernels don't even support O_DIRECT, so what
> > would we do to specify this?  What about just auto-disabling O_DIRECT if
> > the filesystem does not support it; maybe issue a log message about it.
> 
> Yes, you *are* confused.  The problem isn't auto-disabling, we already
> do that.  The problem is *auto-enabling*; ages ago we made the
> assumption that if o_sync was supported, so was o_direct.  We've now
> found out that's not true on all platforms.
> 
> Also, test results show that even when supported, o_direct isn't
> necessarily a win.  Hence, the additional fsync_method options.

I think it would be clear if we did not use o_direct for open_*sync, but
only for open_*sync_direct, so there was no auto-direct anything --- you
had to ask for it, and if we don't support it, you get an error.  Right
now people aren't sure what they are getting.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-17 Thread Kääriäinen Anssi
> Well I'm not seeing that here

I am not at work at the moment and I don't have the possibility to compile 
PostgreSQL on this computer, so the example here is from memory.

The issue I saw was this: assume you have an extension foo, containing one 
function, test().

CREATE EXTENSION foo;
DROP FUNCTION test();
-- restricted due to dependency

ALTER FUNCTION test() RENAME TO test2;
DROP FUNCTION test2();
-- not restricted!

The same can be done using CREATE OR REPLACE.

I hope this is not an error on my part. It is possible because I had a lot of 
schemas and my search_path might have been wrong...

 - Anssi
PS: Using web email client, I hope this comes out in somewhat sane format.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 12:48 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Jan 17, 2011 at 12:05 PM, Tom Lane  wrote:
>>> On Red Hat, it is not packaged at all (at least not by me), and won't
>>> be unless it goes into contrib. I don't believe it belongs in the
>>> base package.
>
>> I confess to some confusion about what things "belong" where.  Is
>> contrib the right place for this because we think it's half-baked, or
>> because we think most people won't use it, or just because we're
>> violently allergic to adding stuff to src/bin, or what?
>
> The first two, if you ask me.  And there's another point: I disagree
> with the assumption that platform-specific packagings will or should
> include test_fsync by default.  It'd be better for the packager to make
> a platform-specific choice of default for the users.  I don't mind too
> much putting it into a secondary subpackage such as postgresql-contrib,
> but you won't be seeing it in postgresql-server.

I see.  Well, that seems reasonable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Tom Lane
Bruce Momjian  writes:
> Robert Haas wrote:
>> I confess to some confusion about what things "belong" where.

> I was suggesting /contrib because it seems to be of limited usefulness. 
> I assume people want pg_upgrade to stay in /contrib for the same reason.

pg_upgrade is a different issue, really.  I think it's in contrib
because we don't trust it fully and don't want to promise that it will
work in every single future release anyway.  But even if we moved it to
core, it will always be a special case for packagers: not only is it
not appropriate to put it in the base package, it's not useful to
package it at all unless you also provide a copy of a back-rev
postmaster.  So at least for my money it will always be part of its
own special subpackage postgresql-upgrade.  (BTW, I just recently got
round to making that work for Fedora.)

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] READ ONLY fixes

2011-01-17 Thread Kevin Grittner
Jeff Janes  wrote:
 
> A review:
 
Thanks!  Very thorough!
 
> None of the issues I raise above are severe. Does that mean I
> should change the status to "ready for committer"?
 
I see that notion was endorsed by Robert, so I'll leave it alone for
now.  If a committer asks me to do something about any of those
issues (or others, of course), I'll happily do so.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Tom Lane
Robert Haas  writes:
> On Mon, Jan 17, 2011 at 12:05 PM, Tom Lane  wrote:
>> On Red Hat, it is not packaged at all (at least not by me), and won't
>> be unless it goes into contrib. I don't believe it belongs in the
>> base package.

> I confess to some confusion about what things "belong" where.  Is
> contrib the right place for this because we think it's half-baked, or
> because we think most people won't use it, or just because we're
> violently allergic to adding stuff to src/bin, or what?

The first two, if you ask me.  And there's another point: I disagree
with the assumption that platform-specific packagings will or should
include test_fsync by default.  It'd be better for the packager to make
a platform-specific choice of default for the users.  I don't mind too
much putting it into a secondary subpackage such as postgresql-contrib,
but you won't be seeing it in postgresql-server.

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] Moving test_fsync to /contrib?

2011-01-17 Thread Bruce Momjian
Robert Haas wrote:
> On Mon, Jan 17, 2011 at 12:05 PM, Tom Lane  wrote:
> > Robert Haas  writes:
> >> On Mon, Jan 17, 2011 at 11:47 AM, Bruce Momjian  wrote:
> >>> It seems like /contrib would be more natural, no? ?/bin seems like
> >>> overkill because most people will not want to run it. ?Most of /contrib
> >>> is installed already by installers, I think.
> >
> >> At least on Red Hat, it is packaged separately.
> >
> > On Red Hat, it is not packaged at all (at least not by me), and won't
> > be unless it goes into contrib. ?I don't believe it belongs in the
> > base package.
> 
> I confess to some confusion about what things "belong" where.  Is
> contrib the right place for this because we think it's half-baked, or
> because we think most people won't use it, or just because we're
> violently allergic to adding stuff to src/bin, or what?

I was suggesting /contrib because it seems to be of limited usefulness. 
I assume people want pg_upgrade to stay in /contrib for the same reason.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Bruce Momjian
Tom Lane wrote:
> Robert Haas  writes:
> > On Mon, Jan 17, 2011 at 11:47 AM, Bruce Momjian  wrote:
> >> It seems like /contrib would be more natural, no? ?/bin seems like
> >> overkill because most people will not want to run it. ?Most of /contrib
> >> is installed already by installers, I think.
> 
> > At least on Red Hat, it is packaged separately.
> 
> On Red Hat, it is not packaged at all (at least not by me), and won't
> be unless it goes into contrib.  I don't believe it belongs in the
> base package.
> 
> Also, it's not going to get packaged at all unless it gets renamed to
> something less generic, maybe pg_test_fsync; I'm not going to risk the
> oppobrium of sticking something named "test_fsync" into /usr/bin.
> Moving to contrib would be a good opportunity to fix the name.

Agreed on the need for a name change.  /contrib or /bin are fine with
me.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] psql: Add \dL to show languages

2011-01-17 Thread Bruce Momjian
Peter Eisentraut wrote:
> On m?n, 2011-01-17 at 07:37 +0100, Magnus Hagander wrote:
> > >> which, as Magnus points out, includes non-procedural languages (SQL).
> > >>
> > >> I think that "list languages" could be confusing to newcomers -- the
> > >> very people who might be reading through the help output of psql for
> > >> the first time -- who might suppose that "languages" has something to
> > >> do with the character sets supported by PostgreSQL, and might not even
> > >> be aware that a variety of procedural languages can be used inside the
> > >> database.
> > >
> > > Fair point.
> > 
> > Yeah. Procedural langauges may strictly be wrong, but people aren't
> > likely to misunderstand it.
> 
> The term "procedural" in this context originated with Oracle's PL/SQL,
> which is a procedural language extension to the non-procedural SQL
> language.  From this came PostgreSQL's PL/pgSQL, and that naming was
> then continued with PL/Tcl, at which point "PL/$X" lost its original
> meaning of "procedural extension to the non-procedural language $X" and
> meant more or less "handler for writing PostgreSQL functions in language
> $X".
> 
> Otherwise PL/Scheme will blow your mind. :)
> 
> Think of "procedural language" as "language for writing [PostgreSQL]
> procedures".  As was pointed out, it's also a useful convention for
> distinguishing this from other "languages", such as message
> translations.

FYI, I always refer to them as server-side language, to distinguish them
from client-side languages.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Streaming base backups

2011-01-17 Thread Magnus Hagander
On Mon, Jan 17, 2011 at 11:18, Dimitri Fontaine  wrote:
> Magnus Hagander  writes:
>> With pg_basebackup, you can set up streaming replication in what's
>> basically a single command (run the base backup, copy i na
>> recovery.conf file). In my first version I even had a switch that
>> would create the recovery.conf file for you - should we bring that
>> back?
>
> +1.  Well, make it optional maybe?

It has always been optional. Basically it just creates a recovery.conf file with
primary_conninfo=
standby_mode=on


>> It does require you to set a "reasonable" wal_keep_segments, though,
>> but that's really all you need to do on the master side.
>
> Until we get integrated WAL streaming while the base backup is ongoing.
> We don't know when that is (9.1 or future), but that's what we're aiming
> to now, right?

Yeah, it does sound like a plan. But to still allow both - streaming
it in parallell will eat two connections, and I'm sure some people
might consider that a higher cost.


 What Fujii-san unsuccessfully proposed was to have the master restore
 segments from the archive and stream them to clients, on request.  It
 was deemed better to have the slave obtain them from the archive
 directly.
>>>
>>> Did Fuji-san agreed on the conclusion?
>>
>> I can see the point of the mastering being able to do this, but it
>> seems like a pretty narrow usecase, really. I think we invented
>> wal_keep_segments partially to solve this problem in a neater way?
>
> Well I still think that the easier setup we can offer here is to ship
> with integrated libpq based archive and restore commands.  Those could
> be bin/pg_walsender and bin/pg_walreceiver.  They would have some
> switches to make them suitable for running in subprocesses of either the
> base backup utility or the default libpq based archive daemon.

Not sure why they'd run as an archive command and not like now as a
replication client - but let's keep that out of this thread and in a
new one :)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SSI patch version 12

2011-01-17 Thread Kevin Grittner
Anssi Kääriäinen wrote:
 
> I tried to break the version 11 of the patch (some of the work was
> against earlier versions). In total I have used a full work day
> just trying to break things, but haven't been able to find anything
> after version 8. I can verify that the partial index issue is
> fixed, and the count(*) performance is a lot better now.
 
Thanks for all your testing!
 
For the record, Dan found one more issue with the placement of our LW
locking statements which provided a very small window for one process
to see an uninitialized structure from another.  He found it by
uncommenting the '#define TEST_OLDSERXID' line (thereby forcing any
terminated transaction immediately into the SLRU logic) and letting
DBT-2 pound on it for a long time.  If anyone else is going to be
doing heavy stress testing, you might want to apply this fix:
 
http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=036eefe05ba58da6dff9e1cae766e565182f37be
 
The issue is sufficiently hard to hit that I didn't want to post a
whole new patch for it.  I've been considering it now that doc
changes are complete, but am waiting to see how the 2PC work is
going.
 
> One thing I have been thinking about is how does predicate locking
> indexes work when using functional indexes and functions marked as
> immutable but which really aren't. I don't know how predicate
> locking indexes works, so it might be that this is a non-issue.
 
As David pointed out, doing that is likely to cause you bigger
headaches than this; but yes, you can break serializability by
declaring a non-immutable function as immutable and then using it on
a permanent table.  Like David apparently is, I'm skeptical that this
merits specific mention in the docs; but I'm open to arguments to
the contrary -- particularly if they suggest what language to put
where.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] walreceiver fallback_application_name

2011-01-17 Thread Dimitri Fontaine
Fujii Masao  writes:
>>>  http://www.postgresql.org/docs/9.0/interactive/runtime-config-wal.html#GUC-MAX-WAL-SENDERS
>
> +1 though I could not find the mention to "walreceiver" in the doc.

True, we already use "wal sender", I should have said "similar phrasing".

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Streaming base backups

2011-01-17 Thread Dimitri Fontaine
Magnus Hagander  writes:
> With pg_basebackup, you can set up streaming replication in what's
> basically a single command (run the base backup, copy i na
> recovery.conf file). In my first version I even had a switch that
> would create the recovery.conf file for you - should we bring that
> back?

+1.  Well, make it optional maybe?

> It does require you to set a "reasonable" wal_keep_segments, though,
> but that's really all you need to do on the master side.

Until we get integrated WAL streaming while the base backup is ongoing.
We don't know when that is (9.1 or future), but that's what we're aiming
to now, right?

>>> What Fujii-san unsuccessfully proposed was to have the master restore
>>> segments from the archive and stream them to clients, on request.  It
>>> was deemed better to have the slave obtain them from the archive
>>> directly.
>>
>> Did Fuji-san agreed on the conclusion?
>
> I can see the point of the mastering being able to do this, but it
> seems like a pretty narrow usecase, really. I think we invented
> wal_keep_segments partially to solve this problem in a neater way?

Well I still think that the easier setup we can offer here is to ship
with integrated libpq based archive and restore commands.  Those could
be bin/pg_walsender and bin/pg_walreceiver.  They would have some
switches to make them suitable for running in subprocesses of either the
base backup utility or the default libpq based archive daemon.

Again, all of that is not forcibly material for 9.1, despite having all
the pieces already coded and tested, mainly in Magnus hands.  But could
we get agreement about going this route?

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Include WAL in base backup

2011-01-17 Thread Dimitri Fontaine
Magnus Hagander  writes:
> But however we do it, it will be significantly more complex than just
> including the WAL. And I want to make sure we get *something* done in
> time for 9.1, and then improve upon it. If we can get the improvement
> into 9.1 that's great, but if not it will have to wait until 9.2 - and
> I don't want to leave us without anything for 9.1.

+1.  The only point I'm not clear on is the complexity, and I trust you
to cut off at the right point here… meanwhile, I'm still asking for this
little more until you say your plate's full :)

> Right. I did put both the base backup and the wal streaming in the
> same binary earlier, and the only shared code was the one to connect
> to the db. So I split them apart again. This is the reason to put them
> back together perhaps - or just have the ability for pg_basebackup to
> fork()+exec() the wal streamer.

That would be awesome.

Then pg_streamrecv could somehow accept options that make it suitable
for use as an archive command, connecting to your (still?) third-party
daemon?  At this point it'd be pg_walsender :)

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] REVIEW: Extensions support for pg_dump

2011-01-17 Thread Dimitri Fontaine
Alvaro Herrera  writes:

> Excerpts from Anssi Kääriäinen's message of lun ene 17 12:41:25 -0300 2011:
>
>> While it is not possible to drop functions in extensions, it is possible 
>> to rename a function, and also to CREATE OR REPLACE a function in an 
>> extension. After renaming or CORing a function, it is possible to drop 
>> the function.
>
> Hmm, this seems a serious problem.  I imagine that what's going on is
> that the function cannot be dropped because the extension depends on it;
> but after the rename, the dependencies of the function are dropped and
> recreated, but the dependency that relates it to the extension is
> forgotten.

Well I'm not seeing that here:

~:5490=# drop function utils.lo_manage_d();
ERROR:  cannot drop function utils.lo_manage_d() because extension lo requires 
it
HINT:  You can drop extension lo instead.

src/backend/commands/functioncmds.c

/* rename */
namestrcpy(&(procForm->proname), newname);
simple_heap_update(rel, &tup->t_self, tup);
CatalogUpdateIndexes(rel, tup);

But here:

src/backend/catalog/pg_proc.c

/*
 * Create dependencies for the new function.  If we are updating an
 * existing function, first delete any existing pg_depend entries.
 * (However, since we are not changing ownership or permissions, the
 * shared dependencies do *not* need to change, and we leave them 
alone.)
 */
if (is_update)
deleteDependencyRecordsFor(ProcedureRelationId, retval);

[ ... adding all dependencies back ... ]

/* dependency on extension */
if (create_extension)
{
recordDependencyOn(&myself, &CreateExtensionAddress, 
DEPENDENCY_INTERNAL);
}

Will investigate some more later.
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] REVIEW: Extensions support for pg_dump

2011-01-17 Thread Dimitri Fontaine
Hi,

Thanks for your review!

Anssi Kääriäinen  writes:
> Does the patch apply cleanly?
> No:

That was some bitrot, has been fixed, thanks you for working from the
git repository meanwhile.

> pg_dump.c:3748: warning: too many arguments for format

Fixed in v25 already sent this morning.

> And, make check gives me the following errors:
> test sanity_check ... FAILED
> test rules... FAILED

Fixed too.  Was due to git conflict solving where it adds a new line in
the tests and keep the embedded rowcount the same.  I think.

> Does it include reasonable tests, necessary doc patches, etc?
>
> There is enough documentation, but I think the documentation needs some
> polishing. I am not a native English speaker, so it might be it is my
> English that is failing. The data is there, but the representation might be
> a little off.

We already made lots of improvements there thanks to David Wheeler
reviews in the previous commitfest (which shows up already), and I'll be
happy to continue improving as much as I can.  If all it needs is
english native review, I guess that'll be part of the usual marathon
Bruce runs every year there?

> I didn't spot any tests. This could be that I don't know what to look for...

make -C contrib installcheck will exercise CREATE EXTENSION for each
contrib module.

> Usability review:
>
> The patch implements a way to create extensions. While the patch is labeled
> extensions support for pg_dump, it actually implements more. It implements a
> new way to package and install extension, and changes contrib extensions to
> use the new way.

Well, all there's in the patch is infrastructure to be able to issue
those single lines in your dump :)

> I do think we want these features, and that we do not have those already. As
> far as I understand, there is nothing in the standard regarding this
> feature.
>
> I wonder if the structure of having all the extensions in share/contrib/ is
> a good idea. It might be better if one could separate the contrib extensions
> in one place, and user created extensions in another place. This could make
> it easy to see what user created extensions is installed in (or installable
> to) the cluster. I think this might be helpful to DBAs upgrading PostgreSQL.

It's always been this way and I though it wouldn't be in this patch
scope to re-organise things.  Also I think we should include the UPGRADE
needs when we discuss file system level layout.

> Overall, the system seems intuitive to use. It is relatively easy to create
> extension using this feature, and loading contrib extensions is really
> easy. I haven't tested how easy it is to create C-language extensions using
> this. If I am not mistaken it just adds the compile & install the
> C-functions before installing the extension.

It's using PGXS which existed before, all you have to do that's new is
care about the Makefile EXTENSION variable and the control file.  Even
when doing C coded extension work.

> When using the plain CREATE EXTENSION foobar command without schema
> qualifier, the extension is created in schema public (by name) without
> regard to the current search path. This is inconsistent with create table,
> and if the public schema is renamed, the command gives error:
>
> ERROR: schema "public" does not exist
>
> This makes the name "public" have a special meaning, and I suspect that is
> not wanted.

Fixed in git, thanks for reporting!

~:5490=# set client_min_messages to debug1;
SET
~:5490=# set search_path to utils;
SET
~:5490=# create extension unaccent;
DEBUG:  parse_extension_control_file(unaccent, 
'/home/dfontaine/pgsql/exts/share/contrib/unaccent.control')
DEBUG:  CreateExtensionStmt: with user data, schema = 'utils', encoding = ''
DEBUG:  Installing extension 'unaccent' from 
'/home/dfontaine/pgsql/exts/share/contrib/unaccent.sql', in schema utils, with 
user data
CREATE EXTENSION
~:5490=# \dx
 List of extensions
 Schema | Name  | Version  |   Description  
 
+---+--+-
 utils  | citext| 9.1devel | case-insensitive character string type
 utils  | hstore| 9.1devel | storing sets of key/value pairs
 utils  | int_aggregate | 9.1devel | integer aggregator and an enumerator 
(obsolete)
 utils  | lo| 9.1devel | managing Large Objects
 utils  | unaccent  | 9.1devel | text search dictionary that removes accents
(5 rows)

> When issuing CREATE EXTENSION foo SCHEMA bar, and the extension foo is not
> relocatable and it's control file uses the SET search_path TO @extschema@,
> the search_path is set to bar for the session. That is, it is not changed to
> the original search_path after the command has completed.

It used to work this way with \i, obviously.  Should the extension patch
care about that and how?  Do we want to RESET search_path or to manually
manage it like we do for the c

Re: [HACKERS] pg_basebackup for streaming base backups

2011-01-17 Thread Dimitri Fontaine
Magnus Hagander  writes:
> The walsender can't read pg_class for example, so it can't generate
> that mapping file.

I don't see any way out here.  So let's call .tar good enough for now…

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Moving test_fsync to /contrib?

2011-01-17 Thread Robert Haas
On Mon, Jan 17, 2011 at 12:05 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Jan 17, 2011 at 11:47 AM, Bruce Momjian  wrote:
>>> It seems like /contrib would be more natural, no?  /bin seems like
>>> overkill because most people will not want to run it.  Most of /contrib
>>> is installed already by installers, I think.
>
>> At least on Red Hat, it is packaged separately.
>
> On Red Hat, it is not packaged at all (at least not by me), and won't
> be unless it goes into contrib.  I don't believe it belongs in the
> base package.

I confess to some confusion about what things "belong" where.  Is
contrib the right place for this because we think it's half-baked, or
because we think most people won't use it, or just because we're
violently allergic to adding stuff to src/bin, or what?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Moving test_fsync to /contrib?

2011-01-17 Thread Tom Lane
Robert Haas  writes:
> On Mon, Jan 17, 2011 at 11:47 AM, Bruce Momjian  wrote:
>> It seems like /contrib would be more natural, no?  /bin seems like
>> overkill because most people will not want to run it.  Most of /contrib
>> is installed already by installers, I think.

> At least on Red Hat, it is packaged separately.

On Red Hat, it is not packaged at all (at least not by me), and won't
be unless it goes into contrib.  I don't believe it belongs in the
base package.

Also, it's not going to get packaged at all unless it gets renamed to
something less generic, maybe pg_test_fsync; I'm not going to risk the
oppobrium of sticking something named "test_fsync" into /usr/bin.
Moving to contrib would be a good opportunity to fix the name.

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] Moving test_fsync to /contrib?

2011-01-17 Thread Cédric Villemain
2011/1/17 Bruce Momjian :
> Robert Haas wrote:
>> On Mon, Jan 17, 2011 at 11:02 AM, Bruce Momjian  wrote:
>> > Is there value in moving test_fsync to /contrib?
>>
>> Why would we want to do that?
>
> If we expect users to run the tool to best choose the best
> wal_sync_method.

+1  to move it to contrib/

>
> --
>  Bruce Momjian          http://momjian.us
>  EnterpriseDB                             http://enterprisedb.com
>
>  + It's impossible for everything to be true. +
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



-- 
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et 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] Moving test_fsync to /contrib?

2011-01-17 Thread Alvaro Herrera
Excerpts from Bruce Momjian's message of lun ene 17 13:47:40 -0300 2011:

> It seems like /contrib would be more natural, no?  /bin seems like
> overkill because most people will not want to run it.  Most of /contrib
> is installed already by installers, I think.

I don't understand why it would be "overkill".  Are you saying people
would complain because you installed a 25 kB executable that they might
not want to use?  Just for fun I checked /usr/bin and noticed that I
have a "pandoc" executable, weighing 17 MB, that I have never used and I
have no idea what is it for.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Replication logging

2011-01-17 Thread Magnus Hagander
On Mon, Jan 17, 2011 at 17:46, Tom Lane  wrote:
> Magnus Hagander  writes:
>> Before I go ahead and commit the part that adds
>> log_replication_connections, anybody else want to object to the idea?
>
> I think it'd make more sense just to say that replication connections
> are subject to the same log_connections rule as others.  An extra GUC
> for this is surely overkill.

I thought so, but Robert didn't agree. And given that things are the
way they are, clearly somebody else didn't agree as well - though I've
been unable to locate the original discussion if there was one.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Add support for logging the current role

2011-01-17 Thread Andrew Dunstan



On 01/17/2011 11:44 AM, Alvaro Herrera wrote:

Excerpts from Tom Lane's message of sáb ene 15 00:34:40 -0300 2011:

Stephen Frost  writes:

What about something other than
version_x_y?  I could maybe see having a 'default' and an 'all'
instead..  Then have the default be what we have currently and 'all' be
the full list I'm thinking about.

If "default" always means what it means today, I can live with that.
But if the meaning of "all" changes from version to version, that seems
like a royal mess.  Again, I'm concerned that an external tool like
pgfouine be able to make sense of the value without too much context.
If it doesn't know what some of the columns are, it can just ignore them
--- but a magic summary identifier that it doesn't understand at all is
a problem.

Maybe if we offered a way for the utility to find out the field list
from the magic identifier, it would be enough.

(It would be neat to have magic identifiers for "terse", "verbose",
etc, that mimicked the behavior of client processing)



Just output a header line with the column names. We've long been able to 
import such files. If the list of columns changes we should rotate log 
files before outputting the new format. That might get a little tricky 
to coordinate between backends.


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


  1   2   >