Skip site navigation (1) Skip section navigation (2)

Re: enhanced error fields

From: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: enhanced error fields
Date: 2012-07-02 21:52:17
Message-ID: CAEYLb_Xz4Ru75LnvZqta8+pRa1razNJFiVZ2j4JChFR=7tn93Q@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On 2 July 2012 15:19, Peter Geoghegan <peter(at)2ndquadrant(dot)com> wrote:
> On 9 May 2012 14:33, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> here is patch with enhancing ErrorData structure. Now constraints
>> errors and RI uses these fields
>
> So I took a look at the patch eelog-2012-05-09.diff today. All of the
> following remarks apply to it alone.

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

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

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

There is a typo here:

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


I'm not sure about this inconsistency within unreserved_keyword:

For routines:
+ 				| K_DIAG_ROUTINE_NAME
+ 				| K_DIAG_ROUTINE_SCHEMA
....

For triggers:
+ 				| K_DIAG_TRIGGER_NAME
+ 				| K_DIAG_TRIGGER_SCHEMA
....

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

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

The doc changes need a little work here too.

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

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

[peter(at)peterlaptop eelog]$ ls
exceptions.sql  test_eelog_outer.sql

Patch (eelog-plpgsql):

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

Master:

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

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

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

Attachment: eelog.tar.gz
Description: application/x-gzip (607 bytes)

In response to

Responses

pgsql-hackers by date

Next:From: Robert HaasDate: 2012-07-02 22:16:42
Subject: Re: Patch: add conversion from pg_wchar to multibyte
Previous:From: Robert HaasDate: 2012-07-02 21:23:02
Subject: Re: Event Triggers reduced, v1

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group