Re: PL/PgSQL: RAISE and the number of parameters

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Marko Tiikkaja <marko(at)joh(dot)to>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/PgSQL: RAISE and the number of parameters
Date: 2014-08-12 13:28:29
Message-ID: CAFj8pRB0zk5eGV9HuVkOb=aT59r4qt24p_1CpCCbkypKmpp=Lw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2014-08-12 15:09 GMT+02:00 Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>:

>
> Hello,
>
>
> - I would suggest to avoid "continue" within a loop so that the code is
>>> simpler to understand, at least for me.
>>>
>>
>> I personally find the code easier to read with the continue.
>>
>
> Hmmm. I had to read the code to check it, and I did it twice. The point is
> that there is 3 exit points instead of 1 in the block, which is not in
> itself very bad, but:
>
> for(...) {
> if (ccc)
> xxx;
> ...
> foo++;
> }
>
> It looks like "foo++" is always executed, and you have to notice that xxx
> a few lines above is a continue to realise that it is only when ccc is
> false. This is also disconcerting if it happens to be the "rare" case, i.e.
> here most of the time the char is not '%', so most of the time foo is not
> incremented, although it is a the highest level. Also the code with
> continue does not really put forward that what is counted is '%' followed
> by a non '%'. Note that the corresponding execution code
> (pgplsql/src/pl_exec.c) uses one continue to get rid of the second '%',
> which is quite defendable in that case as it is a kind of exception, but
> the main condition remains a simple if block. Final argument, the
> structured version is shorter than the unstructured version, with just the
> two continues removed, and one negation also removed.
>
>
> to also turn the ereport()s into elog()s since the user should never see
>> them.
>>
>
> I'm not sure why elog is better than ereport in that case: ISTM that it is
> an error worth reporting if it ever happens, say there is another syntax
> added later on which is not caught for some reason by the compile-time
> check, so I would not change it.
>
>
one note: this patch can enforce a compatibility issues - a partially
broken functions, where some badly written RAISE statements was executed
newer.

I am not against this patch, but it should be in extra check probably ?? Or
we have to documented it as potential compatibility issue

Regards

Pavel

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gabriele Bartolini 2014-08-12 14:17:30 Re: Proposal: Incremental Backup
Previous Message Claudio Freire 2014-08-12 13:25:21 Re: Proposal: Incremental Backup