2012/7/2 Peter Geoghegan <pe...@2ndquadrant.com>:
> On 2 July 2012 15:19, Peter Geoghegan <pe...@2ndquadrant.com> wrote:
>> On 9 May 2012 14:33, Pavel Stehule <pavel.steh...@gmail.com> wrote:
>>> here is patch with enhancing ErrorData structure. Now constraints
>>> errors and RI uses these fields
>>
>> So I took a look at the patch eelog-2012-05-09.diff today. All of the
>> following remarks apply to it alone.
>
> I decided to follow through and take a look at
> eelog-plpgsql-2012-05-09.diff today too, while I have all of this
> swapped into my head.
>
> This patch is not an atomic unit - it builds upon the first patch. I
> successfully merged the local feature branch that I'd created for
> eelog-2012-05-09.diff without any merge conflicts, and I can build
> Postgres and get the regression tests to pass (including a couple of
> new ones, for this added functionality for plggsql - the functionality
> is testing exclusively using the new (9.2) "get stacked diagnostics"
> and "raise custom exception 'some_custom_exception' using...."
> feature).
>
> Since that feature branch had all my revisions committed, my
> observations about redundancies in the other base patch still stand -
> the 2 functions mentioned did not exist for the benefit of this
> further patch either.
>
> There is a typo here:
>
> + case PLPGSQL_RAISEOPTION_TRIGGER_SCHEMA:
> + printf(" TRIGGER_SCHENA = ");
> + break;
> }
>
>
> I'm not sure about this inconsistency within unreserved_keyword:
>
> For routines:
> + | K_DIAG_ROUTINE_NAME
> + | K_DIAG_ROUTINE_SCHEMA
> ....
>
> For triggers:
> + | K_DIAG_TRIGGER_NAME
> + | K_DIAG_TRIGGER_SCHEMA
> ....
>
> For tables:
> + | K_DIAG_SCHEMA_NAME
> .
> . **SNIP**
> .
> + | K_DIAG_TABLE_NAME
>
> The same inconsistency exists within the anonymous enum that contains
> PLPGSQL_GETDIAG_TABLE_NAME (and other constants), as well as the new
> token keywords within plpgsql's gram.y .
>
> The doc changes need a little work here too.
>
> I'm not sure that I agree with the extensive use of the term "routine"
> in all of these constants - sure, information_schema has a view called
> "routines". But wouldn't it be more appropriate to use a
> Postgres-centric term within our own code?
>
> So, what about the concern about performance taking a hit when plpgsql
> exception blocks are entered as a result of this patch? Well, while I
> think that an effort to reduce the overhead of PL exception handling
> would be worthwhile, these patches do not appear to alter things
> appreciable (though the overhead *is* measurable):
>
> [peter@peterlaptop eelog]$ ls
> exceptions.sql test_eelog_outer.sql
>
> Patch (eelog-plpgsql):
>
> [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
> transaction type: Custom query
> scaling factor: 1
> query mode: simple
> number of clients: 10
> number of threads: 1
> duration: 300 s
> number of transactions actually processed: 305756
> tps = 1019.026055 (including connections establishing)
> tps = 1019.090135 (excluding connections establishing)
>
> Master:
>
> [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
> transaction type: Custom query
> scaling factor: 1
> query mode: simple
> number of clients: 10
> number of threads: 1
> duration: 300 s
> number of transactions actually processed: 308376
> tps = 1027.908182 (including connections establishing)
> tps = 1027.977879 (excluding connections establishing)
>
> An archive with simple scripts for repeating this are attached, if
> anyone is interested.
yes, I think so slowdown is not significant for usual situations and
patterns. I got about 3% slowdown on code like
begin
insert into tab -- raise exception
exception when others ..
...
end;
and only when all inserts fails.
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
