Proposal: PL/PgSQL strict_mode

Hi,

After my previous suggestion for adding a STRICT keyword got shot
down[1], I've been thinking about an idea Andrew Gierth tossed out:
adding a new "strict mode" into PL/PgSQL. In this mode, any query which
processes more than one row will raise an exception. This is a bit
similar to specifying INTO STRICT .. for every statement, except
processing no rows does not trigger the exception. The need for this
mode comes from a few observations I make almost every day:
1) The majority of statements only deal with exactly 0 or 1 rows.
2) Checking row_count for a statement is ugly and cumbersome, so
often it just isn't checked. I often use RETURNING TRUE INTO STRICT _OK
for DML, but that a) requires an extra variable, and b) isn't possible
if 0 rows affected is not an error in the application logic.
3) SELECT .. INTO only fetches one row and ignores the rest. Even
row_count is always set to 0 or 1, so there's no way to fetch a value
*and* to check that there would not have been more rows. This creates
bugs which make your queries return wrong results and which could go
undetected for a long time.

Attached is a proof-of-concept patch (no docs, probably some actual code
problems too) to implement this as a compile option:

=# create or replace function footest() returns void as $$
$# #strict_mode strict
$# begin
$# -- not allowed to delete more than one row
$# delete from foo where f1 < 100;
$# end$$ language plpgsql;
CREATE FUNCTION
=# select footest();
ERROR: query processed more than one row
CONTEXT: PL/pgSQL function footest() line 5 at SQL statement

Now while I think this is a step into the right direction, I do have a
couple of problems with this patch:
1) I'm not sure what would be the correct behaviour with EXECUTE.
I'm tempted to just leave EXECUTE alone, as it has slightly different
rules anyway.
2) If you're running in strict mode and you want to
insert/update/delete more than one row, things get a bit uglier; a wCTE
would work for some cases. If strict mode doesn't affect EXECUTE (see
point 1 above), that could work too. Or maybe there could be a new
command which runs a query, discards the results and ignores the number
of rows processed.

I'll be adding this to the open commitfest in hopes of getting some
feedback on this idea (I'm prepared to hear a lot of "you're crazy!"),
but feel free to comment away any time you please.

Regards,
Marko Tiikkaja

[1]: http://www.postgresql.org/message-id/510BF731.5020802@gmx.net

On 14/09/2013 06:28, I wrote:
> 2) Checking row_count for a statement is ugly and cumbersome, so
> often it just isn't checked. I often use RETURNING TRUE INTO STRICT _OK
> for DML, but that a) requires an extra variable, and b) isn't possible
> if 0 rows affected is not an error in the application logic.

The b) part here wasn't exactly true; you could use RETURNING TRUE INTO
OK as INSERT/UPDATE/DELETE with RETURNING .. INTO raises an exception if
it returns more than one row, but it's even more convoluted than the
RETURNING TRUE INTO STRICT _OK, and it's repetitive.

Regards,
Marko Tiikkaja

A few thoughts about this.

> On 14 September 2013 at 05:28 Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>
>
> Hi,
>
> After my previous suggestion for adding a STRICT keyword got shot
> down[1], I've been thinking about an idea Andrew Gierth tossed out:
> adding a new "strict mode" into PL/PgSQL. In this mode, any query which
> processes more than one row will raise an exception. This is a bit
> similar to specifying INTO STRICT .. for every statement, except
> processing no rows does not trigger the exception. The need for this
> mode comes from a few observations I make almost every day:
> 1) The majority of statements only deal with exactly 0 or 1 rows.
> 2) Checking row_count for a statement is ugly and cumbersome, so
> often it just isn't checked. I often use RETURNING TRUE INTO STRICT _OK
> for DML, but that a) requires an extra variable, and b) isn't possible
> if 0 rows affected is not an error in the application logic.
> 3) SELECT .. INTO only fetches one row and ignores the rest. Even
> row_count is always set to 0 or 1, so there's no way to fetch a value
> *and* to check that there would not have been more rows. This creates
> bugs which make your queries return wrong results and which could go
> undetected for a long time.

I am going to suggest that STRICT is semantically pretty far from what is meant
here in common speech. I think STRICT here would be confusing. This would be
really pretty severe for people coming from Perl or MySQL backgrounds.

May I suggest SINGLE as a key word instead? It might be worth having attached
to a INSERT, UPDATE, and DELETE statements.

I am thinking something like:

DELETE SINGLE FROM foo WHERE f1 < 1000;

would be more clearer. Similarly one could have:

INSERT SINGLE INTO foo SELECT * from foo2;

and

UPDATE SINGLE foo

You could even use SELECT SINGLE but not sure where the use case is there where
unique indexes are not sufficient.

>
> Attached is a proof-of-concept patch (no docs, probably some actual code
> problems too) to implement this as a compile option:
>
> =# create or replace function footest() returns void as $$
> $# #strict_mode strict
> $# begin
> $# -- not allowed to delete more than one row
> $# delete from foo where f1 < 100;
> $# end$$ language plpgsql;
> CREATE FUNCTION
> =# select footest();
> ERROR: query processed more than one row
> CONTEXT: PL/pgSQL function footest() line 5 at SQL statement
>
> Now while I think this is a step into the right direction, I do have a
> couple of problems with this patch:
> 1) I'm not sure what would be the correct behaviour with EXECUTE.
> I'm tempted to just leave EXECUTE alone, as it has slightly different
> rules anyway.
> 2) If you're running in strict mode and you want to
> insert/update/delete more than one row, things get a bit uglier; a wCTE
> would work for some cases. If strict mode doesn't affect EXECUTE (see
> point 1 above), that could work too. Or maybe there could be a new
> command which runs a query, discards the results and ignores the number
> of rows processed.

Yeah, I am worried about this one. I am concerned that if you can't disable on
a statement by statement basis, then you have a problem where you end up
removing the mode from the function and then it becomes a lot harder for
everyone maintaining the function to have a clear picture of what is going on.
I am further worried that hacked ugly code ways around it will introduce plenty
of other maintenance pain points that will be worse than what you are solving.
>
> I'll be adding this to the open commitfest in hopes of getting some
> feedback on this idea (I'm prepared to hear a lot of "you're crazy!"),
> but feel free to comment away any time you please.

Well, I don't know if my feedback above is helpful, but there it is ;-)
>
>
> Regards,
> Marko Tiikkaja
>
> [1]: http://www.postgresql.org/message-id/510BF731.5020802@gmx.net
>
> --
> 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
Best Wishes,
Chris Travers
http://www.2ndquadrant.com
PostgreSQL Services, Training, and Support

2013/9/14 chris travers <chris(at)2ndquadrant(dot)com>

> **
> A few thoughts about this.
>
> > On 14 September 2013 at 05:28 Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> >
> >
> > Hi,
> >
> > After my previous suggestion for adding a STRICT keyword got shot
> > down[1], I've been thinking about an idea Andrew Gierth tossed out:
> > adding a new "strict mode" into PL/PgSQL. In this mode, any query which
> > processes more than one row will raise an exception. This is a bit
> > similar to specifying INTO STRICT .. for every statement, except
> > processing no rows does not trigger the exception. The need for this
> > mode comes from a few observations I make almost every day:
> > 1) The majority of statements only deal with exactly 0 or 1 rows.
> > 2) Checking row_count for a statement is ugly and cumbersome, so
> > often it just isn't checked. I often use RETURNING TRUE INTO STRICT _OK
> > for DML, but that a) requires an extra variable, and b) isn't possible
> > if 0 rows affected is not an error in the application logic.
> > 3) SELECT .. INTO only fetches one row and ignores the rest. Even
> > row_count is always set to 0 or 1, so there's no way to fetch a value
> > *and* to check that there would not have been more rows. This creates
> > bugs which make your queries return wrong results and which could go
> > undetected for a long time.
>
> I am going to suggest that STRICT is semantically pretty far from what is
> meant here in common speech. I think STRICT here would be confusing. This
> would be really pretty severe for people coming from Perl or MySQL
> backgrounds.
>
> May I suggest SINGLE as a key word instead? It might be worth having
> attached to a INSERT, UPDATE, and DELETE statements.
>

I don't think so SINGLE is better. There is a risk of confusing with LIMIT
clause. (More - we use a STRICT now, so new keyword increase a possible
confusing)

When I look on translation of "STRICT" to Czech language, I am thinging so
STRICT is good enough.

If we do some change, then I propose a little bit Cobol solution CHECK ONE
ROW EXPECTED" clause.

instead DELETE FROM foo WHERE f1 < 1000

do

DELETE FROM foo WHERE f1 < 1000 CHECK ONE ROW EXPECTED;

Regards

Pavel

> I am thinking something like:
>
> DELETE SINGLE FROM foo WHERE f1 < 1000;
>
> would be more clearer. Similarly one could have:
>
> INSERT SINGLE INTO foo SELECT * from foo2;
>
> and
>
> UPDATE SINGLE foo
>
> You could even use SELECT SINGLE but not sure where the use case is there
> where unique indexes are not sufficient.
>
>
> >
> > Attached is a proof-of-concept patch (no docs, probably some actual code
> > problems too) to implement this as a compile option:
> >
> > =# create or replace function footest() returns void as $$
> > $# #strict_mode strict
> > $# begin
> > $# -- not allowed to delete more than one row
> > $# delete from foo where f1 < 100;
> > $# end$$ language plpgsql;
> > CREATE FUNCTION
> > =# select footest();
> > ERROR: query processed more than one row
> > CONTEXT: PL/pgSQL function footest() line 5 at SQL statement
> >
> > Now while I think this is a step into the right direction, I do have a
> > couple of problems with this patch:
> > 1) I'm not sure what would be the correct behaviour with EXECUTE.
> > I'm tempted to just leave EXECUTE alone, as it has slightly different
> > rules anyway.
> > 2) If you're running in strict mode and you want to
> > insert/update/delete more than one row, things get a bit uglier; a wCTE
> > would work for some cases. If strict mode doesn't affect EXECUTE (see
> > point 1 above), that could work too. Or maybe there could be a new
> > command which runs a query, discards the results and ignores the number
> > of rows processed.
>
> Yeah, I am worried about this one. I am concerned that if you can't
> disable on a statement by statement basis, then you have a problem where
> you end up removing the mode from the function and then it becomes a lot
> harder for everyone maintaining the function to have a clear picture of
> what is going on. I am further worried that hacked ugly code ways around
> it will introduce plenty of other maintenance pain points that will be
> worse than what you are solving.
> >
> > I'll be adding this to the open commitfest in hopes of getting some
> > feedback on this idea (I'm prepared to hear a lot of "you're crazy!"),
> > but feel free to comment away any time you please.
>
> Well, I don't know if my feedback above is helpful, but there it is ;-)
> >
> >
> > Regards,
> > Marko Tiikkaja
> >
> > [1]: http://www.postgresql.org/message-id/510BF731.5020802@gmx.net
> >
> > --
> > 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
> Best Wishes,
> Chris Travers
> http://www.2ndquadrant.com
> PostgreSQL Services, Training, and Support
>

On 14/09/2013 06:53, chris travers wrote:
> I am going to suggest that STRICT is semantically pretty far from what is meant
> here in common speech. I think STRICT here would be confusing. This would be
> really pretty severe for people coming from Perl or MySQL backgrounds.

The name of the feature doesn't really matter. I chose STRICT because
the behaviour is somewhat similar to INTO .. STRICT.

> May I suggest SINGLE as a key word instead? It might be worth having attached
> to a INSERT, UPDATE, and DELETE statements.

I already suggested the "attach a keyword to every statement" approach,
and got shot down quite quickly.

Regards,
Marko Tiikkaja

(Oops, just realized I only replied to a part of your message. I'm
blaming it on my lack of sleep.)

On 14/09/2013 06:53, chris travers wrote:
>> 2) If you're running in strict mode and you want to
>> insert/update/delete more than one row, things get a bit uglier; a wCTE
>> would work for some cases. If strict mode doesn't affect EXECUTE (see
>> point 1 above), that could work too. Or maybe there could be a new
>> command which runs a query, discards the results and ignores the number
>> of rows processed.
>
> Yeah, I am worried about this one. I am concerned that if you can't disable on
> a statement by statement basis, then you have a problem where you end up
> removing the mode from the function and then it becomes a lot harder for
> everyone maintaining the function to have a clear picture of what is going on.
> I am further worried that hacked ugly code ways around it will introduce plenty
> of other maintenance pain points that will be worse than what you are solving.

I completely agree. If you can't turn the option on globally, you're
bound to hit problems at some point.

My thinking currently is that this mode would require adding a new type
of statement (I'd call it PERFORM, but I know that one's been taken, so
please don't get hung up on that):

PERFORM UPDATE foo SET ..;
or
PERFORM SELECT foo(id) FROM ..;

This statement would allow you to run any statement in strict mode
without hitting the "more than one row processed" error message.

>> I'll be adding this to the open commitfest in hopes of getting some
>> feedback on this idea (I'm prepared to hear a lot of "you're crazy!"),
>> but feel free to comment away any time you please.
>
> Well, I don't know if my feedback above is helpful, but there it is ;-)

Thanks for your input!

Regards,
Marko Tiikkaja