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
>
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 |