Re: [HACKERS] Additional SPI functions

2009-12-20 Thread James William Pye
On Dec 20, 2009, at 12:03 AM, Tom Lane wrote:
 Why not code a loop around one of the existing SPI execution functions?

*shrug* seemed nicer to push it on the parser than to force the user to split 
up the statements/calls. Or split up the statements myself(well, the parser 
does it so swimmingly =).

It's purpose is to allow the user to put a chunk of SQL into a single big block:

sqlexec(
CREATE TEMP TABLE one ...;
CREATE TEMP TABLE two ...;
init temp tables with data for use in the procedure
)

For me, that tends to read better than breaking up the calls.
Well, the above may be a bad example for crying about readability, but I'm 
thinking of cases with a bit more SQL in em'..


[spi_prepare_statement]
 This looks like it's most likely redundant with the stuff I added
 recently for the plpgsql parser rewrite.

If that allows me to identify the parameter type Oids of the statement, 
optionally supply constant parameters after identifying the types(so I can 
properly create the parameter Datums), and provides access to the resultDesc, 
then yes it is redundant. Personally, I'm hoping for redundant. =)

  Please see if you can use that instead.


I took a very short peak (wasn't really looking..) earlier today (err yesterday 
now) and nothing jumped out at me, but I'll take a closer look now.


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


[HACKERS] alpha3 bundled -- please verify

2009-12-20 Thread Peter Eisentraut
Alpha3 has been bundled and is available at

http://developer.postgresql.org/~petere/alpha/

Please check that it is sane.

No one has written an announcement yet.  I will do it unless someone
feels inspired.  Release should be Monday or Tuesday.


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


Re: [HACKERS] creating index names automatically?

2009-12-20 Thread Ron Mayer
Peter Eisentraut wrote:
 Could we create an option to create index names automatically, so you'd
 only have to write
 
 CREATE INDEX ON foo (a);
 
 which would pick a name like foo_a_idx.  

Why wouldn't it default to a name more like:

  CREATE INDEX foo(a) on foo(a);

which would extend pretty nicely to things like:

  CREATE INDEX foo USING GIN(hstore) ON foo USING GIN(hstore);'

Seems to be both more readable and less chance for arbitrary
collisions if I have column names with underscores.  Otherwise
what would the rule distinguishing create index on foo(a_b)
from create index on foo(a,b), etc.





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


Re: [HACKERS] alpha3 bundled -- please verify

2009-12-20 Thread James William Pye
On Dec 20, 2009, at 1:36 AM, Peter Eisentraut wrote:
 Please check that it is sane.

I'm up, so:

Works for me on snow leopard.


But it doesn't seem to want to stop configure'ing on my fbsd8/amd64 box:

$ ./configure --prefix=/src/build/pg85a3
$ gmake # GNU make 3.81
keeps running configure again and again
... (last few lines before it appears to restart configure)
configure: creating ./config.status
cd .  ./config.status src/Makefile.global
config.status: creating src/Makefile.global
./config.status GNUmakefile
config.status: creating GNUmakefile
cd .  ./config.status --recheck
running CONFIG_SHELL=/bin/sh /bin/sh ./configure --no-create --no-recursion


However, I can build the REL8_5_ALPHA3_BRANCH from git using that box..
Recently pulled, gmake distclean'd and remade again..


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


[HACKERS] fdw validation function vs zero catalog id

2009-12-20 Thread Martin Pihlak
It appears that the function for validating generic options to a FDW,
SERVER and USER MAPPING is always passed a catalog oid of 0. Whereas
it should actually be passed the oid of the catalog that the options
apply to.

Attached patch fixes the issue by passing the proper catalog id from
transformGenericOptions().

PS. I personally would like this applied to 8.4 as well -- it'd enable
us to write a proper validator for pl/proxy fdw.

regards,
Martin

*** a/src/backend/commands/foreigncmds.c
--- b/src/backend/commands/foreigncmds.c
***
*** 88,94  optionListToArray(List *options)
   * This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER MAPPING.
   */
  static Datum
! transformGenericOptions(Datum oldOptions,
  		List *options,
  		Oid fdwvalidator)
  {
--- 88,95 
   * This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER MAPPING.
   */
  static Datum
! transformGenericOptions(Oid catalogId,
! 		Datum oldOptions,
  		List *options,
  		Oid fdwvalidator)
  {
***
*** 162,168  transformGenericOptions(Datum oldOptions,
  	result = optionListToArray(resultOptions);
  
  	if (fdwvalidator)
! 		OidFunctionCall2(fdwvalidator, result, (Datum) 0);
  
  	return result;
  }
--- 163,169 
  	result = optionListToArray(resultOptions);
  
  	if (fdwvalidator)
! 		OidFunctionCall2(fdwvalidator, result, ObjectIdGetDatum(catalogId));
  
  	return result;
  }
***
*** 384,390  CreateForeignDataWrapper(CreateFdwStmt *stmt)
  
  	nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
  
! 	fdwoptions = transformGenericOptions(PointerGetDatum(NULL), stmt-options,
  		 fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(fdwoptions)))
--- 385,393 
  
  	nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
  
! 	fdwoptions = transformGenericOptions(ForeignDataWrapperRelationId,
! 		 PointerGetDatum(NULL),
! 		 stmt-options,
  		 fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(fdwoptions)))
***
*** 501,507  AlterForeignDataWrapper(AlterFdwStmt *stmt)
  			datum = PointerGetDatum(NULL);
  
  		/* Transform the options */
! 		datum = transformGenericOptions(datum, stmt-options, fdwvalidator);
  
  		if (PointerIsValid(DatumGetPointer(datum)))
  			repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
--- 504,513 
  			datum = PointerGetDatum(NULL);
  
  		/* Transform the options */
! 		datum = transformGenericOptions(ForeignDataWrapperRelationId,
! 		datum,
! 		stmt-options,
! 		fdwvalidator);
  
  		if (PointerIsValid(DatumGetPointer(datum)))
  			repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
***
*** 667,673  CreateForeignServer(CreateForeignServerStmt *stmt)
  	nulls[Anum_pg_foreign_server_srvacl - 1] = true;
  
  	/* Add server options */
! 	srvoptions = transformGenericOptions(PointerGetDatum(NULL), stmt-options,
  		 fdw-fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(srvoptions)))
--- 673,681 
  	nulls[Anum_pg_foreign_server_srvacl - 1] = true;
  
  	/* Add server options */
! 	srvoptions = transformGenericOptions(ForeignServerRelationId,
! 		 PointerGetDatum(NULL),
! 		 stmt-options,
  		 fdw-fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(srvoptions)))
***
*** 765,771  AlterForeignServer(AlterForeignServerStmt *stmt)
  			datum = PointerGetDatum(NULL);
  
  		/* Prepare the options array */
! 		datum = transformGenericOptions(datum, stmt-options,
  		fdw-fdwvalidator);
  
  		if (PointerIsValid(DatumGetPointer(datum)))
--- 773,781 
  			datum = PointerGetDatum(NULL);
  
  		/* Prepare the options array */
! 		datum = transformGenericOptions(ForeignServerRelationId,
! 		datum,
! 		stmt-options,
  		fdw-fdwvalidator);
  
  		if (PointerIsValid(DatumGetPointer(datum)))
***
*** 936,942  CreateUserMapping(CreateUserMappingStmt *stmt)
  	values[Anum_pg_user_mapping_umserver - 1] = ObjectIdGetDatum(srv-serverid);
  
  	/* Add user options */
! 	useoptions = transformGenericOptions(PointerGetDatum(NULL), stmt-options,
  		 fdw-fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(useoptions)))
--- 946,954 
  	values[Anum_pg_user_mapping_umserver - 1] = ObjectIdGetDatum(srv-serverid);
  
  	/* Add user options */
! 	useoptions = transformGenericOptions(UserMappingRelationId,
! 		 PointerGetDatum(NULL),
! 		 stmt-options,
  		 fdw-fdwvalidator);
  
  	if (PointerIsValid(DatumGetPointer(useoptions)))
***
*** 1031,1037  AlterUserMapping(AlterUserMappingStmt *stmt)
  			datum = PointerGetDatum(NULL);
  
  		/* Prepare the options array */
! 		datum = transformGenericOptions(datum, stmt-options,
  		fdw-fdwvalidator);
  
  		if (PointerIsValid(DatumGetPointer(datum)))
--- 1043,1051 
  			datum = PointerGetDatum(NULL);
  
  		/* 

Re: [HACKERS] Aggregate ORDER BY patch

2009-12-20 Thread Pavel Stehule
2009/12/19 Marko Tiikkaja marko.tiikk...@cs.helsinki.fi:
 On 2009-12-15 23:10 +0200, Tom Lane wrote:

 Andrew Gierthand...@tao11.riddles.org.uk  writes:

 Notice that there are cases where agg(distinct x order by x) is
 nondeterministic while agg(distinct x order by x,y) is deterministic.

 Well, I think what you're really describing is a case where you're using
 the wrong sort opclass.  If the aggregate can distinguish two values of
 x, and the sort operator can't, use another sort operator that can.

 If we really wanted to take the above seriously, my opinion is that
 we ought to introduce DISTINCT ON in aggregates.  However, at that
 point you lose the argument of standard syntax, so it's not real
 clear why you shouldn't just fall back on
        select agg(x) from (select distinct on (x) x ... order by x,y)

 FWIW, in my opinion the idea behind this patch is to not fall back on hacks
 like that.  This patch already goes beyond the standard and having this
 seems like a useful feature in some cases.  Although the DISTINCT ON syntax
 would have a bit more resemblance on the existing syntax, I'd still like to
 see agg(distinct x order by x,y).


when we are talking about extensions - did you thing about LIMIT clause?

select agg(distinct x order by x limit 10) ..

Regards
Pavel



 Just my $0.02.


 Regards,
 Marko Tiikkaja

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


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


Re: [HACKERS] alpha3 bundled -- please verify

2009-12-20 Thread Tom Lane
James William Pye li...@jwp.name writes:
 But it doesn't seem to want to stop configure'ing on my fbsd8/amd64 box:

Usually that means timestamp skew, ie file timestamps are later than
your system clock.

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] fdw validation function vs zero catalog id

2009-12-20 Thread Tom Lane
Martin Pihlak martin.pih...@gmail.com writes:
 It appears that the function for validating generic options to a FDW,
 SERVER and USER MAPPING is always passed a catalog oid of 0. Whereas
 it should actually be passed the oid of the catalog that the options
 apply to.

According to what?  I can't find any documentation whatsoever on what
arguments that function is supposed to get.

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] Removing pg_migrator limitations

2009-12-20 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Tom Lane wrote:
 What I had in mind was more like
 
 static Oid next_pg_class_oid = InvalidOid;
 
 void
 set_next_pg_class_oid(Oid oid)
 {
  next_pg_class_oid = oid;
 }

 Does exporting a function buy us anything vs. exporting a variable?

Hmm, probably not.  I generally like to avoid global variables, but
in this case it doesn't seem to buy us anything to do so.  Actually,
you could just have the core code do

/* don't make this static, pg_migrator needs to set it */
Oid next_pg_class_oid = InvalidOid;

and not even bother with an extern declaration in the backend header
files (AFAIK gcc won't complain about that).  That would help keep the
variable private to just the one core module plus pg_migrator.


 I will work on a patch to accomplish this, and have pg_migrator link in
 the .so only if the new server is = 8.5, which allows a single
 pg_migrator binary to work for migration to 8.4 and 8.5.

I think you're just creating useless work for yourself by imagining that
pg_migrator is backend-version-independent.  In fact, I was thinking
about proposing that we pull it in as a contrib module.  Because so much
of what it does is tied to details of backend and pg_dump behavior, it's
just a pipe dream to think that developing it as a separate project is
helpful.

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] Removing pg_migrator limitations

2009-12-20 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Tom Lane wrote:
  What I had in mind was more like
  
  static Oid next_pg_class_oid = InvalidOid;
  
  void
  set_next_pg_class_oid(Oid oid)
  {
 next_pg_class_oid = oid;
  }
 
  Does exporting a function buy us anything vs. exporting a variable?
 
 Hmm, probably not.  I generally like to avoid global variables, but
 in this case it doesn't seem to buy us anything to do so.  Actually,
 you could just have the core code do
 
   /* don't make this static, pg_migrator needs to set it */
   Oid next_pg_class_oid = InvalidOid;
 
 and not even bother with an extern declaration in the backend header
 files (AFAIK gcc won't complain about that).  That would help keep the
 variable private to just the one core module plus pg_migrator.

Yes, that was the idea.  We wouldn't expose the variable in other C
files unless necessary.

  I will work on a patch to accomplish this, and have pg_migrator link in
  the .so only if the new server is = 8.5, which allows a single
  pg_migrator binary to work for migration to 8.4 and 8.5.
 
 I think you're just creating useless work for yourself by imagining that
 pg_migrator is backend-version-independent.  In fact, I was thinking
 about proposing that we pull it in as a contrib module.  Because so much
 of what it does is tied to details of backend and pg_dump behavior, it's
 just a pipe dream to think that developing it as a separate project is
 helpful.

Well, I do think it will work for 8.3 to 8.4 ,and 8.4 to 8.5 --- I test
that regularly and I have not seen any failures in that regard.  If we
move it into /contrib, I will make sure fixes get backpatched.  FYI, a
typical test is:

if (GET_MAJOR_VERSION(ctx.old.pg_version) = 803 
GET_MAJOR_VERSION(ctx.new.pg_version) = 804)

The biggest issue with versions is that it is hard for pg_migrator to
text changes in the server during major development so for example once
we add pg_dump support for system oids, it will then require CVS HEAD
for 8.5, but there are not that many people testing pg_migrator with
non-HEAD versions.

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

  + If your life is a hard drive, Christ can be your backup. +

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


[HACKERS] LIKE INCLUDING COMMENTS code is a flight of fancy

2009-12-20 Thread Tom Lane
I just got done fixing a different problem in that area, and then I
noticed this:

regression=# create table src (f1 text);
CREATE TABLE
regression=# create index srclower on src(lower(f1));
CREATE INDEX
regression=# comment on column srclower.pg_expression_1 is 'a comment';
COMMENT
regression=# create index srcupper on src(upper(f1));
CREATE INDEX
regression=# comment on column srcupper.pg_expression_1 is 'another comment';
COMMENT
regression=# create table dest (like src including all);
ERROR:  relation dest_key already exists

The reason this fails is that the LIKE INCLUDING COMMENTS code thinks it
can use ChooseRelationName() in advance of actually creating the
indexes (cf. chooseIndexName in parse_utilcmd.c).  This does not work.
That function is only meant to select a name for an index that will be
created immediately --- otherwise, its logic for avoiding collisions
does not work at all.  As illustrated above.

I don't immediately see a fix for this that isn't a great deal more
trouble than the feature is worth.  I suggest that we might want to just
rip out the support for copying comments on indexes.  Or maybe even the
whole copy-comments aspect of it.

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] Removing pg_migrator limitations

2009-12-20 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Tom Lane wrote:
 I think you're just creating useless work for yourself by imagining that
 pg_migrator is backend-version-independent.  In fact, I was thinking
 about proposing that we pull it in as a contrib module.  Because so much
 of what it does is tied to details of backend and pg_dump behavior, it's
 just a pipe dream to think that developing it as a separate project is
 helpful.

 Well, I do think it will work for 8.3 to 8.4 ,and 8.4 to 8.5 --- I test
 that regularly and I have not seen any failures in that regard.  If we
 move it into /contrib, I will make sure fixes get backpatched.  FYI, a
 typical test is:

 if (GET_MAJOR_VERSION(ctx.old.pg_version) = 803 
 GET_MAJOR_VERSION(ctx.new.pg_version) = 804)

Well, yeah, you can probably make it work if you're willing to carry
enoguh version tests and alternate sets of logic in the source code.
I don't think that is a particularly good engineering approach however.
It makes things less readable and probably more buggy.  Particularly
so since we are talking about some quite significant logic changes here.

There's a reason to clutter, eg, pg_dump with multiple version support.
I don't see the argument for doing so with pg_migrator.  Separate source
code branches seem like a much better idea.

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] creating index names automatically?

2009-12-20 Thread Tom Lane
I wrote:
 Although, having said that, I realize we just opened that can of worms
 with the exclusion-constraint patch:

 regression=# create table foo (f1 text, exclude (lower(f1) with =));
 NOTICE:  CREATE TABLE / EXCLUDE will create implicit index foo_exclusion 
 for table foo
 CREATE TABLE

 The above behavior seems to need improvement already.

And poking further, CREATE TABLE LIKE INCLUDING INDEXES is another place
where we've already bought into automatically generating index names for
arbitrary non-constraint indexes.  And it's even dumber --- you get
names involving _key for indexes that aren't even unique.  So it seems
like we already have a bit of a problem here.

The first thoughts I have about this are:

* Use FigureColname to derive a name for an expression column, except
I'd be inclined to have the fallback case be expr not ?column?.

* Append _index not _key if it's not a constraint-related index.

I'm also a bit tempted to propose that we start using FigureColname
for the actual attribute names of expression indexes, instead of the
not terribly helpful pg_expression_n convention.  In this case we'd
have to append a number if necessary to make the name unique among the
column names of the index.

Comments?

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] Removing pg_migrator limitations

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 1:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:
     if (GET_MAJOR_VERSION(ctx.old.pg_version) = 803 
         GET_MAJOR_VERSION(ctx.new.pg_version) = 804)

 Well, yeah, you can probably make it work if you're willing to carry
 enoguh version tests and alternate sets of logic in the source code.
 I don't think that is a particularly good engineering approach however.
 It makes things less readable and probably more buggy.  Particularly
 so since we are talking about some quite significant logic changes here.

 There's a reason to clutter, eg, pg_dump with multiple version support.
 I don't see the argument for doing so with pg_migrator.  Separate source
 code branches seem like a much better idea.

I guess we have to look at the specific cases that come up, but ISTM
that a branch here amounts to a second copy of the code that has to be
separately maintained.  I'm having a hard time working up much
enthusiasm for that prospect.

...Robert

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


[HACKERS] [WIP] Inspection of row types in pl/pgsq l and pl/sql

2009-12-20 Thread Florian G. Pflug

Hi

I've completed a (first) working version of a extension that allows
easier introspection of composite types from SQL and pl/PGSQL.

The original proposal and ensuing discussion can be found here:
http://archives.postgresql.org/pgsql-hackers/2009-11/msg00695.php

The extension can be found on:
http://github.com/fgp/pg_record_inspect

This is what the extension currently provides (all in schema
record_inspect).

* fieldinfo [composite type]
  Used to by fieldinfos() to describe a record's fields.
  Contains the fields
fieldname (name),
fieldtype (regclass),
fieldtypemod (varchar)

* fieldinfo[] fieldinfos(record)
  Returns an array of fieldinfos describing the record''s fields

* anyelement fieldvalue(record, field name, defval anyelement,
coerce boolean)
  Returns the value of the field field, or defval should the value
  be null. If coerce is true, the value is coerced to defval's type
  if possible, otherwise an error is raised if the field''s type and
  defval's type differ.

* anyelement fieldvalues(record, defval anyelement, coerce boolean)
  Returns an array containing values of the record'' fields. NULL
  values are replaced by defval. If coerce is false, only the
  fields with the same type as defval are considered. Otherwise, the
  field'' values are coerced if possible, or an error is raised if not.

The most hacky part of the code is probably coerceDatum() - needed to
coerce a field's value to the requested output type. I wanted to avoid
creating and parsing an actual SQL statement for every cast, and instead
chose to use coerce_to_target_type() to create the expression trees
representing casts. I use the noe type CoerceToDomainValue to inject the
source value into the cast plan upon execution - see makeCastPlan() and
execCastPlan() for details. If anyone has a better idea, please speak up

I personally would like to see this becoming a contrib module one day,
but that of course depends on how much interest there is in such a feature.

best regards,
Florian Pflug



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [HACKERS] Removing pg_migrator limitations

2009-12-20 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sun, Dec 20, 2009 at 1:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 There's a reason to clutter, eg, pg_dump with multiple version support.
 I don't see the argument for doing so with pg_migrator.  Separate source
 code branches seem like a much better idea.

 I guess we have to look at the specific cases that come up, but ISTM
 that a branch here amounts to a second copy of the code that has to be
 separately maintained.  I'm having a hard time working up much
 enthusiasm for that prospect.

Well, it'd be exactly like back-patching fixes across multiple branches
is for fixes in the core code now.  In code that hasn't changed across
branches, that's not terribly painful.  If the code has changed, then
you're going to have pain no matter which way you do it.

But the real problem with having pg_migrator be a separate project is
that a lot of the time fix pg_migrator is really going to mean fix
pg_dump or even change the backend.  We already had problems with
version skew between pg_migrator and various 8.4 alpha/beta releases.
That type of problem isn't likely to magically go away in the future.

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] Removing pg_migrator limitations

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 2:08 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sun, Dec 20, 2009 at 1:49 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 There's a reason to clutter, eg, pg_dump with multiple version support.
 I don't see the argument for doing so with pg_migrator.  Separate source
 code branches seem like a much better idea.

 I guess we have to look at the specific cases that come up, but ISTM
 that a branch here amounts to a second copy of the code that has to be
 separately maintained.  I'm having a hard time working up much
 enthusiasm for that prospect.

 Well, it'd be exactly like back-patching fixes across multiple branches
 is for fixes in the core code now.  In code that hasn't changed across
 branches, that's not terribly painful.  If the code has changed, then
 you're going to have pain no matter which way you do it.

 But the real problem with having pg_migrator be a separate project is
 that a lot of the time fix pg_migrator is really going to mean fix
 pg_dump or even change the backend.  We already had problems with
 version skew between pg_migrator and various 8.4 alpha/beta releases.
 That type of problem isn't likely to magically go away in the future.

I agree that pulling pg_migrator into contrib seems pretty sensible.
What I want to make sure we're on the same page about is which
versions the 8.5 pg_migrator will allow you to upgrade from and to.  I
think we should at least support 8.3 - 8.5 and 8.4 - 8.5.  If you're
saying we don't need to support 8.3 - 8.4 any more once 8.5 comes
out, I'm probably OK with that, but perhaps we should try to get a few
more opinions before setting that policy in stone.

...Robert

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


Re: [HACKERS] alpha3 release schedule?

2009-12-20 Thread Simon Riggs
On Sat, 2009-12-19 at 20:59 +0200, Heikki Linnakangas wrote:

 Well, that was the criteria I used to decide whether to commit or not.
 Not everyone agreed to begin with, and the reason I used that criteria
 was a selfish one: I didn't want to be forced to fix loose ends after
 the commitfest myself. The big reason for that was that I didn't know
 how much time I would have for that. I have no complaints about Simon's
 commit. Knowing that I'm not on the hook to close the loose ends, I'm
 very happy that it's finally in. (That doesn't mean that I'll stop
 paying attention to this patch; I will do as much as I have time to.)
 
 Regarding the bugs you found, I put them on the TODO list at
 https://wiki.postgresql.org/wiki/Hot_Standby_TODO, under the must-fix
 category. I think they need to be fixed before final release, but
 there's no need to delay the alpha release for them.

Hmmm, well, if you are still paying attention you'll know that neither
of those issues are bugs in the code that was committed. One of them has
been fixed and the deadlock problem has a workaround applied. 

That workaround, err... works, but I accept its not ideal. But then a
few things are not ideal and it does seem unlikely that every one of
them will have a perfect fix in the next two months.

I'll change the TODO page.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Removing pg_migrator limitations

2009-12-20 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I agree that pulling pg_migrator into contrib seems pretty sensible.
 What I want to make sure we're on the same page about is which
 versions the 8.5 pg_migrator will allow you to upgrade from and to.  I
 think we should at least support 8.3 - 8.5 and 8.4 - 8.5.  If you're
 saying we don't need to support 8.3 - 8.4 any more once 8.5 comes
 out, I'm probably OK with that, but perhaps we should try to get a few
 more opinions before setting that policy in stone.

If we can do that reasonably (which might well be the case), I'd be for
it.  What I'm objecting to is what I take to be Bruce's plan of
supporting 8.3 - 8.4 and 8.4 - 8.5 with the same pg_migrator sources.
The stuff we're talking about doing in this thread is going to cause
those two cases to diverge rather drastically.  8.3 - 8.5 and 8.4 -
8.5 may be close enough together to be reasonable to support in one
set of source code.

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] alpha3 bundled -- please verify

2009-12-20 Thread James William Pye
On Dec 20, 2009, at 9:20 AM, Tom Lane wrote:
 Usually that means timestamp skew, ie file timestamps are later than
 your system clock.

Yep. It's working now.

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


Re: [HACKERS] alpha3 bundled -- please verify

2009-12-20 Thread Marko Tiikkaja

On 2009-12-20 18:20 +0200, Tom Lane wrote:

James William Pyeli...@jwp.name  writes:

But it doesn't seem to want to stop configure'ing on my fbsd8/amd64 box:


Usually that means timestamp skew, ie file timestamps are later than
your system clock.


I've hit this problem before and could not figure out what was wrong. 
Is this documented somewhere?



Regards,
Marko Tiikkaja

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


Re: [HACKERS] alpha3 release schedule?

2009-12-20 Thread Simon Riggs
On Sat, 2009-12-19 at 14:20 +0200, Peter Eisentraut wrote:

 Do people want more time to play with hot standby?  Otherwise alpha3
 should go out on Monday or Tuesday.

No thanks. There were no known bugs in the code I committed, excepting
the need to address VACUUM FULL. That will take longer than two days and
isn't sufficient reason to halt Alpha3, IMHO.

If others wish to revoke, then maybe we should consider a specific Alpha
version just for Hot Standby.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] alpha3 release schedule?

2009-12-20 Thread Simon Riggs
On Sat, 2009-12-19 at 20:59 +0200, Heikki Linnakangas wrote:
 I put them on the TODO list at
 https://wiki.postgresql.org/wiki/Hot_Standby_TODO, under the must-fix
 category.

I notice you also re-arranged other items on there, specifically the
notion that starting from a shutdown checkpoint is somehow important.
It's definitely not any kind of bug.

We've discussed this on-list and I've requested that you justify this.
So far, nothing you've said on that issue has been at all convincing for
me or others. The topic is already mentioned on the HS todo, since if
one person requests something we should track that, just in case others
eventually agree. But having said that, it clearly isn't a priority, so
rearranging the item like that was not appropriate, unless you were
thinking of doing it yourself, though that wasn't marked.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


[HACKERS] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Pavel Stehule
Hello

I am thinking about implementation of median function. This function
should be implemented in two ways:

a) direct entering an ORDER BY clause for median funcall in gram.y
b) general support for preordered aggregates.

I prefer plan b, because there are more similar aggregates - like
Quantiles. So with general support of preordered aggregates we can
move these aggregates to contrib, separate library or core (if we
would). I am for median in core - and others in contrib - as example
of this kind of aggregates.

What we need:

a) some new intelligence in parser - it use default ORDER BY clause
when explicit ORDER BY clause is missing.

b) for effective implementation we need to know real number of tuples
in state function. Without this knowledge we have to copy tuples to
tuple store or to array. It is useless, because we have to execute
sortby before - so we have this information. There could be one
optimalisation. We don't need to process all rows - for median we have
to process only 1/2 of rows - it could be nice, if state function can
send signal - don't call me more.

New syntax:

  CREATE AGGREGATE name ( input_data_type [ORDER BY [(DESC|ASC)]] [ , ... ] ) (
SFUNC = sfunc,
STYPE = state_data_type
[ , FINALFUNC = ffunc ]
[ , INITCOND = initial_condition ]
[ , SORTOP = sort_operator ]
)

example:

CREATE AGGREGATE median (float8 ORDER BY) (
  SFUNC = median_state,
  STYPE = internal,
  FINALFUNC = median_final,
  SORTOP = ''
)

I would to insure, so this idea is acceptable.

Regards
Pavel Stehule

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


Re: [HACKERS] alpha3 release schedule?

2009-12-20 Thread Simon Riggs
On Sat, 2009-12-19 at 23:22 +0900, Hiroyuki Yamada wrote:
 Do people want more time to play with hot standby?  Otherwise alpha3
 should go out on Monday or Tuesday.
 
 
 Well, I want to know whether the problem I refered to 
 in http://archives.postgresql.org/pgsql-hackers/2009-12/msg01641.php
 is must-fix or not.
 
 This problem is a corollary of the deadlock problem. This is less catstrophic
 but more likely to happen.
 
 If you leave this problem, for example, any long-running transactions,
 holding any cursors in whatever tables, have a possibility of freezing
 whole recovery work in HotStandby node until the transaction commit.

You seem very insistent on bringing up problems just before release.
Almost as if you have a reason to back some other technology other than
this one.

The problem you mention here has been documented and very accessible for
months and not a single person mentioned it up to now. What's more, the
equivalent problem happens in the latest production version of Postgres
- users can delay VACUUM endlessly in just the same way, yet I've not
seen this raised as an issue in many years of using Postgres. Similarly,
there are some ways that Postgres can deadlock that it need not, yet
those negative behaviours are accepted and nobody is rushing to fix
them, nor demanding that they should be. Few things are theoretically
perfect on their first release.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 I am thinking about implementation of median function. This function
 should be implemented in two ways:

 a) direct entering an ORDER BY clause for median funcall in gram.y
 b) general support for preordered aggregates.

 I prefer plan b, because there are more similar aggregates - like
 Quantiles.

This seems like a great deal of mechanism to solve a very localized
problem.

I think that we've already expanded the capabilities of aggregates
a great deal for 8.5, and we should let it sit as-is for a release
or two and see what the real user demand is for additional features.

I'm particularly concerned by the fact that the feature set is already
far out in front of what the planner can optimize effectively (e.g.,
there's no ability to combine the work when multiple aggregates need the
same sorted data).  The more features we add on speculation, the harder
it's going to be to close that gap.

Another risk is that features added now might preclude adding others
later.

regards, tom lane

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


Re: [HACKERS] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Pavel Stehule
2009/12/20 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 I am thinking about implementation of median function. This function
 should be implemented in two ways:

 a) direct entering an ORDER BY clause for median funcall in gram.y
 b) general support for preordered aggregates.

 I prefer plan b, because there are more similar aggregates - like
 Quantiles.

 This seems like a great deal of mechanism to solve a very localized
 problem.


plan a doesn't block plan b - it is very simple. So we can start with a.

 I think that we've already expanded the capabilities of aggregates
 a great deal for 8.5, and we should let it sit as-is for a release
 or two and see what the real user demand is for additional features.

 I'm particularly concerned by the fact that the feature set is already
 far out in front of what the planner can optimize effectively (e.g.,
 there's no ability to combine the work when multiple aggregates need the
 same sorted data).  The more features we add on speculation, the harder
 it's going to be to close that gap.

I didn't thing about this optimalisation, but this point could not be
impossible. Bigger problem is using of indexes.


 Another risk is that features added now might preclude adding others
 later.


sure. It was my question, what is preferred.

Regards
Pavel

                        regards, tom lane


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


Re: [HACKERS] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Dimitri Fontaine
Hi from a real user :)

Le 20 déc. 2009 à 22:08, Tom Lane a écrit :
 Pavel Stehule pavel.steh...@gmail.com writes:
 b) general support for preordered aggregates.
 
 I think that we've already expanded the capabilities of aggregates
 a great deal for 8.5, and we should let it sit as-is for a release
 or two and see what the real user demand is for additional features.

All we can have in PostgreSQL without needing to resort to either PLs or 
application code is worth it from here, and I can already picture the smiling 
on our developers face when I say them median() is there by default.

 I'm particularly concerned by the fact that the feature set is already
 far out in front of what the planner can optimize effectively (e.g.,
 there's no ability to combine the work when multiple aggregates need the
 same sorted data).  The more features we add on speculation, the harder
 it's going to be to close that gap.
 
 Another risk is that features added now might preclude adding others
 later.

Now, I have no idea if augmenting the aggregate properties with an optional 
sorting step is the right approach, but it sounds right on spot (general enough 
without being over engineering). I guess it would give the planner the same 
information as if the user did type the extra order by himself, so I'm not sure 
how much your remarks would apply?

I mean we already have explicit user ordering in aggregates at call site, 
adding the exact same information in the aggregate definition itself surely 
isn't going to be such a change there?

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


Re: [HACKERS] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Tom Lane
Dimitri Fontaine dfonta...@hi-media.com writes:
 Le 20 déc. 2009 à 22:08, Tom Lane a écrit :
 Another risk is that features added now might preclude adding others
 later.

 Now, I have no idea if augmenting the aggregate properties with an optional 
 sorting step is the right approach, but it sounds right on spot (general 
 enough without being over engineering). I guess it would give the planner the 
 same information as if the user did type the extra order by himself, so I'm 
 not sure how much your remarks would apply?

 I mean we already have explicit user ordering in aggregates at call site, 
 adding the exact same information in the aggregate definition itself surely 
 isn't going to be such a change there?

It's not obvious to me that this feature is better than others that
might apply in the same general area, eg the existing min/max
optimization.  Since (to repeat myself) we have no field experience
with ordered aggregate inputs, assuming we know what the next extension
should be in this area seems a bit premature.

Also, you're conveniently ignoring my point about how optimization
should probably be the next area of focus here.  Right now, there
is really little if any value in having this feature, because a
median aggregate could sort internally and not be any more or less
efficient than having nodeAgg do it.  Once we have an optimization
framework there might be some advantage there, but we should not
complicate matters with second-order features before we have 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


Re: [HACKERS] creating index names automatically?

2009-12-20 Thread Michael Glaesemann


On Dec 20, 2009, at 13:58 , Tom Lane wrote:


* Append _index not _key if it's not a constraint-related index.


_idx instead of _index keeps things a bit shorter (and a couple of  
keystrokes further from NAMEDATALEN). There's precedent for  
abbreviations with automatic naming in Postgres, e.g., _fkey.


Michael Glaesemann
grzm seespotcode 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] fdw validation function vs zero catalog id

2009-12-20 Thread Martin Pihlak
Tom Lane wrote:
 According to what?  I can't find any documentation whatsoever on what
 arguments that function is supposed to get.
 
   regards, tom lane
 

According to
http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html:

The validator function must take two arguments: one of type text[], which
will contain the array of options as stored in the system catalogs, and one
of type oid, which will be the OID of the system catalog containing the
options, or zero if the context is not known.

regards,
Martin


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


Re: [HACKERS] fdw validation function vs zero catalog id

2009-12-20 Thread Tom Lane
Martin Pihlak martin.pih...@gmail.com writes:
 Tom Lane wrote:
 According to what?  I can't find any documentation whatsoever on what
 arguments that function is supposed to get.

 According to
 http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html:

 The validator function must take two arguments: one of type text[], which
 will contain the array of options as stored in the system catalogs, and one
 of type oid, which will be the OID of the system catalog containing the
 options, or zero if the context is not known.

Hmm, dunno how I missed that.  But anyway ISTM the current code conforms
to that specification just fine.  I think what you're really lobbying
for is that we remove the or zero escape hatch and insist that the
backend code do whatever it has to do to supply a correct OID.  This
patch shows that that's not too hard right now, but are there going to
be future situations where it's harder?  What was the motivation for
including the escape hatch in the first place?

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] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Andrew Gierth
 Pavel == Pavel Stehule pavel.steh...@gmail.com writes:

  2009/12/20 Tom Lane t...@sss.pgh.pa.us:
  I think that we've already expanded the capabilities of aggregates
  a great deal for 8.5, and we should let it sit as-is for a release
  or two and see what the real user demand is for additional
  features.
  
  I'm particularly concerned by the fact that the feature set is
  already far out in front of what the planner can optimize
  effectively (e.g., there's no ability to combine the work when
  multiple aggregates need the same sorted data).  The more features
  we add on speculation, the harder it's going to be to close that
  gap.

I absolutely agree with Tom here and for some quite specific reasons.

An optimal (or at least more optimal than is currently possible)
implementation of median() on top of the ordered-agg code as it stands
requires additions to the aggregate function interface: the median agg
implementation would have to, as a minimum, know how many rows of
sorted input are available. In addition, it would be desirable for it
to have direct (and possibly bidirectional) access to the tuplesort.

Now, if we look at how ordered aggs ought to be optimized, it's clear
that the planner should take the ordering costs into account and
consider plans that order the input instead. Once you do this, then
there's no longer any pre-computed count or tuplesort object
available, so if you'd implemented a better median() before the
optimizations, you'd end up either having to forgo the optimization or
break the median() agg again; clearly not something we want.

Plus, a feature that I intentionally omitted from the ordered-aggs
patch is the ability to do ordered-aggs as window functions. This is
in the spec, but (a) there were conflicting patches for window
functions on the table and (b) in my opinion, much of the work needed
to implement ordered-agg-as-window-func in an effective manner is
dependent on doing more work on optimization first (or at least will
potentially become easier as a result of that work).

So I think both the optimization issue and the window functions issue
would be best addressed before trying to build any additional features
on top of what we have so far.

-- 
Andrew (irc:RhodiumToad)

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


Re: [HACKERS] creating index names automatically?

2009-12-20 Thread Tom Lane
Michael Glaesemann g...@seespotcode.net writes:
 On Dec 20, 2009, at 13:58 , Tom Lane wrote:
 * Append _index not _key if it's not a constraint-related index.

 _idx instead of _index keeps things a bit shorter (and a couple of  
 keystrokes further from NAMEDATALEN). There's precedent for  
 abbreviations with automatic naming in Postgres, e.g., _fkey.

No objection here.

BTW, I'm having second thoughts about the last part of my proposal:

 I'm also a bit tempted to propose that we start using FigureColname
 for the actual attribute names of expression indexes, instead of the
 not terribly helpful pg_expression_n convention.

The trouble with changing the index attnames for expressions is that it
increases the risk of collisions with attnames for regular index
columns.  You can hit that case today:

regression=# create table foo (f1 int, f2 text);
CREATE TABLE
regression=# create index fooi on foo(f1, lower(f2));
CREATE INDEX
regression=# \d fooi
  Index public.fooi
 Column  |  Type   | Definition 
-+-+
 f1  | integer | f1
 pg_expression_2 | text| lower(f2)
btree, for table public.foo

regression=# alter table foo rename f1 to pg_expression_2;
ERROR:  duplicate key value violates unique constraint 
pg_attribute_relid_attnam_index
DETAIL:  Key (attrelid, attname)=(64621, pg_expression_2) already exists.

but it's not exactly probable that someone would name a column
pg_expression_N.  The risk goes up quite a lot if we might use simple
names like abs or lower for expression columns.

We could work around this by being willing to rename index columns on
the fly, but that creates a big risk of failing to dump and reload
comments on index columns, because the columns might not get the same
names in a newer PG version.  (I seem to remember having objected to the
whole concept of comments on index columns on the grounds that it would
lock us into the current index column naming scheme, and that's exactly
what it's doing here.)

So I think we're stuck with the current column naming rule, but we do
have wiggle room on the name of the index itself.

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] alpha3 release schedule?

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 3:42 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Sat, 2009-12-19 at 20:59 +0200, Heikki Linnakangas wrote:
 I put them on the TODO list at
 https://wiki.postgresql.org/wiki/Hot_Standby_TODO, under the must-fix
 category.

 I notice you also re-arranged other items on there, specifically the
 notion that starting from a shutdown checkpoint is somehow important.
 It's definitely not any kind of bug.

 We've discussed this on-list and I've requested that you justify this.
 So far, nothing you've said on that issue has been at all convincing for
 me or others. The topic is already mentioned on the HS todo, since if
 one person requests something we should track that, just in case others
 eventually agree. But having said that, it clearly isn't a priority, so
 rearranging the item like that was not appropriate, unless you were
 thinking of doing it yourself, though that wasn't marked.

This doesn't match my recollection of the previous discussion on this
topic.  I am not sure that I'd call it a bug, but I'd definitely like
to see it fixed, and I think I mentioned that previously, though I
don't have the email in front ATM.  I am also not aware that anyone
other than yourself has opined that we should not worry about fixing
it, although I might be wrong about that too.  At any rate, clearly
not a priority seems like an overstatement relative to my memory of
that conversation.

...Robert

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


Re: [HACKERS] Largeobject Access Controls (r2460)

2009-12-20 Thread KaiGai Kohei
(2009/12/19 12:05), Robert Haas wrote:
 On Fri, Dec 18, 2009 at 9:48 PM, Tom Lanet...@sss.pgh.pa.us  wrote:
 Robert Haasrobertmh...@gmail.com  writes:
 Oh.  This is more complicated than it appeared on the surface.  It
 seems that the string BLOB COMMENTS actually gets inserted into
 custom dumps somewhere, so I'm not sure whether we can just change it.
   Was this issue discussed at some point before this was committed?
 Changing it would seem to require inserting some backward
 compatibility code here.  Another option would be to add a separate
 section for BLOB METADATA, and leave BLOB COMMENTS alone.  Can
 anyone comment on what the Right Thing To Do is here?

 The BLOB COMMENTS label is, or was, correct for what it contained.
 If this patch has usurped it to contain other things
 
 It has.
 
 I would argue
 that that is seriously wrong.  pg_dump already has a clear notion
 of how to handle ACLs for objects.  ACLs for blobs ought to be
 made to fit into that structure, not dumped in some random place
 because that saved a few lines of code.
 
 OK.  Hopefully KaiGai or Takahiro can suggest a fix.

Currently, BLOBS (and BLOB COMMENTS) section does not contain
owner of the large objects, because it may press the local memory
of pg_dump when the database contains massive amount of large
objects.
I believe it is the reason why we dump all the large objects in
a single section. Correct?

I don't think it is reasonable to dump all the large object with
its individual section.
However, we can categorize them per owner. In generally, we can
assume the number of database users are smaller than the number
of large objects.
In other word, we can obtain the number of sections to be dumped
as result of the following query:

  SELECT DISTINCT lomowner FROM pg_largeobject_metadata;

Then, we can dump them per users.

For earlier versions, all the large objects will be dumped in
a single section with anonymous user.

What's your opinion?
-- 
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.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] Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 7:20 PM, John Naylor jcnay...@gmail.com wrote:
 Greetings,

 Following up on my experimental patch last month to revamp the BKI
 infrastructure, I am proposing a less invasive set of changes with the
 hope of offering something committable. Some of these were discussed
 by Robert Haas and others last summer.

 1. Remove all DATA() declarations from pg_attribute.h, since they are
 easily generated. Introduce a new BKI pseudo-command
 BKI_NAILED_IN_CACHE, which indicates that relcache.c needs a
 Schema_pg_foo declaration for that catalog. Place these declarations
 in a new header schemapg.h. This will reduce the effort to add or
 change critical tables.

 2. Use identical scripts on Posix and Windows systems, using Perl 5.6
 (no CPAN modules needed). The grepping of the catalog headers is done
 by Catalog.pm, which gives the scripts gen_bki.pl and gen_fmgr.pl a
 structured interface to the data. The pg_type info is saved so that
 the relevant fields can be copied into those of pg_attribute.

 3. Make the BKI files, fmgrtab.c, fmgroids.h, and schemapg.h distprep
 targets, so distribution tarballs can still be built without Perl on
 Posix systems.

 Feedback on the Makefile changes would be appreciated, since that was
 the hardest part for me. The MSVC changes are untested.

This is really nice.  I haven't done a full review, but it seems as if
you've eliminated some of the more controversial aspects of what I did
before, as well as done some good cleanup and refactoring.  One minor
nit is that I think we should end up with genbki.pl rather than
gen_bki.pl, in the interest of changing nothing without a good reason.

A more important point is whether we really need to make this
dependent on Perl 5.6 or later.  What features are we using here that
actually require Perl 5.6?  I suspect the answer is none, but we
don't like writing the code in a way that is backward compatible to
crufty, ancient versions of Perl, to which my response is get over
it.  :-)   I always use the three-argument form of open() in all new
code, but for fixed strings the two-argument form is just as good and
has been supported for far longer.  Any suggestions that we should
prefer clean code over portability are, in my opinion, non-starters.

Again, thank you for your work on this!

...Robert

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


[HACKERS] Re: Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Greg Stark
Incidentally there are O(n) algorithms for finding the median (or any
specific offset). It shouldn't be necessary to sort at all.

I'm not sure which path this argues for - perhaps Tom's position that
we need more optimiser infrastructure so we can see how to accomplish
this. Perhaps it means you should really implement median() with an
internal selection alagorithm and not depend on the optimizer at all.


-- 
greg

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


Re: [HACKERS] Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread Andres Freund
Hi,

On Monday 21 December 2009 02:23:39 Robert Haas wrote:
 A more important point is whether we really need to make this
 dependent on Perl 5.6 or later.  What features are we using here that
 actually require Perl 5.6?  I suspect the answer is none, but we
 don't like writing the code in a way that is backward compatible to
 crufty, ancient versions of Perl, to which my response is get over
 it.  :-)   I always use the three-argument form of open() in all new
 code, but for fixed strings the two-argument form is just as good and
 has been supported for far longer.  Any suggestions that we should
 prefer clean code over portability are, in my opinion, non-starters.
I dont see a platform without perl 5.6 where a new enough flex/bison is 
available...

Andres

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


Re: [HACKERS] using separate parameters in psql query execution

2009-12-20 Thread Robert Haas
On Mon, Nov 16, 2009 at 5:01 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Hello

 now - complete patch

 ToDo:
 * enhance a documentation (any volunteer?)
 * check name for backslash command

I read through this patch tonight and I don't understand what the
point of this change is.  That's something that should probably be
discussed and also incorporated into the documentation.

...Robert

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


[HACKERS] Small Bug in GetConflictingVirtualXIDs

2009-12-20 Thread Andres Freund
Hi Simon, Hi all,


HS currently does the following in GetConflictingVirtualXIDs


TransactionId pxmin = proc-xmin;

/*
 * We ignore an invalid pxmin because this means that backend
 * has no snapshot and cannot get another one while we hold exclusive lock.
 */
if (TransactionIdIsValid(pxmin)  !TransactionIdFollows(pxmin, limitXmin))
{
VirtualTransactionId vxid;

GET_VXID_FROM_PGPROC(vxid, *proc);
if (VirtualTransactionIdIsValid(vxid))
vxids[count++] = vxid;
}


The logic behind this seems fine except in the case of dropping a database. 
There you very well might have a open connection without an open snapshot.

This leads to nice errors when youre connected to a database on the slave 
without having a in progress transaction while dropping the database on the 
primary:


CET FATAL:  terminating connection due to administrator command
CET LOG:  shutting down
CET LOG:  restartpoint starting: shutdown immediate
CET FATAL:  could not open file base/16604/1259: No such file or directory
CET CONTEXT:  writing block 5 of relation base/16604/1259
CET WARNING:  could not write block 5 of base/16604/1259
CET DETAIL:  Multiple failures --- write error might be permanent.
CET CONTEXT:  writing block 5 of relation base/16604/1259

Should easily be solvable through an extra parameter for 
GetConflictingVirtualXIDs. Want me to prepare a patch?

Andres

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


[HACKERS] Possible patch for better index name choosing

2009-12-20 Thread Tom Lane
Attached is a WIP patch for addressing the problems mentioned in this
thread:
http://archives.postgresql.org/pgsql-hackers/2009-12/msg01764.php

The main things that it does are (1) consider all index columns, not
just the first one as formerly; and (2) try to generate a usable name
for index expression columns, rather than just ignoring them which was
the effective behavior formerly.

There are several changes in the regression test outputs, mostly as a
result of choice (1).  I've not bothered to update the expected files
yet but just attached the output diffs to show what happens.

There is one thing that is not terribly nice about the behavior, which
is that CREATE TABLE LIKE INCLUDING INDEXES is unable to generate smart
names for expression indexes; it falls back to expr, as for example
in

regression=# create table foo (f1 text, exclude (lower(f1) with =));
NOTICE:  CREATE TABLE / EXCLUDE will create implicit index 
foo_lower_exclusion for table foo
CREATE TABLE
regression=# create table foo2 (like foo including all);
NOTICE:  CREATE TABLE / EXCLUDE will create implicit index 
foo2_expr_exclusion for table foo2
CREATE TABLE

The reason for this is that the patch depends on FigureColname which
works on untransformed parse trees, and we don't have access to such
a tree when copying an existing index.  There seem to be three possible
responses to that:

1. Decide this isn't worth worrying about and use the patch as-is.

2. Change FigureColname to work on already-transformed expressions.
I don't care for this idea much, for two reasons.  First, FigureColname
would become significantly slower (eg, it would have to do catalog
lookups to resolve names of Vars, instead of just pulling the name out
of a ColumnRef), and this is objectionable considering it's part of
the required parsing path for even very simple commands.  Second, there
are various corner cases where we'd get different results, which would
likely break applications that are expecting specific result column
names from given queries.

3. Implement a separate FigureIndexColname function that works as much
like FigureColname as it can, but takes a transformed parse tree.
This fixes the LIKE case and also removes the need for the iexprname
field that the attached patch adds to IndexElem.  I think it largely
overcomes the two objections to idea #2, since an extra few lookups
during index creation are hardly a performance problem, and exact
application compatibility shouldn't be an issue here either.  It's
a bit ugly to have to keep two such functions in sync though.

I'm not real sure whether to go with the patch as-is or use idea #3.
It seems to depend on how annoyed you are by the LIKE behavior.

A different consideration is whether it's really a good idea to be
messing with default index names at all.  As illustrated in the attached
regression diffs, this does impact the error messages returned to
applications for unique-index failures.  I don't think this is a serious
problem across a major version update, but maybe someone thinks
differently.

Comments?

regards, tom lane

Index: src/backend/bootstrap/bootparse.y
===
RCS file: /cvsroot/pgsql/src/backend/bootstrap/bootparse.y,v
retrieving revision 1.101
diff -c -r1.101 bootparse.y
*** src/backend/bootstrap/bootparse.y	7 Dec 2009 05:22:21 -	1.101
--- src/backend/bootstrap/bootparse.y	21 Dec 2009 02:47:04 -
***
*** 322,328 
  {
  	IndexElem *n = makeNode(IndexElem);
  	n-name = $1;
! 	n-expr = NULL;
  	n-opclass = list_make1(makeString($2));
  	n-ordering = SORTBY_DEFAULT;
  	n-nulls_ordering = SORTBY_NULLS_DEFAULT;
--- 322,329 
  {
  	IndexElem *n = makeNode(IndexElem);
  	n-name = $1;
! 	n-iexpr = NULL;
! 	n-iexprname = NULL;
  	n-opclass = list_make1(makeString($2));
  	n-ordering = SORTBY_DEFAULT;
  	n-nulls_ordering = SORTBY_NULLS_DEFAULT;
Index: src/backend/catalog/index.c
===
RCS file: /cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.326
diff -c -r1.326 index.c
*** src/backend/catalog/index.c	9 Dec 2009 21:57:50 -	1.326
--- src/backend/catalog/index.c	21 Dec 2009 02:47:04 -
***
*** 217,224 
  			indexpr_item = lnext(indexpr_item);
  
  			/*
! 			 * Make the attribute's name pg_expresssion_nnn (maybe think of
! 			 * something better later)
  			 */
  			sprintf(NameStr(to-attname), pg_expression_%d, i + 1);
  
--- 217,226 
  			indexpr_item = lnext(indexpr_item);
  
  			/*
! 			 * Make the attribute's name pg_expression_nnn (maybe think of
! 			 * something better later?  we're kind of locked in now, though,
! 			 * because pg_dump exposes these names when dumping comments on
! 			 * index columns)
  			 */
  			sprintf(NameStr(to-attname), pg_expression_%d, i + 1);
  
Index: src/backend/commands/indexcmds.c

Re: [HACKERS] Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread Tom Lane
Andres Freund and...@anarazel.de writes:
 On Monday 21 December 2009 02:23:39 Robert Haas wrote:
 A more important point is whether we really need to make this
 dependent on Perl 5.6 or later.

 I dont see a platform without perl 5.6 where a new enough flex/bison is 
 available...

That argument only holds water if you are willing to follow the same
rules as we use for flex/bison, ie, they are not needed to build from
a source tarball.  Otherwise this *is* moving the goalposts on required
tool support.

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: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread Andres Freund
On Monday 21 December 2009 04:23:57 Tom Lane wrote:
 Andres Freund and...@anarazel.de writes:
  On Monday 21 December 2009 02:23:39 Robert Haas wrote:
  A more important point is whether we really need to make this
  dependent on Perl 5.6 or later.
 
  I dont see a platform without perl 5.6 where a new enough flex/bison is
  available...
 
 That argument only holds water if you are willing to follow the same
 rules as we use for flex/bison, ie, they are not needed to build from
 a source tarball.  Otherwise this *is* moving the goalposts on required
 tool support.
The patch already does that if I understood John correctly.

Andres

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


Re: [HACKERS] [PATCH] remove redundant ownership checks

2009-12-20 Thread Robert Haas
On Thu, Dec 17, 2009 at 7:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 KaiGai Kohei kai...@ak.jp.nec.com writes:
 [ patch to remove EnableDisableRule's permissions check ]

 I don't particularly like this patch, mainly because I disagree with
 randomly removing permissions checks without any sort of plan about
 where they ought to go.

So where ought they to go?

 If we're going to start moving these checks around we need a very
 well-defined notion of where permissions checks should be made, so that
 everyone knows what to expect.  I have not seen any plan for that.

This seems to me to get right the heart of the matter.  When I
submitted my machine-readable explain patch, you critiqued it for
implementing half of an abstraction layer, and it seems to me that our
current permissions-checking logic has precisely the same issue.  We
consistently write code that starts by checking permissions and then
moves right along to implementing the action.  Those two things need
to be severed.  I see two ways to do this.  Given a function that (A)
does some prep work, (B) checks permissions, and (C) performs the
action, we could either:

1. Make the existing function do (A) and (B) and then call another
function to do (C), or
2. Make the existing function do (A), call another function to do (B),
and then do (C) itself.

I'm not sure which will work better, but I think making a decision
about which way to do it and how to name the functions would be a big
step towards having a coherent plan for this project.

A related issue is where parse analysis should be performed.  We're
not completely consistent about this right now.  Most of it seems to
be done by code in the parser directory, but there are several
exceptions, including DefineRule().

...Robert

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


Re: [HACKERS] Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread John Naylor
On Sun, Dec 20, 2009 at 7:24 PM, Andres Freund and...@anarazel.de wrote:
 On Monday 21 December 2009 04:23:57 Tom Lane wrote:
 Andres Freund and...@anarazel.de writes:
  On Monday 21 December 2009 02:23:39 Robert Haas wrote:
  A more important point is whether we really need to make this
  dependent on Perl 5.6 or later.
 
  I dont see a platform without perl 5.6 where a new enough flex/bison is
  available...

 That argument only holds water if you are willing to follow the same
 rules as we use for flex/bison, ie, they are not needed to build from
 a source tarball.  Otherwise this *is* moving the goalposts on required
 tool support.
 The patch already does that if I understood John correctly.

Yes, everything output by Perl in my patch is a distprep target.

Some minor changes would enable it to work on 5.0, but I'd like to
make sure that's necessary.

So, what Perl version should be targeted for those building from CVS,
and is that already documented somewhere? Does anyone know the
earliest version on the build farm?

John

-- 
Sent 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: Remove all declarations from pg_attribute.h, consolidate BKI scripts

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 10:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Andres Freund and...@anarazel.de writes:
 On Monday 21 December 2009 02:23:39 Robert Haas wrote:
 A more important point is whether we really need to make this
 dependent on Perl 5.6 or later.

 I dont see a platform without perl 5.6 where a new enough flex/bison is
 available...

 That argument only holds water if you are willing to follow the same
 rules as we use for flex/bison, ie, they are not needed to build from
 a source tarball.  Otherwise this *is* moving the goalposts on required
 tool support.

I believe that we have long had agreement on making the relevant files
distprep targets, so this will not be an issue.  Anyway, the whole
thing is a silly argument anyway: we can certainly make this
compatible back even as far as Perl 5.0 if need be for very little
extra work.

What is worth a little bit of effort to establish is exactly what
version of Perl we're already depending on, so that we can document
that for the benefit of future tool writers.  There's no reason why
this particular thing needs to be compatible further back than what is
already required otherwise.

...Robert

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


Re: [HACKERS] Possible patch for better index name choosing

2009-12-20 Thread Robert Haas
On Sun, Dec 20, 2009 at 10:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Attached is a WIP patch for addressing the problems mentioned in this
 thread:
 http://archives.postgresql.org/pgsql-hackers/2009-12/msg01764.php

 The main things that it does are (1) consider all index columns, not
 just the first one as formerly; and (2) try to generate a usable name
 for index expression columns, rather than just ignoring them which was
 the effective behavior formerly.

I'm not really sure there's any point to this.  Anyone who cares about
giving their index an intelligible name should manually assign one.
If they don't bother doing that, I don't really see why we should
worry about it either.  If anything, it seems like we should err on
the side of simplicity, since some users (or even applications) might
attempt to identify or predict automatically generated names.

 A different consideration is whether it's really a good idea to be
 messing with default index names at all.  As illustrated in the attached
 regression diffs, this does impact the error messages returned to
 applications for unique-index failures.  I don't think this is a serious
 problem across a major version update, but maybe someone thinks
 differently.

Maybe I'll reserve final judgement pending further discussion, but my
first reaction is to say it's not worth the risk.  Probably this
shouldn't be an issue for a well-designed application, but the world
is full of badly-written code.  We shouldn't throw up barriers (even
relatively trivial ones) to updating applications unless we get
something out of it, and I'm not convinced that's the case here.

...Robert

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


Re: [HACKERS] Removing pg_migrator limitations

2009-12-20 Thread Bruce Momjian
Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  I agree that pulling pg_migrator into contrib seems pretty sensible.
  What I want to make sure we're on the same page about is which
  versions the 8.5 pg_migrator will allow you to upgrade from and to.  I
  think we should at least support 8.3 - 8.5 and 8.4 - 8.5.  If you're
  saying we don't need to support 8.3 - 8.4 any more once 8.5 comes
  out, I'm probably OK with that, but perhaps we should try to get a few
  more opinions before setting that policy in stone.
 
 If we can do that reasonably (which might well be the case), I'd be for
 it.  What I'm objecting to is what I take to be Bruce's plan of
 supporting 8.3 - 8.4 and 8.4 - 8.5 with the same pg_migrator sources.
 The stuff we're talking about doing in this thread is going to cause
 those two cases to diverge rather drastically.  8.3 - 8.5 and 8.4 -
 8.5 may be close enough together to be reasonable to support in one
 set of source code.

Basically there isn't much extra work to go from 8.3 to 8.4 compared to
8.3 to 8.5.  Now, if could support only 8.4 to 8.5 I could remove some
code, but that seems counterproductive.

The other problem with moving to /contrib is that I can't put out
pg_migrator updates independently of the main community release, which
could be bad.

I am glad some people think pg_migrator is ready for /contrib.

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

  + If your life is a hard drive, Christ can be your backup. +

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


[HACKERS] Re: Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Pavel Stehule
2009/12/21 Greg Stark gsst...@mit.edu:
 Incidentally there are O(n) algorithms for finding the median (or any
 specific offset). It shouldn't be necessary to sort at all.

it is interesting information. It could to help with missing
optimalisations now.

Pavel


 I'm not sure which path this argues for - perhaps Tom's position that
 we need more optimiser infrastructure so we can see how to accomplish
 this. Perhaps it means you should really implement median() with an
 internal selection alagorithm and not depend on the optimizer at all.


 --
 greg


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


Re: [HACKERS] Possible patch for better index name choosing

2009-12-20 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sun, Dec 20, 2009 at 10:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Attached is a WIP patch for addressing the problems mentioned in this
 thread:
 http://archives.postgresql.org/pgsql-hackers/2009-12/msg01764.php

 I'm not really sure there's any point to this.  Anyone who cares about
 giving their index an intelligible name should manually assign one.
 If they don't bother doing that, I don't really see why we should
 worry about it either.

Mainly because we historically *have* put some work into it, and it
would be inconsistent to not pay attention to the point now as we extend
the set of possible index-building constraints further.  In particular
we're going to see a lot of exclusion constraints named foo_exclusionN
if we don't expend any effort on it now.  I also claim that this is
necessary infrastructure if we are going to accept Peter's proposal of
allowing CREATE INDEX without an explicit index name.  That is really
dependent on the assumption that the system will expend more than no
effort on picking useful names.

 Maybe I'll reserve final judgement pending further discussion, but my
 first reaction is to say it's not worth the risk.  Probably this
 shouldn't be an issue for a well-designed application, but the world
 is full of badly-written code.  We shouldn't throw up barriers (even
 relatively trivial ones) to updating applications unless we get
 something out of it, and I'm not convinced that's the case here.

Well, we could tamp down the risks considerably if we undid my point
(1), namely to still consider only the first index column when
generating a name.  I am not really happy with that answer though.
I could turn your first point back on you: if an app is concerned about
the exact names assigned to indexes, why isn't it specifying them?

It's worth noting that pg_dump does preserve index names, so this isn't
going to be an issue in any case for existing apps that dump and reload
their databases.  AFAICS the only case where it would actually create a
compatibility issue is if an existing app creates multi-column UNIQUE
(non-PKEY) constraints on-the-fly, without a constraint name, and
depends on the generated name being the same as before.

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] Removing pg_migrator limitations

2009-12-20 Thread Marc G. Fournier

On Sun, 20 Dec 2009, Bruce Momjian wrote:

The other problem with moving to /contrib is that I can't put out 
pg_migrator updates independently of the main community release, which 
could be bad.


Why not?


Marc G. FournierHub.Org Hosting Solutions S.A.
scra...@hub.org http://www.hub.org

Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org

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


Re: [HACKERS] Removing pg_migrator limitations

2009-12-20 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Basically there isn't much extra work to go from 8.3 to 8.4 compared to
 8.3 to 8.5.

That might be true today, but it will stop being true once we replace
the oid/relfilenode management hac^Wcode with the proposed new approach.

 The other problem with moving to /contrib is that I can't put out
 pg_migrator updates independently of the main community release, which
 could be bad.

That was a good thing while pg_migrator was in its wild west
development stage.  But if you have ambitions that people should trust it
enough to risk their production DBs on it, then it had better be stable
enough for this not to be a big drawback.

Also note the point about how it'll be a lot easier to keep it in sync
with pg_dump and backend behavior if it's only got to work with the
pg_dump version that's in the same release.  Again, the proposed changes
tie it to a particular pg_dump and target backend version noticeably
more than it was before; so if you try to keep it separate this is going
to be an even bigger headache than it already was during the run-up to
8.4.

Lastly, getting pg_migrator working reliably would be a sufficiently
Big Deal that I think a critical pg_migrator bug would be sufficient
grounds for forcing a minor release, just as we sometimes force a
release for critical backend bugs.

 I am glad some people think pg_migrator is ready for /contrib.

To be clear, I don't think it's really ready for contrib today.  I think
it could be up to that level by the time 8.5 releases, but we have to
get serious about it, and stop pretending it's an arm's-length project
the core project doesn't really care about.

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] Possible patch for better index name choosing

2009-12-20 Thread Robert Haas
On Mon, Dec 21, 2009 at 12:03 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sun, Dec 20, 2009 at 10:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Attached is a WIP patch for addressing the problems mentioned in this
 thread:
 http://archives.postgresql.org/pgsql-hackers/2009-12/msg01764.php

 I'm not really sure there's any point to this.  Anyone who cares about
 giving their index an intelligible name should manually assign one.
 If they don't bother doing that, I don't really see why we should
 worry about it either.

 Mainly because we historically *have* put some work into it, and it
 would be inconsistent to not pay attention to the point now as we extend
 the set of possible index-building constraints further.  In particular
 we're going to see a lot of exclusion constraints named foo_exclusionN
 if we don't expend any effort on it now.

Maybe that's worth fixing and maybe it isn't.  My first reaction is
so what?  In all likelihood, you're going to have to look at the
index definition to see what the thing does anyway.

 I also claim that this is
 necessary infrastructure if we are going to accept Peter's proposal of
 allowing CREATE INDEX without an explicit index name.  That is really
 dependent on the assumption that the system will expend more than no
 effort on picking useful names.

That's a point to consider, though perhaps if they aren't specifying a
name it means they don't care that much.

 Maybe I'll reserve final judgement pending further discussion, but my
 first reaction is to say it's not worth the risk.  Probably this
 shouldn't be an issue for a well-designed application, but the world
 is full of badly-written code.  We shouldn't throw up barriers (even
 relatively trivial ones) to updating applications unless we get
 something out of it, and I'm not convinced that's the case here.

 Well, we could tamp down the risks considerably if we undid my point
 (1), namely to still consider only the first index column when
 generating a name.  I am not really happy with that answer though.
 I could turn your first point back on you: if an app is concerned about
 the exact names assigned to indexes, why isn't it specifying them?

 It's worth noting that pg_dump does preserve index names, so this isn't
 going to be an issue in any case for existing apps that dump and reload
 their databases.  AFAICS the only case where it would actually create a
 compatibility issue is if an existing app creates multi-column UNIQUE
 (non-PKEY) constraints on-the-fly, without a constraint name, and
 depends on the generated name being the same as before.

Right.  Imagine, for example, a poorly written initialization script
for an app.  Existing instances that are dumped and reloaded will be
OK, but new instances might not come out as expected.

I don't think that what you're proposing here is completely stupid;
I'm just wondering if it's not an ultimately somewhat pointless
activity.  I'm not convinced that it's possible or sensible to try to
stringify all the things that people put in their index definitions,
or that we're going to be able to do it well enough to really add any
value.Perhaps I should RTFP before sticking my neck out too far,
but... will you serialize EXCLUDE (a =), EXCLUDE (a ), and EXCLUDE
(a some other operator) differently?  And if so, do you expect the
user to be able to reconstruct what the constraint is doing by looking
at the serialized version?  It seems like something reasonably sane
can be done when the definition uses mostly column names and
functions, but operators seem like more of a problem.  I think mostly
people are going to see the constraint name that got violated and then
run \d on the table and look for it.  foo_exclusion3 may not be very
informative, but it's easy to remember for long enough to find it in
the \d output, whereas something long and hairy may not be.

...Robert

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


Re: [HACKERS] Removing pg_migrator limitations

2009-12-20 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Basically there isn't much extra work to go from 8.3 to 8.4 compared to
  8.3 to 8.5.
 
 That might be true today, but it will stop being true once we replace
 the oid/relfilenode management hac^Wcode with the proposed new approach.
 
  The other problem with moving to /contrib is that I can't put out
  pg_migrator updates independently of the main community release, which
  could be bad.
 
 That was a good thing while pg_migrator was in its wild west
 development stage.  But if you have ambitions that people should trust it
 enough to risk their production DBs on it, then it had better be stable
 enough for this not to be a big drawback.
 
 Also note the point about how it'll be a lot easier to keep it in sync
 with pg_dump and backend behavior if it's only got to work with the
 pg_dump version that's in the same release.  Again, the proposed changes
 tie it to a particular pg_dump and target backend version noticeably
 more than it was before; so if you try to keep it separate this is going
 to be an even bigger headache than it already was during the run-up to
 8.4.
 
 Lastly, getting pg_migrator working reliably would be a sufficiently
 Big Deal that I think a critical pg_migrator bug would be sufficient
 grounds for forcing a minor release, just as we sometimes force a
 release for critical backend bugs.
 
  I am glad some people think pg_migrator is ready for /contrib.
 
 To be clear, I don't think it's really ready for contrib today.  I think
 it could be up to that level by the time 8.5 releases, but we have to
 get serious about it, and stop pretending it's an arm's-length project
 the core project doesn't really care about.

OK, I am convinced.

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

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Possible patch for better index name choosing

2009-12-20 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Perhaps I should RTFP before sticking my neck out too far,
 but... will you serialize EXCLUDE (a =), EXCLUDE (a ), and EXCLUDE
 (a some other operator) differently?

No, and I'm not proposing to expose ASC/DESC/NULLS FIRST/LAST or
nondefault opclasses (to say nothing of non-btree AMs) or index
predicates either.  The proposed patch is to my mind just a logical
extension of what we have always done --- namely, to pay attention
to index column names --- to some new cases that were never exposed
before.

We could certainly make it pay attention to all that stuff, but I have
the same feeling you do that it wouldn't produce readable results.
And it would make any compatibility issues a lot worse.

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] Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support

2009-12-20 Thread Pavel Stehule
2009/12/20 Andrew Gierth and...@tao11.riddles.org.uk:
 Pavel == Pavel Stehule pavel.steh...@gmail.com writes:

   2009/12/20 Tom Lane t...@sss.pgh.pa.us:
   I think that we've already expanded the capabilities of aggregates
   a great deal for 8.5, and we should let it sit as-is for a release
   or two and see what the real user demand is for additional
   features.
  
   I'm particularly concerned by the fact that the feature set is
   already far out in front of what the planner can optimize
   effectively (e.g., there's no ability to combine the work when
   multiple aggregates need the same sorted data).  The more features
   we add on speculation, the harder it's going to be to close that
   gap.

 I absolutely agree with Tom here and for some quite specific reasons.

 An optimal (or at least more optimal than is currently possible)
 implementation of median() on top of the ordered-agg code as it stands
 requires additions to the aggregate function interface: the median agg
 implementation would have to, as a minimum, know how many rows of
 sorted input are available. In addition, it would be desirable for it
 to have direct (and possibly bidirectional) access to the tuplesort.

I was thinking about some new kind of aggregates based on tuple-store.



 Now, if we look at how ordered aggs ought to be optimized, it's clear
 that the planner should take the ordering costs into account and
 consider plans that order the input instead. Once you do this, then
 there's no longer any pre-computed count or tuplesort object
 available, so if you'd implemented a better median() before the
 optimizations, you'd end up either having to forgo the optimization or
 break the median() agg again; clearly not something we want.


Information about count of rows are not detected in planner time. This
 have to by available in any executor's sort node. So this value will
be available every time - index scan is problem. Interesting is Greg's
info about O(N) algorithms. Then we can implement median as classic
aggregate.

 Plus, a feature that I intentionally omitted from the ordered-aggs
 patch is the ability to do ordered-aggs as window functions. This is
 in the spec, but (a) there were conflicting patches for window
 functions on the table and (b) in my opinion, much of the work needed
 to implement ordered-agg-as-window-func in an effective manner is
 dependent on doing more work on optimization first (or at least will
 potentially become easier as a result of that work).


I fully agree, so agg(x order by x) needs some work more - so general
design is premature.

On second hand - any internal implementation of median will be faster,
then currently used techniques.

Pavel

 So I think both the optimization issue and the window functions issue
 would be best addressed before trying to build any additional features
 on top of what we have so far.

 --
 Andrew (irc:RhodiumToad)


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


Re: [HACKERS] using separate parameters in psql query execution

2009-12-20 Thread Pavel Stehule
2009/12/21 Robert Haas robertmh...@gmail.com:
 On Mon, Nov 16, 2009 at 5:01 PM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 Hello

 now - complete patch

 ToDo:
 * enhance a documentation (any volunteer?)
 * check name for backslash command

 I read through this patch tonight and I don't understand what the
 point of this change is.  That's something that should probably be
 discussed and also incorporated into the documentation.

Do you ask about ToDo points?

I used pexec as switch. Probably better name is

parametrized-execution, send-parameters-separately or parametrized-queries

general goal of this patch is removing issues with  variables quoting
- using psql variables should be more robust and more secure.

I checked second design based on enhanced syntax -
http://www.postgres.cz/index.php/Enhanced-psql#Variables_quoting . It
working too, but it needs one exec more.

Regards
Pavel


 ...Robert


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