[HACKERS] SPI API and exceptions

2012-12-28 Thread Peter Eisentraut
SPI was invented before there was proper exception handling, so it
communicates errors by return values.  This makes programming with SPI
either prone to errors of omission, or very ugly (ultimately, the
standard reasons why exceptions were invented).

So I was pondering whether we could introduce exceptions to SPI in some
way.  I'm not sure how.  We could invent an entirely separate set of
functions, or do some tricks in the header that things before
differently depending on some #define.  Any ideas?




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


Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Peter Eisentraut
On Mon, 2012-12-17 at 16:34 -0500, Peter Eisentraut wrote:
 Yes, this would be a good solution for some applications, but the only
 way I can think of to manage the compatibility issue is to invent some
 function attribute system like
 
 CREATE FUNCTION ... OPTIONS (call_convention 'xyz')

An alternative that has some amount of precedent in the Python world
would be to use comment pragmas, like this:

CREATE FUNCTION foo(a,b,c) AS $$
# plpython: module
import x
 from __future__ import nex_cool_feature

 def helper_function(x):
...

 def __pg_main__(a,b,c):
 defined function body here

$$;

The source code parser would look for this string on, say, the first two
lines, and then decide which way to process the source text.

This way we could get this done fairly easily without any new
infrastructure outside the language handler.



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


Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Pavel Stehule
2012/12/28 Peter Eisentraut pete...@gmx.net:
 On Mon, 2012-12-17 at 16:34 -0500, Peter Eisentraut wrote:
 Yes, this would be a good solution for some applications, but the only
 way I can think of to manage the compatibility issue is to invent some
 function attribute system like

 CREATE FUNCTION ... OPTIONS (call_convention 'xyz')

 An alternative that has some amount of precedent in the Python world
 would be to use comment pragmas, like this:

 CREATE FUNCTION foo(a,b,c) AS $$
 # plpython: module
 import x
  from __future__ import nex_cool_feature

  def helper_function(x):
 ...

  def __pg_main__(a,b,c):
  defined function body here

 $$;

 The source code parser would look for this string on, say, the first two
 lines, and then decide which way to process the source text.

 This way we could get this done fairly easily without any new
 infrastructure outside the language handler.

this concept looks like more stronger and cleaner

+1

I thing so same idea is used in PL/v8

Regards

Pavel




 --
 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] Proposal: Store timestamptz of database creation on pg_database

2012-12-28 Thread Dimitri Fontaine
Stephen Frost sfr...@snowman.net writes:
 Apparently I've managed to miss the tricky case..?

That shouldn't be tricky as a user, but has been a tricky subject every
time we've been talking about implement Event Triggers in the past two
years, so I though I would include it:

create schema test
   create table foo(id serial primary key, f1 text);

create event trigger track_table
  on ddl_command_trace
 when tag in ('create table', 'alter table', 'drop table')
  and context in ('toplevel', 'generated', 'subcommand')
   execute procedure public.track_table_activity();

The trick is that you then want to fire the event trigger for a command
in a 'subcommand' context, as seen in the logs provided by the snitch
example:

NOTICE:  snitch event: ddl_command_end, context: SUBCOMMAND
NOTICE:   tag: CREATE TABLE, operation: CREATE, type: TABLE
NOTICE:   oid: 25139, schema: test, name: foo

 Sure, dropping tables, schemas, etc, would have an impact on the values.

we don't have, as of yet, support for a 'cascade' context. We will need
some heavy refactoring to get there, basically forcing the cascade drops
to happen via ProcessUtility(), but having a single DropStmt to handle
that I guess it shouldn't be very hard to do.

 being told oh, well, you *could* have been collecting it all along if
 you knew about event triggers isn't a particularly satisfying answer.

True that.

Now, having at least a way to do that without resorting to hacking the
backend or writing a C coded extension sure feels nice enough an answer
to me here.

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] Performance Improvement by reducing WAL for Update Operation

2012-12-28 Thread Simon Riggs
On 28 December 2012 08:07, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:

 Hello, I saw this patch and confirmed that

  - Coding style looks good.
  - Appliable onto HEAD.
  - Some mis-codings are fixed.

I've had a quick review of the patch to see how close we're getting.
The perf tests look to me like we're getting what we wanted from this
and I'm happy with the recovery performance trade-offs. Well done to
both author and testers.

My comments

* There is a fixed 75% heuristic in the patch. Can we document where
that came from? Can we have a parameter that sets that please? This
can be used to have further tests to confirm the useful setting of
this. I expect it to be removed before we release, but it will help
during beta.

* The compression algorithm depends completely upon new row length
savings. If the new row is short, it would seem easier to just skip
the checks and include it anyway. We can say if old and new vary in
length by  50% of each other, just include new as-is, since the rows
very clearly differ in a big way. Also, if tuple is same length as
before, can we compare the whole tuple at once to save doing
per-column checks?

* If full page writes is on and the page is very old, we are just
going to copy the whole block. So why not check for that rather than
do all these push ups and then just copy the page anyway?

* TOAST is not handled at all. No comments about it, nothing. Does
that mean it hasn't been considered? Or did we decide not to care in
this release? Presumably that means we are comparing toast pointers
byte by byte to see if they are the same?

* I'd like to see a specific test in regression that is designed to
exercise the code here. That way we will be certain that the code is
getting regularly tested.

* The internal docs are completely absent. We need at least a whole
page of descriptive comment, discussing trade-offs and design
decisions. This is very important because it will help locate bugs
much faster if these things are clealry documented. It also helps
reviewers. This is a big timewaster for committers because you have to
read the whole patch and understand it before you can attempt to form
opinions. Commits happen quicker and easier with good comments.

* Lots of typos in comments. Many comments say nothing more than the
words already used in the function name itself

* flags variables are almost always int or uint in PG source.

* PGLZ_HISTORY_SIZE needs to be documented in the place it is defined,
not the place its used. The test if (oldtuplen  PGLZ_HISTORY_SIZE)
really needs to be a test inside the compression module to maintain
better modularity, so the value itself needn't be exported

* Need to mention the WAL format change, or include the change within
the patch so we can see

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


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


Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation

2012-12-28 Thread Amit Kapila
On Friday, December 28, 2012 3:52 PM Simon Riggs wrote:
 On 28 December 2012 08:07, Kyotaro HORIGUCHI
 horiguchi.kyot...@lab.ntt.co.jp wrote:
 
  Hello, I saw this patch and confirmed that
 
   - Coding style looks good.
   - Appliable onto HEAD.
   - Some mis-codings are fixed.
 
 I've had a quick review of the patch to see how close we're getting.
 The perf tests look to me like we're getting what we wanted from this
 and I'm happy with the recovery performance trade-offs. Well done to
 both author and testers.
 
 My comments
 
 * There is a fixed 75% heuristic in the patch. Can we document where
 that came from? 

It is from LZ compression strategy. Refer PGLZ_Strategy.
I will add comment for it.

 Can we have a parameter that sets that please? This
 can be used to have further tests to confirm the useful setting of
 this. I expect it to be removed before we release, but it will help
 during beta.

I shall add that for test purpose.
 
 * The compression algorithm depends completely upon new row length
 savings. If the new row is short, it would seem easier to just skip
 the checks and include it anyway. We can say if old and new vary in
 length by  50% of each other, just include new as-is, since the rows
 very clearly differ in a big way.

I think it makes more sense. So I shall update the patch.

 Also, if tuple is same length as
 before, can we compare the whole tuple at once to save doing
 per-column checks?

I shall evaluate and discuss with you.

 * If full page writes is on and the page is very old, we are just
 going to copy the whole block. So why not check for that rather than
 do all these push ups and then just copy the page anyway?

I shall check once and update the patch.

 * TOAST is not handled at all. No comments about it, nothing. Does
 that mean it hasn't been considered? Or did we decide not to care in
 this release? 

 Presumably that means we are comparing toast pointers
 byte by byte to see if they are the same?

Yes, currently this patch is doing byte by byte comparison for toast
pointers. I shall add comment.
In future, we can evaluate if further optimizations can be done.

 * I'd like to see a specific test in regression that is designed to
 exercise the code here. That way we will be certain that the code is
 getting regularly tested.

I shall add more specific tests.

 
 * The internal docs are completely absent. We need at least a whole
 page of descriptive comment, discussing trade-offs and design
 decisions. This is very important because it will help locate bugs
 much faster if these things are clealry documented. It also helps
 reviewers. This is a big timewaster for committers because you have to
 read the whole patch and understand it before you can attempt to form
 opinions. Commits happen quicker and easier with good comments.

Do you have any suggestion for where to put this information, any particular
ReadMe?

 * Lots of typos in comments. Many comments say nothing more than the
 words already used in the function name itself
 
 * flags variables are almost always int or uint in PG source.

 * PGLZ_HISTORY_SIZE needs to be documented in the place it is defined,
 not the place its used. The test if (oldtuplen  PGLZ_HISTORY_SIZE)
 really needs to be a test inside the compression module to maintain
 better modularity, so the value itself needn't be exported

I shall update the patch to address it.

 
 * Need to mention the WAL format change, or include the change within
 the patch so we can see

Sure, I will update this in code comments and internals docs.


With Regards,
Amit Kapila.



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


Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation

2012-12-28 Thread Amit Kapila
On Friday, December 28, 2012 1:38 PM Kyotaro HORIGUCHI wrote:
 Hello, I saw this patch and confirmed that
 
  - Coding style looks good.
  - Appliable onto HEAD.
  - Some mis-codings are fixed.
 
 And took the performance figures for 4 types of modification versus 2
 benchmarks.
 
 As a whole, this patch brings very large gain in its effective range -
 e.g. updates of relatively small portions in a tuple, but negligible
 loss of performance is observed outside of its effective range on the
 test machine. I suppose the losses will be emphasized by the more
 higher performance of seq write of WAL devices


Thank you very much for the review.

With Regards,
Amit Kapila.



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


[HACKERS] pg_stat_statements: calls under-estimation propagation

2012-12-28 Thread Daniel Farina
Hello,

After long delay (sorry) here's a patch implementing what was
hand-waved at in
http://archives.postgresql.org/pgsql-hackers/2012-10/msg00176.php

I am still something at a loss at how to test it besides prodding it
by hand; it seems like it's going to involve infrastructure or
introducing hooks into pg_stat_statements for the express purpose.

The patch can also be sourced from:

  https://github.com/fdr/postgres.git error-prop-pg_stat_statements

Without further ado, the cover letter taken from the top of the patch:

This tries to establish a maximum under-estimate of the number of
calls for a given pg_stat_statements entry.  That means the number of
calls to the canonical form of the query is between 'calls' and 'calls
+ calls_underest'.

This is useful to determine when accumulating statistics if a given
record is bouncing in and out of the pg_stat_statements table, having
its ncalls reset all the time, but also having calls_underest grow
very rapidly.

Records that always stay in pg_stat_statements will have a
calls_underest that do not change at all.

An interesting case is when a query that usually is called is not
called for a while, and falls out of pg_stat_statements.  The result
can be that the query with the most 'calls' can also have more
uncertainty than the query with the second most calls, which is also
exactly the truth in reality.

Unceremoniously bundled into this patch is a reduction in the minimum
table size for pg_stat_statements, from 100 to 1.  Using tiny values
is not likely to be seen in production, but makes testing the patch a
lot easier in some situations.

I will add this to the commitfest.

--
fdr


add-pg_stat_statements-calls-underestimation-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] Performance Improvement by reducing WAL for Update Operation

2012-12-28 Thread Simon Riggs
On 28 December 2012 11:27, Amit Kapila amit.kap...@huawei.com wrote:

 * The internal docs are completely absent. We need at least a whole
 page of descriptive comment, discussing trade-offs and design
 decisions. This is very important because it will help locate bugs
 much faster if these things are clealry documented. It also helps
 reviewers. This is a big timewaster for committers because you have to
 read the whole patch and understand it before you can attempt to form
 opinions. Commits happen quicker and easier with good comments.

 Do you have any suggestion for where to put this information, any particular
 ReadMe?

Location is less relevant, since it will show up as additions in the patch.

Put it wherever makes most sense in comparison to existing related
comments/README. I have no preference myself.

If its any consolation, I notice a common issue with patches is lack
of *explanatory* comments, as opposed to line by line comments. So
same review comment to 50-75% of patches I've reviewed recently, which
is also likely why.

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


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


Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation

2012-12-28 Thread Simon Riggs
On 28 December 2012 11:27, Amit Kapila amit.kap...@huawei.com wrote:

 * TOAST is not handled at all. No comments about it, nothing. Does
 that mean it hasn't been considered? Or did we decide not to care in
 this release?

 Presumably that means we are comparing toast pointers
 byte by byte to see if they are the same?

 Yes, currently this patch is doing byte by byte comparison for toast
 pointers. I shall add comment.
 In future, we can evaluate if further optimizations can be done.

Just a comment to say that the comparison takes place after TOASTed
columns have been removed. TOAST is already optimised for whole value
UPDATE anyway, so that is the right place to produce the delta.

It does make me think that we can further optimise TOAST by updating
only the parts of a toasted datum that have changed. That will be
useful for JSON and XML applications that change only a portion of
large documents.

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


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


Re: [HACKERS] Proposal: Store timestamptz of database creation on pg_database

2012-12-28 Thread Fabrízio de Royes Mello
Hi all,

And about proposal that originated this thread... I proposed only to add a
column on shared catalog pg_database with timestamp of its creation.

Event triggers don't cover CREATE DATABASE statement.

Regards,

-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Blog sobre TI: http://fabriziomello.blogspot.com
 Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Hannu Krosing

On 12/28/2012 09:15 AM, Peter Eisentraut wrote:

On Mon, 2012-12-17 at 16:34 -0500, Peter Eisentraut wrote:

Yes, this would be a good solution for some applications, but the only
way I can think of to manage the compatibility issue is to invent some
function attribute system like

CREATE FUNCTION ... OPTIONS (call_convention 'xyz')

An alternative that has some amount of precedent in the Python world
would be to use comment pragmas, like this:

 CREATE FUNCTION foo(a,b,c) AS $$
 # plpython: module

You mean something along the lines of how source encoding is
defined in http://www.python.org/dev/peps/pep-0263/ ?

Sounds like a plan :)

 import x
  from __future__ import nex_cool_feature

  def helper_function(x):
 ...

  def __pg_main__(a,b,c):
  defined function body here

 $$;

The source code parser would look for this string on, say, the first two
lines, and then decide which way to process the source text.

This way we could get this done fairly easily without any new
infrastructure outside the language handler.


I would still not name the defined function with any special static name
(like the samples __pg_main__) but have it default to the name of
PostgreSQL-defined function with possibility to override :

I don't much like the name __pg_main__ for the exported function what 
about __call__ ?


__call__ is used when making an object callable in python, but is not 
used at module

level so modules can't be callables in python.

Maybe just export the function with the same name as is defined in 
CREATE FUNCTION ?


And in case we do want to override it, call it

CREATE FUNCTION foo(a int,b int, c text) AS $$
# plpython: module modulename
import x,y,z

def foo(a,b,c):
return a + b + c

def foo2(a,b,c):
return foo(a,b,int(c))

__call__ = foo2

$$ language plpythonu


If we always require the export using __call__ in new-style pl/python 
functions we could
leave out the # plpython: module  part altogether and just test for 
__call__ in the compiled
modules namespace after trying to compile the source code as module and 
re-compile

using current methods if this fails.

To make this fast also for old-style functions, we should store compiled 
bytecode (.pyc) in
database as well, perhaps putting it in pg_proc.probin as a base64 
encoded string (probin
is not used fro non-C functions) or adding a new bytea column 
pg_proc.procode especially for this.


the module name could default to something more predictable, like 
function name + argtypes

so that it would become exportable by other  plpython functions

thus

CREATE FUNCTION foo(a int,b int, c text)

would create module

plpy.modules.foo_int4_int4_text

and

CREATE FUNCTION foo()

would become simply

plpy.modules.foo

in this case we could reuse the code so that if foo(a,b,c) from the previous 
example needs to be exported it would be just

CREATE FUNCTION foo(a int,b int, c int) AS $$
from plpy.modules.foo_int4_int4_text import foo2
__call__ = foo2
$$ language plpythonu

or even simpler

CREATE FUNCTION foo(a int,b int, c int) AS $$
from plpy.modules.foo_int4_int4_text import foo2 as __call__
$$ language plpythonu

---
Hannu
 







Hannu




























--
Sent 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-12-28 Thread Stephen Frost
Pavel, Peter,

To be honest, I haven't been following this thread at all, but I'm kind
of amazed that this patch seems to be progressing.  I spent quite a bit
of time previously trying to change the CSV log structure to add a
single column and this patch is adding a whole slew of them.  My prior
patch also allowed customization of the CSV log output, which seems like
it would be even more valuable if we're going to be adding a bunch more
fields.  I know there are dissenting viewpoints on that, however, as
application authors would prefer to not have to deal with variable CSV
output formats, which I can understand.

As regards this specific patch, I trust that the protocol error response
changes won't have any impact on existing clients?  Do we anticipate
something like the JDBC driver having any issues?  If there's any chance
that we're going to break a client by sending it things which it won't
understand, it seems we'd need to bump the protocol (which hasn't been
done in quite some time and would likely needs its own discussion...).

Regarding qualifying what we're returning- I tend to agree with Pavel
that if we're going to provide this information, we should do it in a
fully qualified manner.  I can certainly see situations where constraint
names are repeated in environments and it may not be clear what schema
it's in (think application development with a dev and a test schema in
the same database), and the same could hold for constraints against
tables (though having those repeated would certainly be more rare, since
you can't repeat a named constraint in a given schema if it has an index
behind it, though you could with simple CHECK constraints).  Again, my
feeling is that if we're going to provide such information, it should be
fully-qualified.

There are some additional concerns regarding the patch itself that I
have (do we really want to modify ereport() in this way?  How can we
implement something which scales better than just adding more and more
parameters?) but I think we need to figure out exactly what we're agreed
to be doing with this patch and get buy-in from everyone first.

Thanks,

Stephen

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I rechecked your version eelog4 and I am thinking so it is ok. From my
 view it can be enough for application developer and I am not against
 to commit this (or little bit reduced version) as first step.
 
 As plpgsql developer I really miss a fields routine_name,
 routine_schema and trigger_name. These fields can be simply
 implemented without any impact on performance. Because routine_name
 and routine_schema is not unique in postgres, I propose third field
 routine_oid. My patch eelog5 is based on your eelog4 with enhancing
 for these mentioned fields - 5KB more - but only PL/pgSQL is supported
 now. I would to find a acceptable design now.
 
 Second - I don't see any value for forwarding these new fields
 (column_name, table_name, constraint_name, schema_name, routine_name,
 routine_schema, routine_id, trigger_name) to log or to csvlog and I
 propose don't push it to log. We can drop logging in next iteration if
 you agree.
 
 What do you thinking about new version?
 
 Regards
 
 Pavel
 
 
 
 2012/12/10 Peter Geoghegan pe...@2ndquadrant.com:
  So, I took a look at this, and made some more changes.
 
  I have a hard time seeing the utility of some fields that were in this
  patch, and so they have been removed from this revision.
 
  Consider, for example:
 
  +   constraint_table text,
  +   constraint_schema text,
 
  While constraint_name remains, I find it really hard to believe that
  applications need a perfectly unambiguous idea of what constraint
  they're dealing with. If applications have a constraint that has a
  different domain-specific meaning depending on what schema it is in,
  while a table in one schema uses that constraint in another, well,
  that's a fairly awful design. Is it something that we really need to
  care about? You mentioned the SQL standard, but as far as I know the
  SQL standard has nothing to say about these fields within something
  like errdata - their names are lifted from information_schema, but
  being unambiguous is hardly so critical here. We want to identify an
  object for the purposes of displaying an error message only. Caring
  deeply about ambiguity seems wrong-headed to me in this context.
 
  +   routine_name text,
  +   trigger_name text,
  +   trigger_table text,
  +   trigger_schema text,
 
  The whole point of an exception (which ereport() is very similar to)
  is to decouple errors from error handling as much as possible - any
  application that magically takes a different course of action based on
  where that error occurred as reported by the error itself (and not the
  location of where handling the error occurs) seems kind of
  wrong-headed to me. Sure, a backtrace may be supplied, but that's only
  for diagnostic purposes, and a human can just as easily get that
  

Re: [HACKERS] enhanced error fields

2012-12-28 Thread Pavel Stehule
Hello

2012/12/28 Stephen Frost sfr...@snowman.net:
 Pavel, Peter,

 To be honest, I haven't been following this thread at all, but I'm kind
 of amazed that this patch seems to be progressing.  I spent quite a bit
 of time previously trying to change the CSV log structure to add a
 single column and this patch is adding a whole slew of them.  My prior
 patch also allowed customization of the CSV log output, which seems like
 it would be even more valuable if we're going to be adding a bunch more
 fields.  I know there are dissenting viewpoints on that, however, as
 application authors would prefer to not have to deal with variable CSV
 output formats, which I can understand.

Some smarter design of decision what will be stored to CSV and what
now can be great. I am not thinking so these enhanced fields has value
pro typical DBA and should be stored to CSV only when somebody need
it. But it is different story - although it can simplify our work
because then we don't need to solve this issue.


 As regards this specific patch, I trust that the protocol error response
 changes won't have any impact on existing clients?  Do we anticipate
 something like the JDBC driver having any issues?  If there's any chance
 that we're going to break a client by sending it things which it won't
 understand, it seems we'd need to bump the protocol (which hasn't been
 done in quite some time and would likely needs its own discussion...).


Depends on implementation. Implementations designed similar to libpq
should not have a problems.


 Regarding qualifying what we're returning- I tend to agree with Pavel
 that if we're going to provide this information, we should do it in a
 fully qualified manner.  I can certainly see situations where constraint
 names are repeated in environments and it may not be clear what schema
 it's in (think application development with a dev and a test schema in
 the same database), and the same could hold for constraints against
 tables (though having those repeated would certainly be more rare, since
 you can't repeat a named constraint in a given schema if it has an index
 behind it, though you could with simple CHECK constraints).  Again, my
 feeling is that if we're going to provide such information, it should be
 fully-qualified.

 There are some additional concerns regarding the patch itself that I
 have (do we really want to modify ereport() in this way?  How can we
 implement something which scales better than just adding more and more
 parameters?) but I think we need to figure out exactly what we're agreed
 to be doing with this patch and get buy-in from everyone first.

Related fields are fundamental - and I am thinking so is good to
optimize it - it has semantic tag, and tag has one char size. Some set
of fields is given by ANSI - we can select some subset because not all
fields described by ANSI has sense in pg. I don't would to enhance
range of this patch too much and I don't would to redesign current
concept of error handling. I am thinking so it works well and I have
no negative feedback from PostgreSQL users that I know.

But probably only reserved memory for ErrorData is limit for
serialized custom fields - I believe so we will have a associative
array - and one field can be of this type for any custom fields.

Regards

Pavel


 Thanks,

 Stephen

 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I rechecked your version eelog4 and I am thinking so it is ok. From my
 view it can be enough for application developer and I am not against
 to commit this (or little bit reduced version) as first step.

 As plpgsql developer I really miss a fields routine_name,
 routine_schema and trigger_name. These fields can be simply
 implemented without any impact on performance. Because routine_name
 and routine_schema is not unique in postgres, I propose third field
 routine_oid. My patch eelog5 is based on your eelog4 with enhancing
 for these mentioned fields - 5KB more - but only PL/pgSQL is supported
 now. I would to find a acceptable design now.

 Second - I don't see any value for forwarding these new fields
 (column_name, table_name, constraint_name, schema_name, routine_name,
 routine_schema, routine_id, trigger_name) to log or to csvlog and I
 propose don't push it to log. We can drop logging in next iteration if
 you agree.

 What do you thinking about new version?

 Regards

 Pavel



 2012/12/10 Peter Geoghegan pe...@2ndquadrant.com:
  So, I took a look at this, and made some more changes.
 
  I have a hard time seeing the utility of some fields that were in this
  patch, and so they have been removed from this revision.
 
  Consider, for example:
 
  +   constraint_table text,
  +   constraint_schema text,
 
  While constraint_name remains, I find it really hard to believe that
  applications need a perfectly unambiguous idea of what constraint
  they're dealing with. If applications have a constraint that has a
  different domain-specific meaning depending 

Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Andrew Dunstan


On 12/28/2012 03:15 AM, Peter Eisentraut wrote:

On Mon, 2012-12-17 at 16:34 -0500, Peter Eisentraut wrote:

Yes, this would be a good solution for some applications, but the only
way I can think of to manage the compatibility issue is to invent some
function attribute system like

CREATE FUNCTION ... OPTIONS (call_convention 'xyz')

An alternative that has some amount of precedent in the Python world
would be to use comment pragmas, like this:

 CREATE FUNCTION foo(a,b,c) AS $$
 # plpython: module
 import x
  from __future__ import nex_cool_feature

  def helper_function(x):
 ...

  def __pg_main__(a,b,c):
  defined function body here

 $$;

The source code parser would look for this string on, say, the first two
lines, and then decide which way to process the source text.

This way we could get this done fairly easily without any new
infrastructure outside the language handler.




Quite true, but I have wanted something along the lines of function 
attributes for a long time (can't find the email thread right now, but I 
do seem to recall it being discussed in some form), so I think there's a 
good case for the OPTIONS mechanism.


Making the language handler pull this out of the function text seems a 
bit ugly too.


cheers

andrew


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


Re: [HACKERS] SPI API and exceptions

2012-12-28 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 SPI was invented before there was proper exception handling, so it
 communicates errors by return values.  This makes programming with SPI
 either prone to errors of omission, or very ugly (ultimately, the
 standard reasons why exceptions were invented).

 So I was pondering whether we could introduce exceptions to SPI in some
 way.  I'm not sure how.  We could invent an entirely separate set of
 functions, or do some tricks in the header that things before
 differently depending on some #define.  Any ideas?

For practically all the nontrivial SPI functions, callers already have
to expect that exceptions can be thrown; just not for the specific
error causes called out as SPI return codes.  So I wonder whether we
could get away with just converting all the SPI errors to elogs as well.
It would change them from specifically-handled errors into general-case
errors, but for many call sites that's no problem.

Note though that there are some cases where handling of expected
errors would get a lot more annoying if we made them into elogs;
for instance SPI_ERROR_NOATTRIBUTE from SPI_fnumber() and friends.
That particular case might be solvable by defining SPI_ERROR_NOATTRIBUTE
as InvalidAttrNumber.  I think you'd need to go around and look at all
uses of each SPI_ERROR_XXX code before deciding that it's just dead
weight and should be converted to a generic exception.

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] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Tom Lane
 On 12/28/2012 09:15 AM, Peter Eisentraut wrote:
 An alternative that has some amount of precedent in the Python world
 would be to use comment pragmas, like this: ...
 This way we could get this done fairly easily without any new
 infrastructure outside the language handler.

+1 for not cluttering the generic CREATE FUNCTION syntax for this.
I note the parallel to plpgsql's #option syntax, too.

Hannu Krosing ha...@krosing.net writes:
 To make this fast also for old-style functions, we should store compiled 
 bytecode (.pyc) in
 database as well, perhaps putting it in pg_proc.probin as a base64 
 encoded string (probin
 is not used fro non-C functions) or adding a new bytea column 
 pg_proc.procode especially for this.

That might or might not be worth doing.  In the absence of evidence that
plpython byte-compiling represents significant overhead for normal use,
it sounds a lot like premature optimization to me.  In any case, it
should certainly not be done as part of the same patch Peter is thinking
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


[HACKERS] Behaviour of bgworker with SIGHUP

2012-12-28 Thread Guillaume Lelarge
Hi,

Today, I tried to make fun with the new background worker processes in
9.3, but I found something disturbing, and need help to go further.

My code is available on https://github.com/gleu/stats_recorder. If you
take a look, it is basically a copy of Alvarro's worker_spi contrib
module with a few changes. It compiles, and installs OK.

With this code, when I change my config option (stats_recorder.naptime),
I see that PostgreSQL gets the new value, but my background worker
process doesn't. See these log lines:

LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG:  received SIGHUP, reloading configuration files
LOG:  parameter stats_recorder.naptime changed to 5
LOG:  stats recorder, worker_spi_sighup
LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1

Is it the work of the function (pointed by bgw_sighup) to get the new
config values from the postmaster? and if so, how can I get these new
values?

I thought the configuration reloading would work just like a shared
library but it doesn't seem so. I wondered if it was because I had the
sighup function (initialized with bgw_sighup), so I got rid of it. The
new behaviour was actually more surprising as it launched _PG_init each
time I did a pg_ctl reload.

LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG:  stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG:  received SIGHUP, reloading configuration files
LOG:  stats_recorder, _PG_init
FATAL:  cannot create PGC_POSTMASTER variables after startup
LOG:  worker process: stats recorder (PID 5435) exited with exit code 1

Is it the expected behaviour?

Thanks.

Regards.


-- 
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com



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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes:
 To be honest, I haven't been following this thread at all, but I'm kind
 of amazed that this patch seems to be progressing.  I spent quite a bit
 of time previously trying to change the CSV log structure to add a
 single column and this patch is adding a whole slew of them.

I don't think that part's been agreed to at all; it seems entirely
possible that it'll get dropped if/when the patch gets committed.
I'm not convinced that having these fields in the log is worth
the compatibility hit of changing the CSV column set.

 As regards this specific patch, I trust that the protocol error response
 changes won't have any impact on existing clients?  Do we anticipate
 something like the JDBC driver having any issues?  If there's any chance
 that we're going to break a client by sending it things which it won't
 understand, it seems we'd need to bump the protocol (which hasn't been
 done in quite some time and would likely needs its own discussion...).

I think we are okay on this.  The protocol definition has said from the
very beginning Since more field types might be added in future,
frontends should silently ignore fields of unrecognized type.  A client
that fails to do that is buggy, not grounds for bumping the protocol.

I haven't been following the patch so have no thoughts about your
other 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


[HACKERS] proposal: ANSI SQL 2011 syntax for named parameters

2012-12-28 Thread Pavel Stehule
Hello

I am not sure, but maybe is time to introduce ANSI SQL syntax for
functions' named parameters

It is defined in ANSI SQL 2011

 CALL P (B = 1, A = 2)

instead PostgreSQL syntax CALL ( B := 1, A := 2)

Patch is very simple, but there are lot of questions about support
previous syntax.

* should we support both - probably yes

* how long time we will support pg syntax? - 2..5..ever years

* when we mark pg syntax as obsolete?

* when we remove pg syntax?

Regards

Pavel


ansi_named_params.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] pg_stat_statements: calls under-estimation propagation

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 11:43, Daniel Farina drfar...@acm.org wrote:
 Without further ado, the cover letter taken from the top of the patch:

 This tries to establish a maximum under-estimate of the number of
 calls for a given pg_stat_statements entry.  That means the number of
 calls to the canonical form of the query is between 'calls' and 'calls
 + calls_underest'.

Cool.

One possible issue I see with this is that this code:

+
+   /* propagate calls under-estimation bound */
+   entry-counters.calls_underest = pgss-calls_max_underest;
+

which appears within entry_alloc(). So if the first execution of the
query results in an error during planning or (much more likely)
execution, as in the event of an integrity constraint violation,
you're going to get a dead sticky entry that no one will ever see.
Now, we currently decay sticky entries much more aggressively, so the
mere fact that we waste an entry isn't a real problem. This was
established by this commit:

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

However, with this approach, calls_underest values might appear to the
user in what might be considered an astonishing order. Now, I'm not
suggesting that that's a real problem - just that they may not be the
semantics we want, particularly as we can reasonably defer assigning a
calls_underest until a sticky entry is unstuck, and an entry becomes
user-visible, within pgss_store().

Also, it seems like you should initialise pgss-calls_max_underest,
within pgss_shmem_startup(). You should probably serialise the value
to disk, and initialise it to 0 if there is no such value to begin
with.

I wonder if the way that pg_stat_statements throws its hands up when
it comes to crash safety (i.e. the contents of the hash table are
completely lost) could be a concern here. In other words, a program
tasked with tracking execution costs over time and graphing
time-series data from snapshots has to take responsibility for
ensuring that there hasn't been a crash (or, indeed, a reset).

Another issue is that I don't think that what you've done here solves
the problem of uniquely identify each entry over time, in the same way
that simply exposing the hash value would. I'm concerned with the
related problem to the problem solved here - simply identifying the
entry uniquely. As we've already discussed, the query string is an
imperfect proxy for each entry, even with constants swapped with '?'
characters (and even when combined with the userid and dbid values -
it's still not the same as the hashtable key, in a way that is likely
to bother people that use pg_stat_statements for long enough).

I think you probably should have created a
PG_STAT_STATEMENTS_COLS_V1_1 macro, since that version of the module
is now legacy, like *V1_0 is in HEAD.

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


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


Re: [HACKERS] XLByte* usage

2012-12-28 Thread Alvaro Herrera
Andres Freund escribió:
 On 2012-12-17 13:16:47 -0500, Tom Lane wrote:
  Andres Freund and...@2ndquadrant.com writes:
   On 2012-12-17 12:47:41 -0500, Tom Lane wrote:
   But, if the day ever comes when 64 bits doesn't seem like enough, I bet
   we'd move to 128-bit integers, which will surely be available on all
   platforms by then.  So +1 for using plain comparisons --- in fact, I'd
   vote for running around and ripping out the macros altogether.  I had
   already been thinking of fixing the places that are still using memset
   to initialize XLRecPtrs to invalid.
 
   I thought about that and had guessed you would be against it because it
   would cause useless diversion of the branches? Otherwise I am all for
   it.
 
  That's the only argument I can see against doing it --- but Heikki's
  patch was already pretty invasive in the same areas this would touch,
  so I'm thinking this won't make back-patching much worse.
 
 I thought a while about this for while and decided its worth trying to
 this before the next review round of xlogreader.

I have applied these three patches, after merging for recent changes.
Thanks.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


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


[HACKERS] Whats the correct way to change trigdata-tg_relation

2012-12-28 Thread Charles Gomes
I'm creating a simple trigger that will be called during an insert and change 
the destination table.
All values are going to be preserved, just the destination table will be 
different.

From what I see I can't modify trigdata-tg_relation.

All examples use: return Datum(trigdata-tg_trigtuple); // however tg_relation 
does not belong there. I'm trying to avoind having to do a SPI_EXEC;
Should I create a new heap_tuple and call heap_insert() and then Return 
Datum(NULL); ? Or is there another more straight forward way of doing it?  
Looks like if I call heap_insert I will have to update the indexes somehow.

Thank you,
Charles   

-- 
Sent 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-12-28 Thread Peter Geoghegan
On 28 December 2012 14:00, Stephen Frost sfr...@snowman.net wrote:
 There are some additional concerns regarding the patch itself that I
 have (do we really want to modify ereport() in this way?  How can we
 implement something which scales better than just adding more and more
 parameters?) but I think we need to figure out exactly what we're agreed
 to be doing with this patch and get buy-in from everyone first.

I don't think that the need to scale beyond what we have in my
revision really exists. Some of the ereport sites are a bit unwieldy,
but I don't see that there is much that can be done about that - you
need to specify the information somewhere, and it makes sense to do it
at that point. The field names are frequently expanded in the error
message presented to the user anyway.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 15:57, Tom Lane t...@sss.pgh.pa.us wrote:
 I don't think that part's been agreed to at all; it seems entirely
 possible that it'll get dropped if/when the patch gets committed.
 I'm not convinced that having these fields in the log is worth
 the compatibility hit of changing the CSV column set.

I am willing to let that go, because it just doesn't seem important to
me. I just thought that if we were going to break compatibility, we
might as well not hold back on the inclusion of fields. I'm not sure
where this leaves the regular log. Pavel wants to remove that, too. I
defer to others.

Now, as to the question of whether we need to make sure that
everything is always fully qualified - I respectfully disagree with
Stephen, and maintain my position set out in the last round of
feedback [1], which is that we don't need to fully qualify everything,
because *for the purposes of error handling*, which is what I think we
should care about, these fields are sufficient:

+   column_name text,
+   table_name text,
+   constraint_name text,
+   schema_name text,

If you use the same constraint name everywhere, you might get the same
error message. The situations in which this scheme might fall down are
too implausible for me to want to bloat up all those ereport sites any
further (something that Stephen also complained about).

I think that the major outstanding issues are concerning whether or
not I have the API here right. I make explicit guarantees as to the
availability of certain fields for certain errcodes (a small number of
Class 23 - Integrity Constraint Violation codes). No one has really
said anything about that, though I think it's important.

I also continue to think that we shouldn't have routine_name,
routine_schema and trigger_name fields - I think it's wrong-headed
to have an exception handler magically act on knowledge about where
the exception came from that has been baked into the exception - is
there any sort of precedent for this? Pavel disagrees here. Again, I
defer to others.

[1] Post of revision eelog4.diff:
http://archives.postgresql.org/message-id/caeylb_um9z8vitjckaogg2shab1n-71doznhj2pjm2ls96q...@mail.gmail.com
-- 
Peter Geoghegan   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services


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


Re: [HACKERS] A stab at implementing better password hashing, with mixed results

2012-12-28 Thread Alastair Turner
On Thu, Dec 27, 2012 at 5:39 PM, Peter Bex peter@xs4all.nl wrote:
 On Thu, Dec 27, 2012 at 12:31:08PM -0300, Claudio Freire wrote:
 On Thu, Dec 27, 2012 at 11:46 AM, Peter Bex peter@xs4all.nl wrote:
 
  Implementing a more secure challenge-response based algorithm means
  a change in the client-server protocol.  Perhaps something like SCRAM
  (maybe through SASL) really is the way forward for this, but that
  seems like quite a project and it seems to dictate how the passwords are
  stored; it requires a hash of the PBKDF2 algorithm to be stored.

 It would be nonsense to do it in any other way... protecting the
 password store and not the exchange would just shift the weak spot.

 Yeah, that's why I was being rather pessimistic about the patch I posted.
 However, SCRAM will only protect the password; SSL is still required
 to protect against connection hijacking.

The thread that ended with
http://archives.postgresql.org/message-id/5086cb7a.5040...@gmx.net
also tended towards SASL and SCRAM as the best direction for
developing password and GSSAPI/Kerberos authentication.


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


Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Hannu Krosing

On 12/28/2012 09:15 AM, Peter Eisentraut wrote:

On Mon, 2012-12-17 at 16:34 -0500, Peter Eisentraut wrote:

Yes, this would be a good solution for some applications, but the only
way I can think of to manage the compatibility issue is to invent some
function attribute system like

CREATE FUNCTION ... OPTIONS (call_convention 'xyz')

An alternative that has some amount of precedent in the Python world
would be to use comment pragmas, like this:

 CREATE FUNCTION foo(a,b,c) AS $$
 # plpython: module
 import x
  from __future__ import nex_cool_feature

  def helper_function(x):
 ...

  def __pg_main__(a,b,c):
  defined function body here

 $$;

The source code parser would look for this string on, say, the first two
lines, and then decide which way to process the source text.

This way we could get this done fairly easily without any new
infrastructure outside the language handler.

Peter, are you expecting to make write this patch ?

If so, then the very same approach (except the comment pragma
magic which is not needed there) is already done in python-postgres/be plpy
language handler. ( git://github.com/python-postgres/be.git )
Except there the modules exported function is named __main__ :)

And as a matter of bikeshedding I'd still prefer to name exported
function __call__ .

-
Hannu




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


Re: [HACKERS] XLByte* usage

2012-12-28 Thread Andres Freund
On 2012-12-28 14:59:50 -0300, Alvaro Herrera wrote:
 Andres Freund escribió:
  On 2012-12-17 13:16:47 -0500, Tom Lane wrote:
   Andres Freund and...@2ndquadrant.com writes:
On 2012-12-17 12:47:41 -0500, Tom Lane wrote:
But, if the day ever comes when 64 bits doesn't seem like enough, I bet
we'd move to 128-bit integers, which will surely be available on all
platforms by then.  So +1 for using plain comparisons --- in fact, I'd
vote for running around and ripping out the macros altogether.  I had
already been thinking of fixing the places that are still using memset
to initialize XLRecPtrs to invalid.
  
I thought about that and had guessed you would be against it because it
would cause useless diversion of the branches? Otherwise I am all for
it.
  
   That's the only argument I can see against doing it --- but Heikki's
   patch was already pretty invasive in the same areas this would touch,
   so I'm thinking this won't make back-patching much worse.
 
  I thought a while about this for while and decided its worth trying to
  this before the next review round of xlogreader.

 I have applied these three patches, after merging for recent changes.
 Thanks.

Thanks!

Greetings,

Andres Freund

--
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


Re: [HACKERS] Proposal: Store timestamptz of database creation on pg_database

2012-12-28 Thread Josh Berkus

On 12/28/12 4:05 AM, Fabrízio de Royes Mello wrote:

Hi all,

And about proposal that originated this thread... I proposed only to add a
column on shared catalog pg_database with timestamp of its creation.

Event triggers don't cover CREATE DATABASE statement.


Works for me, in that case.

You'd need something a lot more mature than checking the file timestamp 
on PG_VERSION, though.



--
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] enhanced error fields

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 18:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 If
 I understand you, we have a fields that has behave that you expected -
 filename and funcname. And I have not used these fields for
 application programming.

No, that's not what I mean. What I mean is that it seems questionable
to want to do anything *within* an exception handler on the basis of
where an exception was raised from. You should just place exception
handlers more carefully instead.

Are you aware of any popular programming language that provides this
kind of information? Can you tell in a well-principled way what
function a Python exception originated from, for example? These are
the built-in Python 2 exception classes:

http://docs.python.org/2/library/exceptions.html

None of the Python built-in exception types have this kind of
information available from fields or anything. Now, you might say that
Python isn't Postgres, and you'd be right. I'm fairly confident that
you'll find no precedent for a routine_name field in any code that is
even roughly analogous to the elog infrastructure, though, because
acting on the basis of what particular function the exception was
raised from seems quite hazardous - it breaks any kind of
encapsulation that might have existed.

If one person slightly refactors their code, it could break the code
of another person who has never even met or talked to the first.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Pavel Stehule
2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 18:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 If
 I understand you, we have a fields that has behave that you expected -
 filename and funcname. And I have not used these fields for
 application programming.

 No, that's not what I mean. What I mean is that it seems questionable
 to want to do anything *within* an exception handler on the basis of
 where an exception was raised from. You should just place exception
 handlers more carefully instead.

 Are you aware of any popular programming language that provides this
 kind of information? Can you tell in a well-principled way what
 function a Python exception originated from, for example? These are
 the built-in Python 2 exception classes:

 http://docs.python.org/2/library/exceptions.html


for this subject ANSI SQL is more relevant source or manual for DB2 or
Oracle. Design of Python and native PL languages are different. Python
can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
for work with simply scalar types. So these environments are not
comparable.

 None of the Python built-in exception types have this kind of
 information available from fields or anything. Now, you might say that
 Python isn't Postgres, and you'd be right. I'm fairly confident that
 you'll find no precedent for a routine_name field in any code that is
 even roughly analogous to the elog infrastructure, though, because
 acting on the basis of what particular function the exception was
 raised from seems quite hazardous - it breaks any kind of
 encapsulation that might have existed.

 If one person slightly refactors their code, it could break the code
 of another person who has never even met or talked to the first.

yes, it is not valid argument, I am sorry. Lot of error fields doesn't
work if developer doesn't respect some coding standards. It is not
just routine_name. Only when your implementation is correct, then code
works.

Best regards

Pavel


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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 19:23, Pavel Stehule pavel.steh...@gmail.com wrote:
 for this subject ANSI SQL is more relevant source or manual for DB2 or
 Oracle. Design of Python and native PL languages are different. Python
 can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
 for work with simply scalar types. So these environments are not
 comparable.

I don't see how the fact that Python can use nested data structures
has any bearing (you could argue that plpgsql does too, fwiw).

Please direct me towards the manual of DB2 or Oracle where it says
that something like routine_name is exposed for error handling
purposes. Correct me if I'm mistaken, but I don't believe that ANSI
SQL has anything to say about any of these error fields. You've just
lifted the names of the fields from various information_schema
catalogs, which is hardly the same thing.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Pavel Stehule
2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 19:23, Pavel Stehule pavel.steh...@gmail.com wrote:
 for this subject ANSI SQL is more relevant source or manual for DB2 or
 Oracle. Design of Python and native PL languages are different. Python
 can use complex nested structures. PL - PL/pgSQL or PL/PSM is designed
 for work with simply scalar types. So these environments are not
 comparable.

 I don't see how the fact that Python can use nested data structures
 has any bearing (you could argue that plpgsql does too, fwiw).

 Please direct me towards the manual of DB2 or Oracle where it says
 that something like routine_name is exposed for error handling
 purposes. Correct me if I'm mistaken, but I don't believe that ANSI
 SQL has anything to say about any of these error fields. You've just
 lifted the names of the fields from various information_schema
 catalogs, which is hardly the same thing.

http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm
http://savage.net.au/SQL/sql-2003-2.bnf - GET DIAGNOSTICS statement

SQL: 1999, Jim Melton,Alan R. Simon - description of GET DIAGNOSTICS statement
ANSI SQL - Feature F122, Enhanced diagnostics management - table
identifiers for use with get diagnostics statement - and related
description

Regards

Pavel




please, search

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Eisentraut
On 12/10/12 4:23 PM, Peter Geoghegan wrote:
 Well, this is an area that the standard doesn't have anything to say
 about. The standard defines errcodes, but not what fields they make
 available. I suppose you could say that the patch proposes that they
 become a Postgres extension to the standard.

The standard certainly does define extra fields for errors; see get
diagnostics statement.  We have a GET DIAGNOSTICS statement in
PL/pgSQL, so there is certainly precedent for all of 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] Submission Review: User control over psql error stream

2012-12-28 Thread Alastair Turner
Hi Karl,

Sorry for the slow reply ...

Excerpt from Karl O. Pinc k...@meme.com On Mon, Dec 10, 2012 at 5:00 AM:

 I was thinking along the same lines, that case 2) stderr to a file
 or pipe needs addressing.  I think it's necessary to address the
 issue now.  Otherwise we risk cluttering up the syntax in
 inhospitable ways.

The discussion needs to be a little broader than stdout and stderr,
there are currently three output streams from psql:
 - stdout - prompts, not tabular output such as the results from \set and \c
 - stderr - errors, notices, ...
 - query output - result from queries and \ commands which return
tables such as \l - this is what is currently piped or redirected by
\o

I see a patch like this adding a fourth - diagnostic output. While
this would probably be the same as stderr initially but I think that
the option to make them subtly different should be kept open.

 It occurs to me that my reply does not address the issue
 of case 3, displaying or not) errors to the terminal in
 addition to wherever else errors are sent.

I don't know of any precedent syntax for this - in *nix shells you'd
pipe to tee rather than modify the redirect in some way.

 I think you're right, whether or not errors continue to be sent
 to stdout when they are directed elsewhere should be a separate
 flag.  My inclination is to have another special psql variable
 to control this but that would break backwards compatibility.
 Instead, let's work out a syntax for the rest of the functionality
 and try to fit this in.

 One nice thing about special psql variables is that they self-report
 their state.  I mention this since we're adding more state.
 It would be nice if the chosen syntax does not preclude some
 additional tweaking to report on the state involving the
 output streams.  (Although I don't think that needs to be
 a part of this patch.)

State reporting and reset tend to use the same syntax - the command
without parameters. I think the best route to achieve this would be
for the commands to set a variables which would be reported by \set.

In my initial list of what needed to be specified for output
management I missed one - whether file output should be appended or
should rewrite the file. It's not likely to be particularly useful for
query output but would matter for diagnostics.

Given all of these considerations I would propose changing your
calling syntax to:

\o reset all output redirection

\oq   reset query output redirection
\o[q] [] file   query output to file
\o[q]  file   append query output to file
\o[q] | progpipe query output to program
\o[q] {[]|||}+   display query output on stdout in addition to redirection

\od   reset query output redirection
\odq diagnostic output to query output stream
\odq +  diagnostic output to stderr and query output
stream (same as \odq and \o + in combination)
\od [] file diagnostic output to file
\od  file append diagnostic output to file
\od | prog  pipe diagnostic output to program
\od {[]|||}+ display diagnostic output on stderr in addition to
redirection

To meet its original goals your patch would need to implement \o, \od,
\odq and \odq +.

Regards,
Alastair.


-- 
Sent 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-12-28 Thread Peter Eisentraut
On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 No, that's not what I mean. What I mean is that it seems questionable
 to want to do anything *within* an exception handler on the basis of
 where an exception was raised from.

Isn't that the whole point of this patch?  The only purpose of this
feature is to make the exception information available in a
machine-readable way.  That functionality has been requested many
times over the years.


-- 
Sent 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-12-28 Thread Peter Geoghegan
On 28 December 2012 19:55, Pavel Stehule pavel.steh...@gmail.com wrote:
 http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm

I'm unconvinced by this. First of all, it only applies to the GET
DIAGNOSTICS statement, and the only implementation that actually
currently uses that is DB2, AFAICT. Secondly, DB2 only provides it for
very specific errcode classes that relate to problems that are
peculiar to routines/functions, so in general you cannot rely on the
information being available in the same way as you intend. Clearly,
DB2 provides it for completeness, and not because you can rely on it
being available for error handling purposes. On the other hand, your
latest revision of the patch (eelog5.patch) sees plpgsql jury-rigged
to set the fields itself, which seems like a whole other proposition
entirely.

What's more, you're changing ErrorData to make this happen, rather
than having the user interrogate the server for this information as
GET DIAGNOSTICS does. So I don't see that this supports your case at
all. I maintain that having an exception handler's behaviour vary
based on a field that describes a routine that originally raised the
function is a really bad idea, and that we should not facilitate it.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Eisentraut
On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 Are you aware of any popular programming language that provides this
 kind of information? Can you tell in a well-principled way what
 function a Python exception originated from, for example? These are
 the built-in Python 2 exception classes:
 
 http://docs.python.org/2/library/exceptions.html
 
 None of the Python built-in exception types have this kind of
 information available from fields or anything.

Sure, OSError has a filename attribute (which I'm sure is qualified by a
directory name if necessary), SyntaxError has filename, lineno, etc.

OSError.filename is essentially the equivalent of what is being proposed
here.


-- 
Sent 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-12-28 Thread Peter Geoghegan
On 28 December 2012 20:34, Peter Eisentraut pete...@gmx.net wrote:
 Isn't that the whole point of this patch?  The only purpose of this
 feature is to make the exception information available in a
 machine-readable way.  That functionality has been requested many
 times over the years.

Right, and I agree that it's very useful for some fields (if you can
actually have a reasonable set of guarantees about where each becomes
available). I just don't think that it's worth including fields like
routine_name within ErrorData, and in fact it may be harmful to do so.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Pavel Stehule
2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 19:55, Pavel Stehule pavel.steh...@gmail.com wrote:
 http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fdb2%2Frbafzmstgetdiag.htm

 I'm unconvinced by this. First of all, it only applies to the GET
 DIAGNOSTICS statement, and the only implementation that actually
 currently uses that is DB2, AFAICT. Secondly, DB2 only provides it for
 very specific errcode classes that relate to problems that are
 peculiar to routines/functions, so in general you cannot rely on the
 information being available in the same way as you intend. Clearly,
 DB2 provides it for completeness, and not because you can rely on it
 being available for error handling purposes. On the other hand, your
 latest revision of the patch (eelog5.patch) sees plpgsql jury-rigged
 to set the fields itself, which seems like a whole other proposition
 entirely.

 What's more, you're changing ErrorData to make this happen, rather
 than having the user interrogate the server for this information as
 GET DIAGNOSTICS does. So I don't see that this supports your case at
 all. I maintain that having an exception handler's behaviour vary
 based on a field that describes a routine that originally raised the
 function is a really bad idea, and that we should not facilitate it.

It cannot to wait to GET DIAGNOSTICS request - because when GET
DIAGNOSTICS is called, then all unsupported info about exception is
lost, all used memory will be released. So if we would to support
ROUTINE_NAME or similar fields like CONTEXT or MESSAGE, we have to
store these values to ErrorData without knowledge if this value will
be used or not.

Pavel


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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 20:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 It cannot to wait to GET DIAGNOSTICS request - because when GET
 DIAGNOSTICS is called, then all unsupported info about exception is
 lost, all used memory will be released.

I'm not suggesting that you do. I'm only suggesting that the two are
not comparable.

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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Pavel Stehule
2012/12/28 Peter Geoghegan pe...@2ndquadrant.com:
 On 28 December 2012 20:40, Pavel Stehule pavel.steh...@gmail.com wrote:
 It cannot to wait to GET DIAGNOSTICS request - because when GET
 DIAGNOSTICS is called, then all unsupported info about exception is
 lost, all used memory will be released.

 I'm not suggesting that you do. I'm only suggesting that the two are
 not comparable

MESSAGE and similar yes. But CONTEXT is comparable. And there is
interesting precedent. Some years PL/Python or PL/Perl doesn't support
CONTEXT and PL/pgSQL yes. And it was working.

Regards

Pavel


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


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


Re: [HACKERS] enhanced error fields

2012-12-28 Thread Peter Geoghegan
On 28 December 2012 20:40, Peter Eisentraut pete...@gmx.net wrote:
 Sure, OSError has a filename attribute (which I'm sure is qualified by a
 directory name if necessary), SyntaxError has filename, lineno, etc.

 OSError.filename is essentially the equivalent of what is being proposed
 here.

I disagree. That isn't the same as what's being proposed here, because
that's something you're only going to get for those particular
exception classes, and I'm guessing that the fields only exist to
facilitate refactoring tools, IDEs and the like.

If BaseException, Exception or StandardError had a function_name
field, and it was idiomatic to change the behaviour of an exception
handler on the basis of the field's value, that would be equivalent.
Obviously that is not the case.

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


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


Re: [HACKERS] multiple CREATE FUNCTION AS items for PLs

2012-12-28 Thread Peter Eisentraut
On 12/28/12 7:09 AM, Hannu Krosing wrote:
 Maybe just export the function with the same name as is defined in
 CREATE FUNCTION ?
 
 And in case we do want to override it, call it
 
 CREATE FUNCTION foo(a int,b int, c text) AS $$
 # plpython: module modulename
 import x,y,z
 
 def foo(a,b,c):
 return a + b + c
 
 def foo2(a,b,c):
 return foo(a,b,int(c))
 
 __call__ = foo2
 
 $$ language plpythonu

I like this approach, except I would call the attribute something like
__plpy_something___.

 If we always require the export using __call__ in new-style pl/python
 functions we could
 leave out the # plpython: module  part altogether and just test for
 __call__ in the compiled
 modules namespace after trying to compile the source code as module and
 re-compile
 using current methods if this fails.

That's a thought.  I'm not yet sure whether I prefer it.


-- 
Sent 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: ANSI SQL 2011 syntax for named parameters

2012-12-28 Thread Peter Eisentraut
On 12/28/12 11:22 AM, Pavel Stehule wrote:
 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters
 
 It is defined in ANSI SQL 2011
 
  CALL P (B = 1, A = 2)
 
 instead PostgreSQL syntax CALL ( B := 1, A := 2)

I agree it's probably time.

 * should we support both - probably yes

yes

 * how long time we will support pg syntax? - 2..5..ever years
 
 * when we mark pg syntax as obsolete?
 
 * when we remove pg syntax?

The := syntax was introduced in 9.0, so it is by now well entrenched.  I
don't think we should remove it at all any time soon.

As for documentation, just state how it is.  The standard syntax is =,
but because of $various_issues, older versions only 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] proposal: ANSI SQL 2011 syntax for named parameters

2012-12-28 Thread Gavin Flower

On 29/12/12 10:19, Peter Eisentraut wrote:

On 12/28/12 11:22 AM, Pavel Stehule wrote:

I am not sure, but maybe is time to introduce ANSI SQL syntax for
functions' named parameters

It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

instead PostgreSQL syntax CALL ( B := 1, A := 2)

I agree it's probably time.


* should we support both - probably yes

yes


* how long time we will support pg syntax? - 2..5..ever years

* when we mark pg syntax as obsolete?

* when we remove pg syntax?

The := syntax was introduced in 9.0, so it is by now well entrenched.  I
don't think we should remove it at all any time soon.

As for documentation, just state how it is.  The standard syntax is =,
but because of $various_issues, older versions only support :=.



To be honest I prefer *:=* as it looks neater than *=*, in part because 
I first saw that notation when I was learning ALGOL 60 and liked the 
justification they gave in the manual.


In fact I find *=* ugly and counter intuitive as I keep having the 
feeling that it points the wrong way, because *A = 2* suggests to me 
that you are setting '2' to the value of 'A' which is plain daft!


I am sure there are worse standardisation formats - but for some reason, 
I find this one disproportionately irritating!  :-)


So I would much prefer to keep the old format, if at all possible.


Cheers,
Gavin






Re: [HACKERS] dynamic SQL - possible performance regression in 9.2

2012-12-28 Thread Peter Eisentraut
On 12/27/12 1:07 AM, Pavel Stehule wrote:
 Hello
 
 I rechecked performance of dynamic SQL and it is significantly slower
 in 9.2 than 9.1
 
 -- 9.1
 postgres=# create or replace function test() returns void as $$ begin
 for i in 1..100 loop execute 'select 1'; end loop; end $$ language
 plpgsql;

I think this is the same as the case discussed at
CAD4+=qWnGU0qi+iq=eph6egpuunscysgdtgkazizevrggjo...@mail.gmail.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] dynamic SQL - possible performance regression in 9.2

2012-12-28 Thread Heikki Linnakangas

On 28.12.2012 23:53, Peter Eisentraut wrote:

On 12/27/12 1:07 AM, Pavel Stehule wrote:

Hello

I rechecked performance of dynamic SQL and it is significantly slower
in 9.2 than 9.1

-- 9.1
postgres=# create or replace function test() returns void as $$ begin
for i in 1..100 loop execute 'select 1'; end loop; end $$ language
plpgsql;


I think this is the same as the case discussed at
CAD4+=qWnGU0qi+iq=eph6egpuunscysgdtgkazizevrggjo...@mail.gmail.com.


Yeah, probably so.

As it happens, I just spent a lot of time today narrowing down yet 
another report of a regression in 9.2, when running DBT-2: 
http://archives.postgresql.org/pgsql-performance/2012-11/msg7.php. 
It looks like that is also caused by the plancache changes. DBT-2 
implements the transactions using C functions, which use SPI_execute() 
to run all the queries.


It looks like the regression is caused by extra copying of the parse 
tree and plan trees. Node-copy-related functions like AllocSetAlloc and 
_copy* are high in the profile, They are also high in the 9.1 profile, 
but even more so in 9.2.


I hacked together a quickdirty patch to reduce the copying of 
single-shot plans, and was able to buy back much of the regression I was 
seeing on DBT-2. Patch attached. But of course, DBT-2 really should be 
preparing the queries once with SPI_prepare, and reusing them thereafter.


- Heikki
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index e3de7f2..0b047d0 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -75,7 +75,7 @@ PrepareQuery(PrepareStmt *stmt, const char *queryString)
 	 * to see the unmodified raw parse tree.
 	 */
 	plansource = CreateCachedPlan(stmt-query, queryString,
-  CreateCommandTag(stmt-query));
+  CreateCommandTag(stmt-query), false);
 
 	/* Transform list of TypeNames to array of type OIDs */
 	nargs = list_length(stmt-argtypes);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 5a11c6f..d7522f1 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -49,7 +49,7 @@ static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
 		 ParamListInfo paramLI, bool read_only);
 
 static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan,
-  ParamListInfo boundParams);
+			  ParamListInfo boundParams, bool oneshot);
 
 static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
   Snapshot snapshot, Snapshot crosscheck_snapshot,
@@ -354,7 +354,7 @@ SPI_execute(const char *src, bool read_only, long tcount)
 	plan.magic = _SPI_PLAN_MAGIC;
 	plan.cursor_options = 0;
 
-	_SPI_prepare_plan(src, plan, NULL);
+	_SPI_prepare_plan(src, plan, NULL, true);
 
 	res = _SPI_execute_plan(plan, NULL,
 			InvalidSnapshot, InvalidSnapshot,
@@ -505,7 +505,7 @@ SPI_execute_with_args(const char *src,
 	paramLI = _SPI_convert_params(nargs, argtypes,
   Values, Nulls);
 
-	_SPI_prepare_plan(src, plan, paramLI);
+	_SPI_prepare_plan(src, plan, paramLI, true);
 
 	res = _SPI_execute_plan(plan, paramLI,
 			InvalidSnapshot, InvalidSnapshot,
@@ -546,7 +546,7 @@ SPI_prepare_cursor(const char *src, int nargs, Oid *argtypes,
 	plan.parserSetup = NULL;
 	plan.parserSetupArg = NULL;
 
-	_SPI_prepare_plan(src, plan, NULL);
+	_SPI_prepare_plan(src, plan, NULL, false);
 
 	/* copy plan to procedure context */
 	result = _SPI_make_plan_non_temp(plan);
@@ -583,7 +583,7 @@ SPI_prepare_params(const char *src,
 	plan.parserSetup = parserSetup;
 	plan.parserSetupArg = parserSetupArg;
 
-	_SPI_prepare_plan(src, plan, NULL);
+	_SPI_prepare_plan(src, plan, NULL, false);
 
 	/* copy plan to procedure context */
 	result = _SPI_make_plan_non_temp(plan);
@@ -1082,7 +1082,7 @@ SPI_cursor_open_with_args(const char *name,
 	paramLI = _SPI_convert_params(nargs, argtypes,
   Values, Nulls);
 
-	_SPI_prepare_plan(src, plan, paramLI);
+	_SPI_prepare_plan(src, plan, paramLI, true);
 
 	/* We needn't copy the plan; SPI_cursor_open_internal will do so */
 
@@ -1656,7 +1656,8 @@ spi_printtup(TupleTableSlot *slot, DestReceiver *self)
  * parsing is also left in CurrentMemoryContext.
  */
 static void
-_SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
+_SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams,
+  bool oneshot)
 {
 	List	   *raw_parsetree_list;
 	List	   *plancache_list;
@@ -1688,6 +1689,8 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
 		Node	   *parsetree = (Node *) lfirst(list_item);
 		List	   *stmt_list;
 		CachedPlanSource *plansource;
+		MemoryContext querytree_ctx;
+		MemoryContext oldctx;
 
 		/*
 		 * Create the CachedPlanSource before we do parse analysis, since it
@@ -1695,12 +1698,25 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan, ParamListInfo boundParams)
 		 */
 		plansource = CreateCachedPlan(parsetree,
 	  src,
-	  CreateCommandTag(parsetree));
+		

[HACKERS] ILIKE vs indices

2012-12-28 Thread James Cloos
While tuning an application, I found the posts from 2003 recomending the
use of LOWER() and LIKE in place of ILIKE to take advantage of indices.

For this app, given the limitations of the upper-layer protocol it must
support, that change replaced about 30 minutes of repeated seq scans with
about 1 minute of repeated index scans!  On a query-set often repeated
several times per day.  (Probably more times per day now.)

Is there any contraindication to recasting:

  foo ILIKE 'bar'

into:

  LOWER(foo) LIKE LOWER('bar')

and documenting that an index has to be on LOWER(column) to benefit ILIKE?

Perhaps the parser could read the former as the latter?

-JimC
-- 
James Cloos cl...@jhcloos.com OpenPGP: 1024D/ED7DAEA6


-- 
Sent 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-12-28 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 None of the Python built-in exception types have this kind of
 information available from fields or anything.

 Sure, OSError has a filename attribute (which I'm sure is qualified by a
 directory name if necessary), SyntaxError has filename, lineno, etc.

No no no ... that's completely unrelated.  Those fields report
information about the user-visible object that the error is about.

The point Peter G is making is that reporting the source of the error is
like having the kernel report the name of the function inside the kernel
that threw the error.  Or perhaps the name of the kernel source file
containing that function.

Now, these things are quite useful *to a kernel developer* who's trying
to debug a problem involving an error report.  But they're pretty
useless to an application developer, and certainly any application
developer who relied on such information to control his error handling
code would receive no sympathy when (not if) a new kernel version broke
it.

In the same way, the filename/lineno stuff that we include in ereports
is sometimes useful to Postgres developers --- but it is very hard to
see a reason why application code would do anything else with it except
possibly print it for human reference.

And, in the same way, a CONTEXT traceback is sometimes useful for
debugging a collection of server-side functions --- but it's hard to see
any client-side code using that information in a mechanized way, at
least not while respecting a sane degree of separation between
server-side and client-side code.

So I'm with Peter G on this: the existing CONTEXT mechanism is good
enough, we do not need to split out selected sub-parts of that as
separate error fields.  Moreover, doing so would provide only an utterly
arbitrary subset of the context stack: who's to say whether it would be
more important to report the most closely nested function, or the
outermost one?

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] ILIKE vs indices

2012-12-28 Thread Tom Lane
James Cloos cl...@jhcloos.com writes:
 Is there any contraindication to recasting:
   foo ILIKE 'bar'
 into:
   LOWER(foo) LIKE LOWER('bar')

In some locales those are not equivalent, I believe, or at least
shouldn't be.  (What the current code actually does is a separate
question.)

 Perhaps the parser could read the former as the latter?

Not unless the equivalence can be shown to be exact, which I doubt.
In any case it's not obvious why LOWER rather than UPPER.

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] Rewriter hook

2012-12-28 Thread Vlad Arkhipov

Hi all,

Are there any plans on adding a rewriter hook? There are already exist 
parser, planner, executor hooks but there is no way to control rewriter 
from plugins.


Some use cases:
1. Complex rules in C language.
2. Transforming an original query into a series of queries. For example, 
instead of UPDATE query on a table you may wish to execute UPDATE and 
INSERT into *the same* table.



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


Re: [HACKERS] Rewriter hook

2012-12-28 Thread Peter Geoghegan
On 29 December 2012 01:36, Vlad Arkhipov arhi...@dc.baikal.ru wrote:
 Are there any plans on adding a rewriter hook?

I doubt it will ever happen.

If you look at QueryRewrite(Query *parsetree), the primary entry point
to the rewriter, it's easy enough to get a high level overview of what
goes on.

You can get the post-parse-analyze tree by registering a
post_parse_analyze_hook (like pg_stat_statements), and the same tree
*after* rule expansion by registering a planner_hook. It isn't obvious
what you're missing that a hook into the rewriter would get you.

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


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


Re: [HACKERS] Rewriter hook

2012-12-28 Thread Jaime Casanova
On Fri, Dec 28, 2012 at 8:36 PM, Vlad Arkhipov arhi...@dc.baikal.ru wrote:

 Some use cases:
 1. Complex rules in C language.
 2. Transforming an original query into a series of queries. For example,
 instead of UPDATE query on a table you may wish to execute UPDATE and INSERT
 into *the same* table.


the second one you can do it with a trigger, and i'm pretty sure you
can use triggers to solve most of the problems... what are you trying
to do?

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Rewriter hook

2012-12-28 Thread Vlad Arkhipov

On 12/29/2012 11:05 AM, Jaime Casanova wrote:

On Fri, Dec 28, 2012 at 8:36 PM, Vlad Arkhipov arhi...@dc.baikal.ru wrote:

Some use cases:
1. Complex rules in C language.
2. Transforming an original query into a series of queries. For example,
instead of UPDATE query on a table you may wish to execute UPDATE and INSERT
into *the same* table.


the second one you can do it with a trigger, and i'm pretty sure you
can use triggers to solve most of the problems... what are you trying
to do?

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157



I'm trying to make an extension that rewrites UPDATE/DELETE queries to 
specific tables using some additional information via special functions 
into a query (in order to not extend the parser with special syntax). 
For example, a query


UPDATE my_table
SET val = 2
WHERE id = 1
  AND special_func(daterange('2000-02-01', '2000-03-01'));

needs to be rewritten into the following three queries:

UPDATE my_table SET val = 2
WHERE id = 1
  AND period @ daterange('2000-02-01', '2000-03-01');

UPDATE my_table SET period = period - daterange('2000-02-01', '2000-03-01')
WHERE id = 1
  AND period  daterange('2000-02-01', '2000-03-01')
  AND (period  daterange('2000-02-01', '2000-03-01')
OR period  daterange('2000-02-01', '2000-03-01'));

INSERT INTO my_table (id, val, period)
SELECT 1, 2, period * daterange('2000-02-01', '2000-03-01')
FROM my_table
WHERE id = 1
  AND period  daterange('2000-02-01', '2000-03-01')
  AND (NOT period  daterange('2000-02-01', '2000-03-01')
OR NOT period  daterange('2000-02-01', '2000-03-01'));

Here is the same query with special syntax (SQL-2011 variant):

UPDATE my_table FOR PORTION OF period_name FROM '2000-02-01' TO '2000-03-01'
SET val = 2
WHERE id = 1;


--
Sent 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-12-28 Thread Pavel Stehule
2012/12/29 Tom Lane t...@sss.pgh.pa.us:
 Peter Eisentraut pete...@gmx.net writes:
 On 12/28/12 2:03 PM, Peter Geoghegan wrote:
 None of the Python built-in exception types have this kind of
 information available from fields or anything.

 Sure, OSError has a filename attribute (which I'm sure is qualified by a
 directory name if necessary), SyntaxError has filename, lineno, etc.

 No no no ... that's completely unrelated.  Those fields report
 information about the user-visible object that the error is about.

 The point Peter G is making is that reporting the source of the error is
 like having the kernel report the name of the function inside the kernel
 that threw the error.  Or perhaps the name of the kernel source file
 containing that function.

 Now, these things are quite useful *to a kernel developer* who's trying
 to debug a problem involving an error report.  But they're pretty
 useless to an application developer, and certainly any application
 developer who relied on such information to control his error handling
 code would receive no sympathy when (not if) a new kernel version broke
 it.

 In the same way, the filename/lineno stuff that we include in ereports
 is sometimes useful to Postgres developers --- but it is very hard to
 see a reason why application code would do anything else with it except
 possibly print it for human reference.

I wrote exactly this - our filename, funcname is not useful for PL
application developers


 And, in the same way, a CONTEXT traceback is sometimes useful for
 debugging a collection of server-side functions --- but it's hard to see
 any client-side code using that information in a mechanized way, at
 least not while respecting a sane degree of separation between
 server-side and client-side code.

again - it is reason why I propose ROUTINE_NAME and TRIGGER_NAME - it
can be useful for some use cases, where developer should to handle
exception when they coming from known functions/triggers and he would
to raise exception, when it was raised elsewhere. Is possible working
with CONTEXT, but it needs little bit more work and situation is very
similar to fields COLUMN_NAME, where I can parse message and I can get
this information. But then it depends on message format.


 So I'm with Peter G on this: the existing CONTEXT mechanism is good
 enough, we do not need to split out selected sub-parts of that as
 separate error fields.  Moreover, doing so would provide only an utterly
 arbitrary subset of the context stack: who's to say whether it would be
 more important to report the most closely nested function, or the
 outermost one?

I don't agree with this argument - you can say too COLUMN_NAME,
TABLE_NAME is not necessary, because MESSAGE is good enough. I don't
see any difference - and I would to use these variables for error
handling (not for logging) without dependency on current format of
MESSAGE or CONTEXT.

Second question - what should be context is important. I am thinking
so standard and implementation in other databases is relative clean -
it is name of custom routine, that doesn't handle exception. Moving to
pg - it is name of routine on the top on error callback stack, because
builtin functions doesn't set it - I don't need on top of CONTEX -
div_int or other name of builtin function - but I need there function
foo(b): a := 10/b.

other point - theoretically I can get ROUTINE_NAME from CONTEXT, but I
cannot get TRIGGER_NAME - this information is lost.

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] dynamic SQL - possible performance regression in 9.2

2012-12-28 Thread Pavel Stehule
Hello

2012/12/28 Heikki Linnakangas hlinnakan...@vmware.com:
 On 28.12.2012 23:53, Peter Eisentraut wrote:

 On 12/27/12 1:07 AM, Pavel Stehule wrote:

 Hello

 I rechecked performance of dynamic SQL and it is significantly slower
 in 9.2 than 9.1

 -- 9.1
 postgres=# create or replace function test() returns void as $$ begin
 for i in 1..100 loop execute 'select 1'; end loop; end $$ language
 plpgsql;


 I think this is the same as the case discussed at
 CAD4+=qWnGU0qi+iq=eph6egpuunscysgdtgkazizevrggjo...@mail.gmail.com.


 Yeah, probably so.

 As it happens, I just spent a lot of time today narrowing down yet another
 report of a regression in 9.2, when running DBT-2:
 http://archives.postgresql.org/pgsql-performance/2012-11/msg7.php. It
 looks like that is also caused by the plancache changes. DBT-2 implements
 the transactions using C functions, which use SPI_execute() to run all the
 queries.

 It looks like the regression is caused by extra copying of the parse tree
 and plan trees. Node-copy-related functions like AllocSetAlloc and _copy*
 are high in the profile, They are also high in the 9.1 profile, but even
 more so in 9.2.

 I hacked together a quickdirty patch to reduce the copying of single-shot
 plans, and was able to buy back much of the regression I was seeing on
 DBT-2. Patch attached. But of course, DBT-2 really should be preparing the
 queries once with SPI_prepare, and reusing them thereafter.


performance regression is about 30-50%.

You copy_reduce_patch increase speed about 8%

Regards

Pavel


 - Heikki


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