Re: plpgsql.consistent_into

Hello

2014/1/12 Marko Tiikkaja <marko(at)joh(dot)to>

> Greetings fellow elephants,
>
> I would humbly like to submit for your consideration my proposal for
> alleviating pain caused by one of the most annoying footguns in PL/PgSQL:
> the behaviour of SELECT .. INTO when the query returns more than one row.
> Some of you might know that no exception is raised in this case (as
> opposed to INSERT/UPDATE/DELETE .. INTO, all of them yielding
> TOO_MANY_ROWS), which can hide subtle bugs in queries if during testing the
> query always returns only one row or the "correct" one happens to be picked
> up every time. Additionally, the row_count() after execution is always
> going to be either 0 or 1, so even if you want to explicitly guard against
> potentially broken queries, you can't do so!
>

It is not bad and, sure, - it is very useful and important

but - it is a redundant to INTO STRICT clause. When you use it, then you
change a INTO behaviour. Is not better to ensure STRICT option than hidden
redefining INTO?

Option INTO (without STRICT clause) is not safe and we should to disallow.
I see a three states (not only two)

a) disallow INTO without STRICT (as preferred for new code)
b) implicit check after every INTO without STRICT
c) without check

these modes should be: "strict_required", "strict_default", "strict_legacy"

> So I added the following compile-time option:
>
>
> set plpgsql.consistent_into to true;
>

This name is not best (there is not clean with it a into should be
consistent)

Is question, if this functionality should be enabled by GUC to be used for
legacy code (as protection against some kind of hidden bugs)

This topic is interesting idea for me - some checks can be pushed to
plpgsql_check (as errors or warnings) too.

Generally I like proposed functionality, just I am not sure, so hidden
redefining INTO clause (to INTO STRICT) is what we want. We can do it (but
explicitly). I don't know any situation where INTO without STRICT is valid.
Introduction of STRICT option was wrong idea - and now is not way to back.

Regards

Pavel

>
> create or replace function footest() returns void as $$
> declare
> x int;
> begin
> -- too many rows
> select 1 from foo into x;
> end$$ language plpgsql;
>
> select footest();
> ERROR: query returned more than one row
>
> It defaults to false to preserve full backwards compatibility. Also
> turning it on makes the executor try and find two rows, so it might have an
> effect on performance as well. The patch, as currently written, also
> changes the behaviour of EXECUTE .. INTO, but I don't feel strongly about
> whether that should be affected as well or not.
>
>
> Regards,
> Marko Tiikkaja
>
>
> --
> 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
>
>