Re: [HACKERS] Schema version management

2012-07-05 Thread Joel Jacobson
On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas robertmh...@gmail.com wrote:

 My vote is - when there's an overloaded function, put each version in
 its own file.  And name the files something like
 functionname_something.sql.  And just document that something may not
 be entirely stable.


I would agree that's better if the dump order isn't deterministic.

However, it looks like an easy fix to make the dump order deterministic:
http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php

If the dump order is deterministic, I think its cleaner to put all versions
in the same file.

Benefits:
+ Pretty looking filename
+ Same file structure for all object types, no special exception for
functions


Re: [HACKERS] Schema version management

2012-07-05 Thread Gurjeet Singh
On Thu, Jul 5, 2012 at 3:15 AM, Joel Jacobson j...@trustly.com wrote:

 On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas robertmh...@gmail.com wrote:

 My vote is - when there's an overloaded function, put each version in
 its own file.  And name the files something like
 functionname_something.sql.  And just document that something may not
 be entirely stable.


 I would agree that's better if the dump order isn't deterministic.

 However, it looks like an easy fix to make the dump order deterministic:
 http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php

 If the dump order is deterministic, I think its cleaner to put all
 versions in the same file.

 Benefits:
 + Pretty looking filename
 + Same file structure for all object types, no special exception for
 functions


I think there's a merit to keeping all overloaded variations of a function
in a single file, apart from the simplicity and benefits noted above. A
change in one variation of the function may also be applicable to other
variations, say in bug-fixes or enhancements. So keeping all variations in
one file would make sense, since it is logically one object.

Best regards,
-- 
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company


Re: [HACKERS] pgsql_fdw in contrib

2012-07-05 Thread Shigeru HANADA
On Tue, Jun 26, 2012 at 10:50 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
   * Regarding to deparseSimpleSql(), it pulls attributes being referenced
 from baserestrictinfo and reltargetlist using pull_var_clause().
 Is it unavailable to use local_conds instead of baserestrictinfo?
 We can optimize references to the variable being consumed at the
 remote side only. All we need to transfer is variables referenced
 in targetlist and local-clauses.

Indeed, fixed.

 In addition, is pull_var_clause() reasonable to list up all the attribute
 referenced at the both expression tree? It seems to be pull_varattnos()
 is more useful API in this situation.

Only for searching, yes.  However, sooner or later we need Var objects
to deparse remote SELECT clause, so pull_var_clause seems better here.
ISTM one of advantage to use bitmap is matching with another bitmap,
like index only scan code checks whether all used attributes is
contained by indexed attributes.

   * Regarding to deparseRelation(), it scans simple_rte_array to fetch
 RangeTblEntry with relation-id of the target foreign table.
 It does not match correct entry if same foreign table is appeared in
 this list twice or more, like a case of self-join. I'd like to recommend
 to use simple_rte_array[baserel-relid] instead.

Reasonable idea.  After some thoughts, I think that deparseRelation
should receive RangeTblEntry instead of relation oid (and then
PlannerInfo is not necessary).

 In addition, it checks whether relkind is RELKIND_FOREIGN_TABLE,
 or not. It seems to me this check should be Assert(), if routines of
 pgsql_fdw is called towards regular relations.

   * Regarding to deparseVar(), is it unnecessary to check relkind of
 the relation being referenced by Var node, isn't it?

IIRC, this is remain of past trial for general deparser...  Removed
unnecessary relkind checks.  Fixed.

   * Regarding to deparseBoolExpr(), compiler raised a warning
 because op can be used without initialized.

Fixed.

   * Even though it is harmless, sortConditions() is a misleading function
 name. How about categolizeConditions() or screeningConditions()?

How about classifyConditions?
# I hope native English speaker's help for wording issue... :-)

Regards,
-- 
Shigeru Hanada

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


Re: [HACKERS] [BUGS] Tab completion of function arguments not working in all cases

2012-07-05 Thread Magnus Hagander
On Mon, Jun 18, 2012 at 5:45 PM, Josh Kupershmidt schmi...@gmail.com wrote:
 On Mon, Jun 18, 2012 at 3:56 AM, Dean Rasheed dean.a.rash...@gmail.com 
 wrote:
 On 18 June 2012 04:21, Josh Kupershmidt schmi...@gmail.com wrote:

 As a side note unrelated to this patch, I also dislike how function
 name tab-completions will not fill in the opening parenthesis, which
 makes for unnecessary work for the user, as one then has to type the
 parenthesis and hit tab again to get possible completions for the
 function arguments. The current behavior causes:
  DROP FUNCTION my_fTAB

 which completes to:
  DROP FUNCTION my_function

 enter parenthesis, and hit tab:
  DROP FUNCTION my_function(TAB

 which, if there is only one match, could complete to:
  DROP FUNCTION my_function(integer)

 when the last three steps could have been consolidated with better
 tab-completion. Perhaps this could be a TODO.


 Hmm, I find that it does automatically fill in the opening
 parenthesis, but only once there is a space after the completed
 function name. So
 DROP FUNCTION my_fTAB completes to DROP FUNCTION my_function 
 (note the space at the end). Then pressing TAB one more time gives
 DROP FUNCTION my_function ( , and then pressing TAB again gives
 the function arguments.

 Alternatively DROP FUNCTION my_functionTAB (no space after
 function name) first completes to DROP FUNCTION my_function  (adding
 the space), and then completes with the opening parenthesis, and then
 with the function arguments.

 It's a bit clunky, but I find that repeatedly pressing TAB is easier
 than typing the opening bracket.

 Interesting, I see the behavior you describe on Linux, using psql +
 libreadline, and the behavior I showed (i.e. repeatedly pressing tab
 won't automatically fill in the function arguments after the function
 name is completed, seemingly because no space is deposited after the
 completed function name)  is with OS X 10.6, using psql + libedit.

 [snip]
 you get tab-completions for both text) and bytea(, when you
 probably expected only the former. That's easy to fix though, please
 see attached v2 patch.

 Good catch.
 I think that's a useful additional test, and is also consistent with
 the existing code in Query_for_list_of_attributes.

 OK, I'll mark Ready for Committer in the CF.

Applied with minor revisions:
* the comment above the COMPLETE_WITH_xyz macros needed an update
* I renamed the macro to COMPLETE_WITH_FUNCTION_ARG - to make it even
more clear what it does

Thanks!

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

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


Re: [HACKERS] [PATCH] pg_dump: Sort overloaded functions in deterministic order

2012-07-05 Thread Joel Jacobson
New version, made a typo in last one.


pg_dump_deterministic_order_v3.patch
Description: Binary data

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Andrew Dunstan
On Thu, Jul 5, 2012 at 4:04 AM, Gurjeet Singh singh.gurj...@gmail.comwrote:

 On Thu, Jul 5, 2012 at 3:15 AM, Joel Jacobson j...@trustly.com wrote:

 On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas robertmh...@gmail.comwrote:

 My vote is - when there's an overloaded function, put each version in
 its own file.  And name the files something like
 functionname_something.sql.  And just document that something may not
 be entirely stable.


 I would agree that's better if the dump order isn't deterministic.

 However, it looks like an easy fix to make the dump order deterministic:
 http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php

 If the dump order is deterministic, I think its cleaner to put all
 versions in the same file.

 Benefits:
 + Pretty looking filename
 + Same file structure for all object types, no special exception for
 functions


 I think there's a merit to keeping all overloaded variations of a function
 in a single file, apart from the simplicity and benefits noted above. A
 change in one variation of the function may also be applicable to other
 variations, say in bug-fixes or enhancements. So keeping all variations in
 one file would make sense, since it is logically one object.



No they are not necessarily one logical unit. You could have a bunch of
functions called, say, equal which have pretty much nothing to do with
each other, since they refer to different types.

+1 from me for putting one function definition per file.

cheers

andrew


Re: [HACKERS] Schema version management

2012-07-05 Thread Michael Glaesemann

On Jul 5, 2012, at 9:21, Andrew Dunstan wrote:

 No they are not necessarily one logical unit. You could have a bunch of
 functions called, say, equal which have pretty much nothing to do with
 each other, since they refer to different types.
 
 +1 from me for putting one function definition per file.

+1. It might make sense to include some sort of argument type information. The 
function signature is
really its identifier. The function name is only part of it.

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] Schema version management

2012-07-05 Thread Vik Reykja
On Thu, Jul 5, 2012 at 3:32 PM, Michael Glaesemann g...@seespotcode.netwrote:


 On Jul 5, 2012, at 9:21, Andrew Dunstan wrote:

  No they are not necessarily one logical unit. You could have a bunch of
  functions called, say, equal which have pretty much nothing to do with
  each other, since they refer to different types.
 
  +1 from me for putting one function definition per file.

 +1. It might make sense to include some sort of argument type information.
 The function signature is
 really its identifier. The function name is only part of it.


I'll go against the flow here.  I would prefer to have all overloaded
functions in the same file.


Re: [HACKERS] Schema version management

2012-07-05 Thread Joel Jacobson
Maybe it could be made an option to pg_dump?

Some users and their systems might not even have overloaded functions,
and these users will of course prefer a nice looking filename, i.e. all
functions
having the same name kept in the same file. Which for them will mean only
one function per file anyway.

pg_dump --split --overloaded-functions-to-same-file

Other users and their systems might have a lot of overloaded functions,
like the equal() example mentioned, they will of course prefer to keep
all functions in separate files.

pg_dump --split --overloaded-functions-to-separate-files

Then, one can discuss which one should be the default option for --split,
I would prefer the same file variant, and think most other users would too,
except for users with a lot of overloaded functions.


Re: [HACKERS] Schema version management

2012-07-05 Thread Tom Lane
Joel Jacobson j...@trustly.com writes:
 Maybe it could be made an option to pg_dump?

Ick.  Then we have to deal with all the downsides of *both* methods.

pg_dump is already a bloated, nearly unmaintainable mess.  The very
last thing it needs is even more options.

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] Schema version management

2012-07-05 Thread Joel Jacobson
On Thu, Jul 5, 2012 at 4:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Ick.  Then we have to deal with all the downsides of *both* methods.

 pg_dump is already a bloated, nearly unmaintainable mess.  The very
 last thing it needs is even more options.


When you say bloated, are you referring to the code or the command line
interface?

If you are referring to the code, I don't think that's a good argument
against implementing new good features.
The important ratio is the value of a feature compared to the increased
complexity.
In this case, it's very simple to implement both the --split option and the
fixing of dump order.
I'm not even a C coder and managed to implement it within less of an hour
effective coding.
We are talking ~100 lines of code, with comments and everything.

If you are referring to the command line interface and think it is bloated,
maybe the options should be hidden in the normal --help.
We could create a new --help-advanced text, where we could put these
options, and all other existing less common options.
I think this is a quite common and good way to handle the situation for
UNIX command line tools.


Re: [HACKERS] Schema version management

2012-07-05 Thread Alvaro Herrera

Excerpts from Tom Lane's message of jue jul 05 10:46:52 -0400 2012:
 Joel Jacobson j...@trustly.com writes:
  Maybe it could be made an option to pg_dump?
 
 Ick.  Then we have to deal with all the downsides of *both* methods.
 
 pg_dump is already a bloated, nearly unmaintainable mess.  The very
 last thing it needs is even more options.

Agreed.

However I am also against what seems to be the flow.  Normally, you
don't write overloaded plpgsql functions such as equal.  Case in
point, the equality functions in core have funny names like int4eq and
so on.  Instead, at least in my experience, the overloaded functions
people seem to have in their databases are like do_stuff_to_foobars()
and you have one version for foos and another one for bars.

If you're doing lots of equality functions, surely it would make more
sense to package them up as an extension anyway along with all the other
thingies you need for the type you're supposedly writing.  So it's a
completely different market than what we're aiming at here.

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

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Joel Jacobson
On Thu, Jul 5, 2012 at 5:17 PM, Alvaro Herrera
alvhe...@commandprompt.comwrote:

 Agreed.

 However I am also against what seems to be the flow.  Normally, you
 don't write overloaded plpgsql functions such as equal.  Case in
 point, the equality functions in core have funny names like int4eq and
 so on.  Instead, at least in my experience, the overloaded functions
 people seem to have in their databases are like do_stuff_to_foobars()
 and you have one version for foos and another one for bars.

 If you're doing lots of equality functions, surely it would make more
 sense to package them up as an extension anyway along with all the other
 thingies you need for the type you're supposedly writing.  So it's a
 completely different market than what we're aiming at here.


True, very true, I didn't think about that, you are right, I fully agree.
My vote is therefore on the put all overloaded functions in the same file
variant.


Re: [HACKERS] Schema version management

2012-07-05 Thread Michael Glaesemann

On Jul 5, 2012, at 11:17, Alvaro Herrera wrote:

 
 Excerpts from Tom Lane's message of jue jul 05 10:46:52 -0400 2012:
 Joel Jacobson j...@trustly.com writes:
 Maybe it could be made an option to pg_dump?
 
 Ick.  Then we have to deal with all the downsides of *both* methods.
 
 pg_dump is already a bloated, nearly unmaintainable mess.  The very
 last thing it needs is even more options.
 
 Agreed.
 
 However I am also against what seems to be the flow.  Normally, you
 don't write overloaded plpgsql functions such as equal.

I often write functions that perform fetches based on different criteria.
For example, 

-- returns count of all orders for the given customer
int function order_count(in_customer_name text)

-- returns count of all orders for the given customer since the given timestamp
int function order_count(in_customer_name text, in_since timestamp with time 
zone)

-- returns count of orders for the given customer during a given interval
int function order_count(in_customer_name text, in_from timestamp with time 
zone, in_through timestamp with time zone) 


Or, I'll write overloaded functions, one of which provides default values.

-- returns the set of members whose birthday is today. Calls 
birthday_members(CURRENT_DATE)
setof record function birthday_members()

-- returns the set of members whose birthday is on the given date, which makes 
testing a lot easier
setof record function birthday_members(in_date DATE)

Some may disagree that this is a proper usage of function overloading.
Some may even argue that function names shouldn't be overloaded at all.
However, I find this usage of function name overloading useful, especially
for keeping function names relatively short.

If we're dumping objects (tables, views, functions, what-have-you) into 
separate files,
each of these functions is a separate object and should be in its own file.

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] Schema version management

2012-07-05 Thread Alvaro Herrera

Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
 
 On Jul 5, 2012, at 11:17, Alvaro Herrera wrote:

  However I am also against what seems to be the flow.  Normally, you
  don't write overloaded plpgsql functions such as equal.
 
 I often write functions that perform fetches based on different criteria.
 For example, 
 
 -- returns count of all orders for the given customer
 int function order_count(in_customer_name text)
 
 -- returns count of all orders for the given customer since the given 
 timestamp
 int function order_count(in_customer_name text, in_since timestamp with time 
 zone)
 
 -- returns count of orders for the given customer during a given interval
 int function order_count(in_customer_name text, in_from timestamp with time 
 zone, in_through timestamp with time zone) 

Isn't this a perfect example of stuff that, since it does much the same
thing, should be in the same file so that you remember to fix them all
together if you find a bug in one?

 Or, I'll write overloaded functions, one of which provides default values.
 
 -- returns the set of members whose birthday is today. Calls 
 birthday_members(CURRENT_DATE)
 setof record function birthday_members()
 
 -- returns the set of members whose birthday is on the given date, which 
 makes testing a lot easier
 setof record function birthday_members(in_date DATE)

Same.  This seems particularly the case if one implementation calls
another, more general one.

 Some may disagree that this is a proper usage of function overloading.
 Some may even argue that function names shouldn't be overloaded at all.
 However, I find this usage of function name overloading useful, especially
 for keeping function names relatively short.

I completely agree.

 If we're dumping objects (tables, views, functions, what-have-you) into 
 separate files,
 each of these functions is a separate object and should be in its own file.

Clearly there is no consensus here.

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

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Michael Glaesemann

On Jul 5, 2012, at 11:52, Alvaro Herrera wrote:

 Isn't this a perfect example of stuff that, since it does much the same
 thing, should be in the same file so that you remember to fix them all
 together if you find a bug in one?

That's what tests are for.

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] Schema version management

2012-07-05 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
 If we're dumping objects (tables, views, functions, what-have-you) into 
 separate files,
 each of these functions is a separate object and should be in its own file.

 Clearly there is no consensus here.

FWIW, I'm attracted to the all-similarly-named-functions-together
method, mainly because it dodges the problem of how to encode a
function's argument list into a filename.  However, we're being
short-sighted to only think of functions here.  What about operators?
Or casts?  Those don't have simple names either.

regards, tom lane

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Joel Jacobson
On Thu, Jul 5, 2012 at 5:59 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 FWIW, I'm attracted to the all-similarly-named-functions-together
 method, mainly because it dodges the problem of how to encode a
 function's argument list into a filename.  However, we're being
 short-sighted to only think of functions here.  What about operators?
 Or casts?  Those don't have simple names either.


Someone suggested to urlencode them. I think that's a quite good solution.

Personally, I don't have any user-defined operators or casts. Don't know
how common it is in general, but it must of course work for these as well.


Re: [HACKERS] Schema version management

2012-07-05 Thread Tom Lane
Joel Jacobson j...@trustly.com writes:
 On Thu, Jul 5, 2012 at 4:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 pg_dump is already a bloated, nearly unmaintainable mess.  The very
 last thing it needs is even more options.

 If you are referring to the code, I don't think that's a good argument
 against implementing new good features.
 The important ratio is the value of a feature compared to the increased
 complexity.

Well, to be perfectly frank, I already doubt that this entire feature
passes the complexity-versus-value test, because pg_dump is not a
substitute for an SCM --- people who have got enough functions to need
this sort of thing need to be keeping them somewhere else than in dump
files.  Complicating things more by supporting multiple ways of doing it
will make that worse.  I think you need to pick one design and stick
with it, not try to paint the bikeshed every color suggested by anybody.

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] Schema version management

2012-07-05 Thread Christopher Browne
On Thu, Jul 5, 2012 at 11:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
 If we're dumping objects (tables, views, functions, what-have-you) into 
 separate files,
 each of these functions is a separate object and should be in its own file.

 Clearly there is no consensus here.

 FWIW, I'm attracted to the all-similarly-named-functions-together
 method, mainly because it dodges the problem of how to encode a
 function's argument list into a filename.  However, we're being
 short-sighted to only think of functions here.  What about operators?
 Or casts?  Those don't have simple names either.

If you stow them all together, that still leaves a question as to
whether or not they get stored in a deterministic order.

I was recently working on something of the same issue as part of a
schema differencing tool.  It was pointedly *not* sufficient to use
the internal name (e.g. - information_schema.routines.specific_name),
as I wanted to compare things between databases, and it's pretty
certain that oids will differ.

I wound up expanding the function arguments and using function + args
as the name.  That leads to a risk of rather long names for functions,
but there aren't many other ways possible.

Note that pg_autodoc http://www.rbt.ca/autodoc/ takes a similar
approach; it attaches function labels based on function + args.

Here's an expanded example in the Slony docs:

http://slony.info/documentation/2.1/function.ddlscript-prepare-int-p-only-on-node-integer-p-set-id-integer.html

I wouldn't mind stowing functions together in one file, and I'd
actually not get too bent out of shape if the order was somewhat
nondeterministic.  But something like the autodoc naming seems like
the unambiguous answer.  Long, but unambiguous...
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?

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


Re: [HACKERS] Event Triggers reduced, v1

2012-07-05 Thread Robert Haas
On Wed, Jul 4, 2012 at 1:56 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
 Dimitri Fontaine dimi...@2ndquadrant.fr writes:
 Robert Haas robertmh...@gmail.com writes:
 No, I'm not asking you to add any more columns right now (in fact,
 please do not).  But the type of the existing column should change to
 text[].

 Ok, done in the attached. We now read text from either the user input in
 the grammar or from the catalogs (in a 1-D array there), and convert to
 our enum structure for internal code use.

 In the previous one I missed adapting pg_dump and psql, here's the
 updated set of patches. Sorry about that. This unexpected electrical
 shut down in the middle of the day was not cool. Nor was the second one.

Thanks.  There are some review comments from previous rounds that
don't seem to have been fully incorporated to this version:

- You only changed the definition of pg_event_triggers.evttags, not
the documentation.  I don't think we need pg_evttag_to_string aka
pg_event_trigger_command_to_string any more either.
- When you removed format_type_be_without_namespace, you didn't remove
either the function prototype or the related changes in format_type.c.
- I'm pretty sure that a previous review mentioned the compiler
warning in evtcache.c, which seems not to be fixed.  It doesn't look
harmless, either.
- The definitions of EVTG_FIRED_* are still present in
pg_event_trigger.h, even though they're longer used anywhere.
- The comments in parsenodes.h still refer to command triggers rather
than event triggers.  event_trigger.h contains a reference to
CommandTriggerData that should say EventTriggerData.  plperl.sgml has
vestiges of the old terminology as well.

In terms of the schema itself, I think we are almost there, but:

- I noticed while playing around with this that pg_dump emits a
completely empty owner field when dumping an event trigger.  At first
I thought that was just an oversight, but then I realized there's a
deeper reason for it: pg_event_trigger doesn't have an owner field.  I
think it should.  The only other objects in the system that don't have
owners are text search parsers and text search templates (and casts,
sort of).  It might seem redundant to have an owner even when
event-triggers are superuser-only, but we might want to try to relax
that restriction later.  Note that foreign data wrappers, which are
also superuser-create-only, do have an owner.  (Note that if we give
event triggers an owner, then we also need ALTER .. OWNER TO support
for them.)

- It seems pretty clear at this point that the cache you've
implemented in src/backend/utils/cache/evtcache.c is going to do all
the heavy lifting of converting the stored catalog representation to a
form that is suitable for quick access.  Given that, I wonder whether
we should go whole hog and change evtevent to a text field.  This
would simplify things for pg_dump and we could get rid of
pg_evtevent_to_string, at a cost of doing marginally more work when we
rebuild the event cache (but who really cares, given that you're
reading the entire table every time you rebuild the cache anyway?).

Thoughts?

Some other minor comments:

- In the \dy output, command_start is referred to as the condition,
but the documentation calls it an event and the grammar calls it
event_name.  event or event_name seems better, so I think this
is just a matter of changing psql to match.
- AlterEventTrigStmt defines tgenbled as char *; I think it should
just be char.  In gram.y, you can declare the enable_trigger
production as chr (or ival?) rather than str.  At any rate
pstrdup(stmt-tgenabled)[0] doesn't look right; you don't need to copy
a string to fetch the first byte.
- Why did you create a separate file pg_event_trigger_fn.h instead of
just including that single prototype in pg_event_trigger.h?
- The EVENTTRIGGEROID syscache is used only in RemoveEventTriggerById.
 I don't think that's a sufficient justification for eating up more
memory for another system cache.  I think you should just remove that
cache and recode RemoveEventTriggerById to find the correct tuple via
an index scan.  See RemoveEventTriggerById for an example.

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

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


[HACKERS] PG9.2 and FDW query planning.

2012-07-05 Thread Ronan Dunklau
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello.

I'm in the process of porting our multicorn extension to pg9.2, and
I'd like to take advantage of the GetForeignPaths hook.

The multicorn extension allows the creation of a FDW in python using a
simple API, and I'd like to be able to provide FDW implementors a way
to influence the planner.

Let's say I have an IMAP foreign data wrapper, and I write a query
joining the table on itself using the In-Reply-To and Message-ID
headers, is there anything I can do to avoid fetching all the mails
from the remote server ?

If I could somehow inform the planner that it can look up rows by
message-id, thus avoiding the need to fetch everything from the remote
server. Perhaps persuading the planner to use a nested-loop ?

If I understand correctly, I should build one (or more) list of
pathkeys, and add more foreign paths using those pathkeys.

There was a discussion about index on foreign tables back in march.
- From what I understand from this discussion, someone proposed to
locally store information about indexes on the foreign tables, but I
did not find anything on how to build a path from scratch.

Thank you.

- -- 
Ronan Dunklau

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)

iQEcBAEBAgAGBQJP9bsIAAoJECTYLCgFy323KtsH/2/8AVfBRm75oWFMlU0l0oBC
ujxkt338PnpVi1V5gtE5GSRSwybPWytXAkgzIQ5/DEP/RmeW8pliV+0V7zvqlEWG
zgvfA7stRBWtIIIv6mdlTM0eBBgsFnoJLiJDTUDst5vAaj8vg8b+pX/ip7nSF5sw
dV2i3ir6JSsG4nJOcQ/kP6xg4Joan65pOwFDfwnx9pFnerT0YN9f87DRuohcj12e
fgWSqZkGU5nx9yCLWa294YzIFFY7lIjLowzEfg2eP2dVIM09GquKsSXyilJiMy4J
3QL64mUDF/pNZeH0LnpyGJfCfPlQsX4c554rZbO03tVeSEZMyVpCISLqutTnR9I=
=HwMc
-END PGP SIGNATURE-

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Joel Jacobson
On Thu, Jul 5, 2012 at 6:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Well, to be perfectly frank, I already doubt that this entire feature
 passes the complexity-versus-value test, because pg_dump is not a
 substitute for an SCM --- people who have got enough functions to need
 this sort of thing need to be keeping them somewhere else than in dump
 files.  Complicating things more by supporting multiple ways of doing it
 will make that worse.  I think you need to pick one design and stick
 with it, not try to paint the bikeshed every color suggested by anybody.


I agree it should be one option only, and again I think the one file
variant is best.

This is indeed not a substitute for an SCM, but a nice complement.

Personally, I use this feature already to commit the schema for all versions
of my databases (production, test, development) into a git repo every
minute.
It only commits if something has changed.

This makes it super easy to compare the schema of the actual production
database
between different points in time.

This would not be possible if only manually committing stuff to the normal
git repo,
where I also have all the functions, which I modify when developing and
testing.

pg_dump - git means you can be 100% certain version X of the schema was
active in the production database at date/time T.


Re: [HACKERS] PG9.2 and FDW query planning.

2012-07-05 Thread Tom Lane
Ronan Dunklau rdunk...@gmail.com writes:
 Let's say I have an IMAP foreign data wrapper, and I write a query
 joining the table on itself using the In-Reply-To and Message-ID
 headers, is there anything I can do to avoid fetching all the mails
 from the remote server ?

 If I could somehow inform the planner that it can look up rows by
 message-id, thus avoiding the need to fetch everything from the remote
 server. Perhaps persuading the planner to use a nested-loop ?

OK, so what you're saying is that the imap server can effectively
provide an index on message_id.  What you'd do is create a parameterized
path that uses the tbl.message_id = other_tbl.in_reply_to join clause.
If that's enough cheaper than a full scan, the planner would select it.

FWIW, I'm not sure that it's sane to try to expose this stuff to python
yet.  It's complicated and still something of a moving target.  Once
we've got a few more C-coded FDWs that can do this type of optimization,
things might be stable enough that it'd be useful to try to provide a
high-level API.

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] Schema version management

2012-07-05 Thread Alvaro Herrera

Excerpts from Christopher Browne's message of jue jul 05 12:10:09 -0400 2012:

 I wound up expanding the function arguments and using function + args
 as the name.  That leads to a risk of rather long names for functions,
 but there aren't many other ways possible.

Well, maybe not many, but you don't need many, only some.  You could
stringify the list of arguments and use a hash of the string.  That's
also unambiguous and the length is constrained, regardless of the number
of args.

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

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


Re: [HACKERS] enhanced error fields

2012-07-05 Thread Pavel Stehule
Hello

there is a updated patch:

* renamed auxiliary functions and moved it elog.c - header is new file
relerror.h
* new fields constraint_table and trigger_table - constraints and
triggers are related to relation in pg, not just to schema
* removed using implicit constraints without unique name
* better coverage of enhancing errors in source code
* removed inline keywords



 /* Class 23 - Integrity Constraint Violation */

 This would be a reasonably well-defined place to require that this new
 infrastructure be used going forward (emphasis on the well-defined).
 Note that the authors of third-party database drivers define exception
 classes whose structure reflects these errcodes.h codes. To be
 inconsistent here seems unacceptable, since some future client of,
 say, pqxx (the example that I am personally most familiar with) might
 reasonably hope to always see some relation name when they call the
 e.relation_name() of some pqxx::integrity_constraint_violation object.
 If we were to commit the patch as-is, that would not be possible,
 because the following such sites that have not been touched:




 src/backend/executor/execQual.c
 3786:   
 (errcode(ERRCODE_NOT_NULL_VIOLATION),

 src/backend/utils/adt/domains.c
 126:
 (errcode(ERRCODE_NOT_NULL_VIOLATION),

 src/backend/executor/execQual.c
 3815:   
 (errcode(ERRCODE_CHECK_VIOLATION),

 src/backend/utils/adt/domains.c
 162:
 (errcode(ERRCODE_CHECK_VIOLATION),


these exceptions are related to domains - we has not adequate fields
now - and these fields are not in standards

it needs some like DOMAIN_NAME and DOMAIN_SCHEMA ???

Regards

Pavel


eelog-2012-07-05.patch
Description: Binary data

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


Re: [HACKERS] Schema version management

2012-07-05 Thread David E. Wheeler
On Jul 5, 2012, at 3:21 PM, Andrew Dunstan wrote:

 No they are not necessarily one logical unit. You could have a bunch of
 functions called, say, equal which have pretty much nothing to do with
 each other, since they refer to different types.
 
 +1 from me for putting one function definition per file.

+1 for an option (I prefer one file for my projects, but might need multiple 
files for other projects).

David


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


Re: [HACKERS] waitpid in pg_basebackup

2012-07-05 Thread Tom Lane
I wrote:
 I agree, let's drop the support for waitpid() not being present.

BTW, some digging in the commit logs shows that postmaster.c's
existing support for using wait3 in place of waitpid was added in
commit a5494a2d92a2752c610b8b668a7d33478e90c160, Various patches for
nextstep by GregorHoffleit.  NextStep is presumably quite dead by
now, and anyway we officially pulled support for it as of 9.2.

I will go ahead and remove that 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] foreign key locks

2012-07-05 Thread Alvaro Herrera

Excerpts from Alvaro Herrera's message of vie jun 29 19:17:02 -0400 2012:

 I was only testing migrating from an old version into patched, not
 same-version upgrades.
 
 I think I see what's happening here.

Okay, I have pushed the fix to github -- as I suspected, code-wise the
fix was simple.  I will post an updated consolidated patch later today.

make check of pg_upgrade now passes for me.

It would be nice that make installcheck would notify me that some
modules' tests are being skipped ...

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

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


Re: [HACKERS] User-Id Tracking when Portal was started

2012-07-05 Thread Robert Haas
On Wed, Jul 4, 2012 at 4:24 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/7/4 Robert Haas robertmh...@gmail.com:
 On Tue, Jul 3, 2012 at 12:46 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 My point is that it seems like a bug that the secContext gets restored
 in one case and not the other, depending on which user ID was specified
 in SET SESSION AUTHORIZATION.

 Sorry, the above description mention about a case when it does not use
 the marker to distinguish a case to switch user-id from a case not to 
 switch.
 (I though I was asked the behavior if this logic always switches /
 restores ids.)

 The patch itself works correctly, no regression test failed even though
 several tests switches user-id using SET SESSION AUTHORIZATION.

 I don't believe that proves anything.  There are lots of things that
 aren't tested by the regression tests, and there's no guarantee that
 any you've added cover all bases, either.  We always treat user-ID and
 security context as a unit; you haven't given any reason why this case
 should be handled differently, and I bet it shouldn't.

 This patch always handles user-id and security context as a unit.
 In case when it was switched, then it shall be always restored.
 And, in case when it was not switched, then it shall never be restored.

 Was my explanation confusing?

It's not that your explanation is confusing; it's that it doesn't
match the code.

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

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Aidan Van Dyk
On Thu, Jul 5, 2012 at 12:57 PM, David E. Wheeler da...@justatheory.com wrote:
 On Jul 5, 2012, at 3:21 PM, Andrew Dunstan wrote:

 No they are not necessarily one logical unit. You could have a bunch of
 functions called, say, equal which have pretty much nothing to do with
 each other, since they refer to different types.

 +1 from me for putting one function definition per file.

 +1 for an option (I prefer one file for my projects, but might need multiple 
 files for other projects).

-1

I'd rather have the few overloaded-functions in one file (hopefully
with deterministic ordering) and a sane, simple filename, than have
every function in every database in a separate file with some strange
mess in the filename that makes me cringe every time I see it.

a.

-- 
Aidan Van Dyk Create like a god,
ai...@highrise.ca   command like a king,
http://www.highrise.ca/   work like a slave.

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


Re: [HACKERS] waitpid in pg_basebackup

2012-07-05 Thread Fujii Masao
On Fri, Jul 6, 2012 at 2:01 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 I agree, let's drop the support for waitpid() not being present.

 BTW, some digging in the commit logs shows that postmaster.c's
 existing support for using wait3 in place of waitpid was added in
 commit a5494a2d92a2752c610b8b668a7d33478e90c160, Various patches for
 nextstep by GregorHoffleit.  NextStep is presumably quite dead by
 now, and anyway we officially pulled support for it as of 9.2.

 I will go ahead and remove that code.

Thanks!

BTW, I was just implementing the patch ;) Patch attached.
Note that I've not tested the patch on Windows environment
because I don't have that

Regards,

-- 
Fujii Masao


drop_ifdef_waitpid_v1.patch
Description: Binary data

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


Re: [HACKERS] waitpid in pg_basebackup

2012-07-05 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 On Fri, Jul 6, 2012 at 2:01 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I will go ahead and remove that code.

 Thanks!

 BTW, I was just implementing the patch ;) Patch attached.

Oh, I'd already done it when I got your message :-(.  Looks like we
arrived at the same answers, though, except for slightly different
choices for how to do the Windows bit.

 Note that I've not tested the patch on Windows environment
 because I don't have that

Me either; I'll check the buildfarm results 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] Schema version management

2012-07-05 Thread Josh Berkus

 I'd rather have the few overloaded-functions in one file (hopefully
 with deterministic ordering) and a sane, simple filename, than have
 every function in every database in a separate file with some strange
 mess in the filename that makes me cringe every time I see it.

Having tried it both ways, for an existing production project, I agree
with Adrian.  One file, multiple functions.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com



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


Re: [HACKERS] [PATCH] pg_dump: Sort overloaded functions in deterministic order

2012-07-05 Thread Tom Lane
Joel Jacobson j...@trustly.com writes:
 New version, made a typo in last one.

I'm not particularly happy with the idea of adding a sortkey field to
DumpableObject as such, when most object types don't need it.  That just
bloats the code and pg_dump's memory consumption.  It would be better to
modify the already-existing object-type-specific special cases in
DOTypeNameCompare to take additional information into account as needed.

BTW, I see no reason to be adding extra calls of
pg_get_function_identity_arguments.  What is wrong with the funcsig or
aggsig strings that the code already computes?

regards, tom lane

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


[HACKERS] Re: proposal and patch : support INSERT INTO...RETURNING with partitioned table using rule

2012-07-05 Thread John Lumby

First,  apologies for taking so long to reply to your post.

On Fri, 22 Jun 2012 09:55:13, Robert Haas wrote:

 On Wed, Jun 20, 2012 at 12:24 PM, John Lumby johnlumby(at)hotmail(dot)com 
 wrote:
  An INSERT which has a RETURNING clause and which is to be rewritten 
 based on
  a rule will be accepted if the rule is an unconditional DO INSTEAD.
  In general I believe unconditional means no WHERE clause, but in 
 practice
  if the rule is of the form
     CREATE RULE insert_part_history as ON INSERT to history \
   DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
  this is treated as conditional and the query is rejected.
 
 This isn't rejected because the query is treated as condition; it's
 rejected because it's not valid syntax.  A SELECT query can't have a
 RETURNING clause, because the target list (i.e. the part that
 immediately follows the SELECT) already serves that purpose. The fact
 that it's in a CREATE RULE statement is irrelevant.

Thanks for correcting me.   At the time,  it wasn't clear to me whether the 
RETURNING clause
in a CREATE RULE statement belonged to the CREATE RULE statement or the rule 
being created,
but now I see it's the latter.

    .  I propose to extend the rule system to recognize cases where the 
 INSERT query specifies
   RETURNING and the rule promises to return a row,  and to then permit 
 this query to run
   and return the expected row.   In effect,  to widen the definition of 
 unconditional
   to handle cases such as my testcase.
 
 That already (kind of) works:
 
  [...]
 
 I do notice that the RETURNING clause of the INSERT can't reference
 NEW, which seems like a restriction that we probably ought to lift,
 but it doesn't seem to have much to do with your patch.
 

The main use of my proposal is to be able to return the value of the sequence 
assigned
to the NEW.id column,  so yes   that is a serious restriction.

However,   even if that restriction is lifted,  it will not help with the case 
where
the rule is an invocation of a function,   which is the case I need.    For 
example,
in my testcase,  the function is building the SQL statement to be executed 
including the
 table name of the partitioned child table,   based on the timestamp in the NEW 
record.

Your comments have helped me realize that my original subject line did not 
accurately state
my requirement,  which should read
 support INSERT INTO...RETURNING with partitioned table using rule which 
invokes a function

And for this requirement as re-stated,  as far as I can tell,   current 
postgresql has no solution:
  .  only way to invoke a function in a rewrite rule is with SELECT function()
  .  SELECT does not permit RETURNING clause
  .  my proposal provides a way of relaxing this restriction for the case of 
SELECT in a rewrite rule.

I hope this describes the proposal a bit better.

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] pg_dump: Sort overloaded functions in deterministic order

2012-07-05 Thread Joel Jacobson
I agree, good suggestion, I just didn't know how to implement it without a
new field. I'll make a new attempt to get it right.

On Thursday, July 5, 2012, Tom Lane wrote:

 Joel Jacobson j...@trustly.com javascript:; writes:
  New version, made a typo in last one.

 I'm not particularly happy with the idea of adding a sortkey field to
 DumpableObject as such, when most object types don't need it.  That just
 bloats the code and pg_dump's memory consumption.  It would be better to
 modify the already-existing object-type-specific special cases in
 DOTypeNameCompare to take additional information into account as needed.

 BTW, I see no reason to be adding extra calls of
 pg_get_function_identity_arguments.  What is wrong with the funcsig or
 aggsig strings that the code already computes?

 regards, tom lane



Re: [HACKERS] plpython issue with Win64 (PG 9.2)

2012-07-05 Thread Heikki Linnakangas

On 04.07.2012 15:11, Jan Urbański wrote:

On 04/07/12 13:58, Asif Naeem wrote:

I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.


create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');



create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
--+
tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.


Great, this looks correct.

Can we apply this to 9.2?


Committed. This bug was present in versions = 9.0, so backpatched.

I used ereport() rather than elog() in the error message. Correct me if 
that was wrong, but the point was to avoid PLy_elog(), because that 
might cause recursion, and ereport() should be ok. I believe the message 
should be translated, as it's quite possible to get that error, at least 
if you use SQL_ASCII, so ereport() is more approriate than elog().


Thanks!

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] plpython issue with Win64 (PG 9.2)

2012-07-05 Thread Jan Urbański

On 05/07/12 21:37, Heikki Linnakangas wrote:

Committed. This bug was present in versions = 9.0, so backpatched.


Thanks!


I used ereport() rather than elog() in the error message. Correct me if
that was wrong, but the point was to avoid PLy_elog(), because that
might cause recursion, and ereport() should be ok. I believe the message
should be translated, as it's quite possible to get that error, at least
if you use SQL_ASCII, so ereport() is more approriate than elog().


Yes, you're absolutely right.

Cheers,
Jan

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


[HACKERS] obsolete copyright notice

2012-07-05 Thread Antonin Houska

I found out by chance that \copyright command still contains 2011.
Perhaps documentation of new year's day changes needs to be updated, 
if such exists.


Tony H.

--
Sent 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] pg_dump: Sort overloaded functions in deterministic order

2012-07-05 Thread Tom Lane
Joel Jacobson j...@trustly.com writes:
 I agree, good suggestion, I just didn't know how to implement it without a
 new field. I'll make a new attempt to get it right.

You may in fact need a new field --- I'm just saying it should be in the
object-type-specific struct, eg FuncInfo, not DumpableObject.

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] pg_dump: Sort overloaded functions in deterministic order

2012-07-05 Thread Joel Jacobson
Roger that. I'm on it.

On Thursday, July 5, 2012, Tom Lane wrote:

 Joel Jacobson j...@trustly.com javascript:; writes:
 You may in fact need a new field --- I'm just saying it should be in the
 object-type-specific struct, eg FuncInfo, not DumpableObject.

 regards, tom lane



[HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

2012-07-05 Thread Heikki Linnakangas

On 05.07.2012 23:31, Tom Lane wrote:

Heikki Linnakangasheikki.linnakan...@iki.fi  writes:

Fix mapping of PostgreSQL encodings to Python encodings.


The buildfarm doesn't like this --- did you check for side effects on
regression test results?


Hmm, I ran the regressions tests, but not with C encoding. With the 
patch, you no longer get the errdetail you used to, when an encoding 
conversion fails:



***
*** 41,47 

  SELECT unicode_plan1();
  ERROR:  spiexceptions.InternalError: could not convert Python Unicode object 
to PostgreSQL server encoding
- DETAIL:  UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in 
position 0: ordinal not in range(128)
  CONTEXT:  Traceback (most recent call last):
PL/Python function unicode_plan1, line 3, in module
  rv = plpy.execute(plan, [u\x80], 1)
--- 39,44 


We could just update the expected output, there's two expected outputs 
for this test case and one of them is now wrong. But it'd actually be 
quite a shame to lose that extra information, that's quite valuable. 
Perhaps we should go back to using PLu_elog() here, and find some other 
way to avoid the recursion.


- Heikki

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


[HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

2012-07-05 Thread Jan Urbański

On 05/07/12 22:37, Heikki Linnakangas wrote:

On 05.07.2012 23:31, Tom Lane wrote:

Heikki Linnakangasheikki.linnakan...@iki.fi writes:

Fix mapping of PostgreSQL encodings to Python encodings.


The buildfarm doesn't like this --- did you check for side effects on
regression test results?


Hmm, I ran the regressions tests, but not with C encoding. With the
patch, you no longer get the errdetail you used to, when an encoding
conversion fails:


***
*** 41,47 

SELECT unicode_plan1();
ERROR: spiexceptions.InternalError: could not convert Python Unicode
object to PostgreSQL server encoding
- DETAIL: UnicodeEncodeError: 'ascii' codec can't encode character
u'\x80' in position 0: ordinal not in range(128)
CONTEXT: Traceback (most recent call last):
PL/Python function unicode_plan1, line 3, in module
rv = plpy.execute(plan, [u\x80], 1)
--- 39,44 


We could just update the expected output, there's two expected outputs
for this test case and one of them is now wrong. But it'd actually be
quite a shame to lose that extra information, that's quite valuable.
Perhaps we should go back to using PLu_elog() here, and find some other
way to avoid the recursion.


Seems that the problem is that the LC_ALL=C makes Postgres use SQL_ASCII 
as the database encoding and as the comment states, translating PG's 
SQL_ASCII to Python's ascii is not ideal.


The problem is that PLyUnicode_Bytes is (via an ifdef) used as 
PyString_ToString on Python3, which means that there are numerous call 
sites and new ones might appear in any moment. I'm not that keen on 
invoking the traceback machinery on low-level encoding errors.


Hm, since PyUnicode_Bytes should get a unicode object and return bytes 
in the server encoding, we might just say that for SQL_ASCII we 
arbitrarily choose UTF-8 to encode the unicode codepoints, so we'd just 
set serverenc = utf-8 in the first switch case.


That doesn't solve the problem of the missing error detail, though.

Jan

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

2012-07-05 Thread Peter Eisentraut
On tor, 2012-07-05 at 22:53 +0200, Jan Urbański wrote:
 The problem is that PLyUnicode_Bytes is (via an ifdef) used as 
 PyString_ToString on Python3, which means that there are numerous call
 sites and new ones might appear in any moment. I'm not that keen on 
 invoking the traceback machinery on low-level encoding errors.

Why not?


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


Re: [HACKERS] Schema version management

2012-07-05 Thread Dimitri Fontaine
Alvaro Herrera alvhe...@commandprompt.com writes:
 However I am also against what seems to be the flow.  Normally, you
 don't write overloaded plpgsql functions such as equal.  Case in
 point, the equality functions in core have funny names like int4eq and
 so on.  Instead, at least in my experience, the overloaded functions
 people seem to have in their databases are like do_stuff_to_foobars()
 and you have one version for foos and another one for bars.

+1

I too want to have my overloaded functions all in the same file, as much
as to have made that the only behavior in getddl.py:

  https://github.com/dimitri/getddl

 If you're doing lots of equality functions, surely it would make more
 sense to package them up as an extension anyway along with all the other
 thingies you need for the type you're supposedly writing.  So it's a
 completely different market than what we're aiming at here.

+1

Tom Lane t...@sss.pgh.pa.us writes:
 FWIW, I'm attracted to the all-similarly-named-functions-together
 method, mainly because it dodges the problem of how to encode a
 function's argument list into a filename.  However, we're being
 short-sighted to only think of functions here.  What about operators?
 Or casts?  Those don't have simple names either.

I would argue like Álvaro that when dealing with operators and casts
you're probably writing an extension already, and we're providing
another way to deal with that.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

2012-07-05 Thread Jan Urbański

On 05/07/12 23:30, Peter Eisentraut wrote:

On tor, 2012-07-05 at 22:53 +0200, Jan Urbański wrote:

The problem is that PLyUnicode_Bytes is (via an ifdef) used as
PyString_ToString on Python3, which means that there are numerous call
sites and new ones might appear in any moment. I'm not that keen on
invoking the traceback machinery on low-level encoding errors.


Why not?


Because it can lead to recursion errors, like the one this patch was 
supposed to fix. The traceback machinery calls into the encoding 
functions, because it converts Python strings (like function names) into 
C strings.


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


Re: [HACKERS] Schema version management

2012-07-05 Thread Christopher Browne
On Thu, Jul 5, 2012 at 5:52 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 However I am also against what seems to be the flow.  Normally, you
 don't write overloaded plpgsql functions such as equal.  Case in
 point, the equality functions in core have funny names like int4eq and
 so on.  Instead, at least in my experience, the overloaded functions
 people seem to have in their databases are like do_stuff_to_foobars()
 and you have one version for foos and another one for bars.

 +1

 I too want to have my overloaded functions all in the same file, as much
 as to have made that the only behavior in getddl.py:

That seems pretty appropriate to me.

The converse makes my head hurt...

If I have a bunch of overloaded functions, whose definitions *aren't*
really related, are we competing for the obfuscated PostgreSQL
contest?

In practice, that sounds like something I'd want to add to my list of
fire people that do this! Bad Practices.

 If you're doing lots of equality functions, surely it would make more
 sense to package them up as an extension anyway along with all the other
 thingies you need for the type you're supposedly writing.  So it's a
 completely different market than what we're aiming at here.

 +1
+1

 Tom Lane t...@sss.pgh.pa.us writes:
 FWIW, I'm attracted to the all-similarly-named-functions-together
 method, mainly because it dodges the problem of how to encode a
 function's argument list into a filename.  However, we're being
 short-sighted to only think of functions here.  What about operators?
 Or casts?  Those don't have simple names either.

 I would argue like Álvaro that when dealing with operators and casts
 you're probably writing an extension already, and we're providing
 another way to deal with that.

Indeed.  Is this something we ought to document as a recommendation?
It's not exactly reference material, but if it's a good practice,
perhaps it should be in the manuals somewhere...
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?

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


Re: [HACKERS] Schema version management

2012-07-05 Thread Tom Lane
Christopher Browne cbbro...@gmail.com writes:
 On Thu, Jul 5, 2012 at 5:52 PM, Dimitri Fontaine dimi...@2ndquadrant.fr 
 wrote:
 Tom Lane t...@sss.pgh.pa.us writes:
 FWIW, I'm attracted to the all-similarly-named-functions-together
 method, mainly because it dodges the problem of how to encode a
 function's argument list into a filename.  However, we're being
 short-sighted to only think of functions here.  What about operators?
 Or casts?  Those don't have simple names either.

 I would argue like Álvaro that when dealing with operators and casts
 you're probably writing an extension already, and we're providing
 another way to deal with that.

 Indeed.  Is this something we ought to document as a recommendation?

This argument seems a bit irrelevant to me.  pg_dump doesn't get to pick
and choose what will be in the database it's told to dump.  If we're
going to do something like what Joel wants, we have to have file naming
conventions for operator and cast objects.  So we can't just leave them
out of the conversation (or if we do, we shouldn't be surprised when the
ensuing design sucks).

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: add conversion from pg_wchar to multibyte

2012-07-05 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sun, Jul 1, 2012 at 5:11 AM, Alexander Korotkov aekorot...@gmail.com 
 wrote:
 [ new patch ]

 With the improved comments in pg_wchar.h, it seemed clear what needed
 to be done here, so I fixed up the MULE conversion and committed this.
  I'd appreciate it if someone would check my work, but I think it's
 right.

Hm, several of these routines seem to neglect to advance the from
pointer?

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: add conversion from pg_wchar to multibyte

2012-07-05 Thread Tom Lane
Tatsuo Ishii is...@postgresql.org writes:
 So far as I can see, the only LCPRVn marker code that is actually in
 use right now is 0x9d --- there are no instances of 9a, 9b, or 9c
 that I can find.
 
 I also read in the xemacs internals doc, at
 http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145
 that XEmacs thinks the marker code for private single-byte charsets
 is 0x9e (only) and that for private multi-byte charsets is 0x9f (only);
 moreover they think 0x9a-0x9d are potential future official multibyte
 charset codes.  I don't know how we got to the current state of using
 0x9a-0x9d as private charset markers, but it seems pretty inconsistent
 with XEmacs.

 At the time when mule internal code was introduced to PostgreSQL,
 xemacs did not have multi encoding capabilty and mule (a patch to
 emacs) was the only implementation allowed to use multi encoding. So I
 used the specification of mule documented in the URL I wrote.

I see.  Given that upstream has decided that a simpler definition is
more appropriate, is there any reason not to follow their lead, to the
extent that we can do so without breaking existing on-disk data?

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] obsolete copyright notice

2012-07-05 Thread Bruce Momjian
On Thu, Jul 05, 2012 at 10:10:08PM +0200, Antonin Houska wrote:
 I found out by chance that \copyright command still contains 2011.
 Perhaps documentation of new year's day changes needs to be
 updated, if such exists.

What version of Postgres is this?  We don't update the copyright for
minor releases, so my bet is it is 9.1 because that was released in
2011.

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

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

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


Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte

2012-07-05 Thread Robert Haas
On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sun, Jul 1, 2012 at 5:11 AM, Alexander Korotkov aekorot...@gmail.com 
 wrote:
 [ new patch ]

 With the improved comments in pg_wchar.h, it seemed clear what needed
 to be done here, so I fixed up the MULE conversion and committed this.
  I'd appreciate it if someone would check my work, but I think it's
 right.

 Hm, several of these routines seem to neglect to advance the from
 pointer?

Err... yeah.  That's not a bug I introduced, but I should have caught
it... and it does make me wonder how well this code was tested.

Does the attached look like an appropriate fix?

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


wchar-advance-from.patch
Description: Binary data

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


Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte

2012-07-05 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hm, several of these routines seem to neglect to advance the from
 pointer?

 Err... yeah.  That's not a bug I introduced, but I should have caught
 it... and it does make me wonder how well this code was tested.

 Does the attached look like an appropriate fix?

I'd be inclined to put the from++ and len-- at the bottom of each loop,
and in that order every time, just for consistency and obviousness.
But yeah, that's basically what's needed.

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: add conversion from pg_wchar to multibyte

2012-07-05 Thread Tatsuo Ishii
 Tatsuo Ishii is...@postgresql.org writes:
 So far as I can see, the only LCPRVn marker code that is actually in
 use right now is 0x9d --- there are no instances of 9a, 9b, or 9c
 that I can find.
 
 I also read in the xemacs internals doc, at
 http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145
 that XEmacs thinks the marker code for private single-byte charsets
 is 0x9e (only) and that for private multi-byte charsets is 0x9f (only);
 moreover they think 0x9a-0x9d are potential future official multibyte
 charset codes.  I don't know how we got to the current state of using
 0x9a-0x9d as private charset markers, but it seems pretty inconsistent
 with XEmacs.
 
 At the time when mule internal code was introduced to PostgreSQL,
 xemacs did not have multi encoding capabilty and mule (a patch to
 emacs) was the only implementation allowed to use multi encoding. So I
 used the specification of mule documented in the URL I wrote.
 
 I see.  Given that upstream has decided that a simpler definition is
 more appropriate, is there any reason not to follow their lead, to the
 extent that we can do so without breaking existing on-disk data?

Please let me spend week end to understand the their latest spec.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

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


Re: [HACKERS] autocomplete - SELECT fx

2012-07-05 Thread Josh Kupershmidt
On Mon, Jul 2, 2012 at 1:13 AM, Pavel Stehule pavel.steh...@gmail.com wrote:

 I tested Peter's patch and it works well.

I liked it as well. But I'm not sure what should happen with the patch
now. It seems like it'd be commit-ready with just a tweak or two to
the query as I noted in my last mail, but Tom did seem opposed[1] to
the idea in the first thread, and no one else has spoken up recently
in favor of the idea, so maybe it should just be marked Rejected or
Returned with Feedback?

Josh

[1] http://archives.postgresql.org/message-id/13612.1328887227%40sss.pgh.pa.us

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


Re: [HACKERS] pl/perl and utf-8 in sql_ascii databases

2012-07-05 Thread Alex Hunsaker
On Tue, Jul 3, 2012 at 2:59 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
 Hello, Here is regression test runs on pg's also built with
 cygwin-gcc and VC++.

Thank you!

 The patches attached following,

 - plperl_sql_ascii-4.patch : fix for pl/perl utf8 vs sql_ascii
 - plperl_sql_ascii_regress-1.patch : regression test for this patch.
  I added some tests on encoding to this.

 I will mark this patch as 'ready for committer' after this.

Both patches look good to me..

 I've been stuck in mud trying to plperl work on windows
 environment. I saw many messages complaining that plperl wouldn't
 be built to work. For the convenience of those and myself, I
 describe the process of building postgresql with plperl on
 Windows with cygwin and VC++ I've done below.

Hrm, I don't develop on windows here, but out of curiosity, what were
the messages like?

 - The remainder of the patch whic fixes the easy fixable leakes
   of palloc'ed memory won't be ported into 9.1. This is only for
   9.3dev.

 What should I do for this?

Just let the commiter decide? :-)

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


Re: [HACKERS] Event Triggers reduced, v1

2012-07-05 Thread Robert Haas
On Thu, Jul 5, 2012 at 5:31 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
 [ new patch ]

Attached is a incremental patch with a bunch of minor cleanups,
including reverts of a few spurious white space changes.  Could you
merge this into your version?

I have some concerns about pg_dump:

1. Can we spell out EvtTriggerInfo, getEvtTriggers, and dumpEvtTrigger
as EventTriggerInfo, getEventTriggers, and dumpEventTrigger?

2. I don't think that this code properly handles older server
versions.  First, the version test in getEvtTriggers checks against
90200 instead of 90300.  Second, when running against a too-old server
version, this is going to try to execute an empty query and then parse
the results.  I think you want to restructure this code so that it
just plain old returns if the server is too old.  See for example
getForeignDataWrappers.

3. The way you're generating evtfname is unsafe if either the schema
or the function name contains SQL characters.  I think this should
probably be casting the function OID to regproc in lieu of rolling its
own formatting code.  Also, the tags should probably be escaped using
quote_literal, just to be future-proof.

4. I think we should aim to generate all the SQL in upper case.  Right
now CREATE EVENT TRIGGER and EXECUTE PROCEDURE are in upper case
but when tag in is in lower case.

In psql, I think we should similarly have listEventTriggers() rather
than listEvtTriggers(); here as in pg_dump I think you should cast the
evtfoid to regproc to get schema-qualification and escaping, in lieu
of the explicit join.

 In terms of the schema itself, I think we are almost there, but:

 - I noticed while playing around with this that pg_dump emits a
 completely empty owner field when dumping an event trigger.  At first
 I thought that was just an oversight, but then I realized there's a
 deeper reason for it: pg_event_trigger doesn't have an owner field.  I
 think it should.  The only other objects in the system that don't have
 owners are text search parsers and text search templates (and casts,
 sort of).  It might seem redundant to have an owner even when
 event-triggers are superuser-only, but we might want to try to relax
 that restriction later.  Note that foreign data wrappers, which are
 also superuser-create-only, do have an owner.  (Note that if we give
 event triggers an owner, then we also need ALTER .. OWNER TO support
 for them.)

 Damn, I had it on my TODO and Álvaro hinted me already, and I kept
 forgetting about it nonetheless. Fixed now.

evtowner seems to have missed the documentation bus.

With respect to the documentation, keep in mind that the synopsis is
going to show up in the command line help for \h.  I'm thinking that
the full list of command tags is too long for that, and that we should
instead rearrange the page so that the list of supported commands is
outside the synopsis.  The synposis is also somewhat misleading, I
think, in that variable in (tag, ...) conveys the idea that no
matter what the variable is, the items in parentheses will surely be
tags.  I suggest that we say something like filter_variable in
(filter_value, ...) and then document in the text that
filter_variable must currently always be TAG, and that the legal
values for filter_value are dependent on the choice of
filter_variable, and the legal choices for TAG are those listed in the
following table: splat.

The documentation contains the following claim with which I'm
extremely uncomfortable:

+ para
+  Triggers on literalANY/literal command support more commands than
+  just this list, and will only provide the literalcommand
+  tag/literal argument as literalNOT NULL/literal. Supporting more
+  commands is made so that you can actually block xref linkend=ddl
+  commands in one go.
+ /para

A minor issue is that there's no notion of ANY any more; it's just a
consequence of leaving out the WHEN clause.  The bigger issue is that
I can't see any reason to do it that way.  Surely if we're firing the
trigger at all, we can arrange to have the command tag properly filled
in so that we can filter by it.

This might be a crazy idea, but... it seems like it would be awfully
sweet if we could find a way to avoid having to translate between node
tags (i.e. constants beginning with T_* that are used to identify the
type of statement that is executing) and event tags (i.e. constants
beginning with E_* that are used to identify the type of statement
that is executing).  Now obviously this is not quite possible because
in some cases the choice of E_* constant depends on not only the node
tag but also the type of object being operated on.  However, it seems
like this is a surmountable obstacle: since we no longer need to store
the E_* constants in a system catalog, they don't really need to be
integers.  For example, we could define something like this:

typedef struct
{
NodeTag nodetag;
ObjectType objecttype;
} NodeTagWithObjectType;

...and set the 

Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte

2012-07-05 Thread Robert Haas
On Thu, Jul 5, 2012 at 8:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hm, several of these routines seem to neglect to advance the from
 pointer?

 Err... yeah.  That's not a bug I introduced, but I should have caught
 it... and it does make me wonder how well this code was tested.

 Does the attached look like an appropriate fix?

 I'd be inclined to put the from++ and len-- at the bottom of each loop,
 and in that order every time, just for consistency and obviousness.
 But yeah, that's basically what's needed.

OK, I've committed a slightly tweaked version of that patch.

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

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


Re: [HACKERS] autocomplete - SELECT fx

2012-07-05 Thread Pavel Stehule
2012/7/6 Josh Kupershmidt schmi...@gmail.com:
 On Mon, Jul 2, 2012 at 1:13 AM, Pavel Stehule pavel.steh...@gmail.com wrote:

 I tested Peter's patch and it works well.

 I liked it as well. But I'm not sure what should happen with the patch
 now. It seems like it'd be commit-ready with just a tweak or two to
 the query as I noted in my last mail, but Tom did seem opposed[1] to
 the idea in the first thread, and no one else has spoken up recently
 in favor of the idea, so maybe it should just be marked Rejected or
 Returned with Feedback?

Hard to say - it is usable, but on second hand - some user can be
surprised, because it works only for first item in list.

Regards

Pavel


 Josh

 [1] http://archives.postgresql.org/message-id/13612.1328887227%40sss.pgh.pa.us

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


Re: [HACKERS] enhanced error fields

2012-07-05 Thread Pavel Stehule
Hello

* fixed typo
* support two new fields: constraint_table and trigger_table
* routine_table, routine_schema, trigger_name, trigger_table,
trigger_schema has value when exception coming from plpgsql.

Regards

Pavel


2012/7/2 Peter Geoghegan pe...@2ndquadrant.com:
 On 2 July 2012 15:19, Peter Geoghegan pe...@2ndquadrant.com wrote:
 On 9 May 2012 14:33, Pavel Stehule pavel.steh...@gmail.com wrote:
 here is patch with enhancing ErrorData structure. Now constraints
 errors and RI uses these fields

 So I took a look at the patch eelog-2012-05-09.diff today. All of the
 following remarks apply to it alone.

 I decided to follow through and take a look at
 eelog-plpgsql-2012-05-09.diff today too, while I have all of this
 swapped into my head.

 This patch is not an atomic unit - it builds upon the first patch. I
 successfully merged the local feature branch that I'd created for
 eelog-2012-05-09.diff without any merge conflicts, and I can build
 Postgres and get the regression tests to pass (including a couple of
 new ones, for this added functionality for plggsql - the functionality
 is testing exclusively using the new (9.2) get stacked diagnostics
 and raise custom exception 'some_custom_exception' using
 feature).

 Since that feature branch had all my revisions committed, my
 observations about redundancies in the other base patch still stand -
 the 2 functions mentioned did not exist for the benefit of this
 further patch either.

 There is a typo here:

 +   case PLPGSQL_RAISEOPTION_TRIGGER_SCHEMA:
 +   printf(TRIGGER_SCHENA = );
 +   break;
 }


 I'm not sure about this inconsistency within unreserved_keyword:

 For routines:
 +   | K_DIAG_ROUTINE_NAME
 +   | K_DIAG_ROUTINE_SCHEMA
 

 For triggers:
 +   | K_DIAG_TRIGGER_NAME
 +   | K_DIAG_TRIGGER_SCHEMA
 

 For tables:
 +   | K_DIAG_SCHEMA_NAME
 .
 . **SNIP**
 .
 +   | K_DIAG_TABLE_NAME

 The same inconsistency exists within the anonymous enum that contains
 PLPGSQL_GETDIAG_TABLE_NAME (and other constants), as well as the new
 token keywords within plpgsql's gram.y .

 The doc changes need a little work here too.

 I'm not sure that I agree with the extensive use of the term routine
 in all of these constants - sure, information_schema has a view called
 routines. But wouldn't it be more appropriate to use a
 Postgres-centric term within our own code?

 So, what about the concern about performance taking a hit when plpgsql
 exception blocks are entered as a result of this patch? Well, while I
 think that  an effort to reduce the overhead of PL exception handling
 would be worthwhile, these patches do not appear to alter things
 appreciable (though the overhead *is* measurable):

 [peter@peterlaptop eelog]$ ls
 exceptions.sql  test_eelog_outer.sql

 Patch (eelog-plpgsql):

 [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
 transaction type: Custom query
 scaling factor: 1
 query mode: simple
 number of clients: 10
 number of threads: 1
 duration: 300 s
 number of transactions actually processed: 305756
 tps = 1019.026055 (including connections establishing)
 tps = 1019.090135 (excluding connections establishing)

 Master:

 [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
 transaction type: Custom query
 scaling factor: 1
 query mode: simple
 number of clients: 10
 number of threads: 1
 duration: 300 s
 number of transactions actually processed: 308376
 tps = 1027.908182 (including connections establishing)
 tps = 1027.977879 (excluding connections establishing)

 An archive with simple scripts for repeating this are attached, if
 anyone is interested.

 --
 Peter Geoghegan   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training and Services


eelog-plpgsql-2012-07-06.patch
Description: Binary data

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