Thread: PL/PgSQL STRICT

Hi,

Courtesy of me, Christmas comes a bit early this year.  I wrote a patch
which allows you to add STRICT into PERFORM and INSERT/UPDATE/DELETE
without specifying an INTO clause.  Observe:

=# create table foo(a int);
CREATE TABLE
=# create function foof() returns void as $$ begin update strict foo set
a=a+1; end $$ language plpgsql;
CREATE FUNCTION
=# select foof();
ERROR:  query returned no rows

I know everyone obviously wants this, so I will be sending another
version with regression tests and documentation later.  The code is a
bit ugly at places, but I'm going to work on that too.


Regards,
Marko Tiikkaja

Marko Tiikkaja <pgmail@joh.to> writes:
> Courtesy of me, Christmas comes a bit early this year.  I wrote a patch 
> which allows you to add STRICT into PERFORM and INSERT/UPDATE/DELETE 
> without specifying an INTO clause.

What is the use-case for this?  Won't this result in the word STRICT
becoming effectively reserved in contexts where it currently is not?
(IOW, even if the feature is useful, I've got considerable doubts about
this syntax for it.  The INTO clause is an ugly, badly designed kluge
already --- let's not make another one just like it.)
        regards, tom lane

On 12/21/12 4:39 PM, Tom Lane wrote:
> What is the use-case for this?

Currently, the way to do this would be something like:

DECLARE  _ok bool;
BEGIN
UPDATE foo .. RETURNING TRUE INTO STRICT _ok;

We have a lot of code like this, and I bet others do as well.

> Won't this result in the word STRICT
> becoming effectively reserved in contexts where it currently is not?

It will, which probably is not ideal if it can be avoided.  I also 
considered syntax like  INTO STRICT NULL,  but that felt a bit ugly.  It 
would be great if someone had any smart ideas about the syntax.


Regards,
Marko Tiikkaja

On 12/21/12 4:49 PM, I wrote:
> On 12/21/12 4:39 PM, Tom Lane wrote:
>> What is the use-case for this?
>
> Currently, the way to do this would be something like:

I realize I didn't really answer the question.

The use case is when you're UPDATEing or DELETEing a row and you want to 
quickly assert that there should be exactly one row.  For example, if 
you've previously locked a row with SELECT .. FOR UPDATE, and now you 
want to UPDATE or DELETE it, it better be there (or you have a bug 
somewhere).


Regards,
Marko Tiikkaja

On 12/21/12 4:49 PM, I wrote:
>> Won't this result in the word STRICT
>> becoming effectively reserved in contexts where it currently is not?
>
> It will, which probably is not ideal if it can be avoided.  I also
> considered syntax like  INTO STRICT NULL,  but that felt a bit ugly.  It
> would be great if someone had any smart ideas about the syntax.

Another idea would be to force the STRICT to be immediately after 
INSERT, UPDATE or DELETE.


Regards,
Marko Tiikkaja

2012/12/21 Marko Tiikkaja <pgmail@joh.to>:
> On 12/21/12 4:49 PM, I wrote:
>>
>> On 12/21/12 4:39 PM, Tom Lane wrote:
>>>
>>> What is the use-case for this?
>>
>>
>> Currently, the way to do this would be something like:
>
>
> I realize I didn't really answer the question.
>
> The use case is when you're UPDATEing or DELETEing a row and you want to
> quickly assert that there should be exactly one row.  For example, if you've
> previously locked a row with SELECT .. FOR UPDATE, and now you want to
> UPDATE or DELETE it, it better be there (or you have a bug somewhere).
>

yes, it has sense

probably only after keyword it should be simple implementable

Regards

Pavel

>
>
> Regards,
> Marko Tiikkaja
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-
> owner@postgresql.org] On Behalf Of Marko Tiikkaja
> Sent: Friday, December 21, 2012 10:53 AM
> To: Tom Lane
> Cc: PostgreSQL-development
> Subject: Re: [HACKERS] PL/PgSQL STRICT
> 
> On 12/21/12 4:49 PM, I wrote:
> > On 12/21/12 4:39 PM, Tom Lane wrote:
> >> What is the use-case for this?
> >
> > Currently, the way to do this would be something like:
> 
> I realize I didn't really answer the question.
> 
> The use case is when you're UPDATEing or DELETEing a row and you want to
> quickly assert that there should be exactly one row.  For example, if
you've
> previously locked a row with SELECT .. FOR UPDATE, and now you want to
> UPDATE or DELETE it, it better be there (or you have a bug somewhere).

There had better be exactly one row - but who cares whether that is the row
we were actually expecting to delete/update...

I've recently had the experience of missing a "WHERE pk = ..." clause in an
UPDATE statement inside a function so I do see the value in having an "easy
to implement" safety idiom along these lines.

Along the lines of "EXPLAIN (options) CMD" would something like
"UPDATE|DELETE (STRICT) identifier" work?

David J.

Marko Tiikkaja <pgmail@joh.to> writes:
> Another idea would be to force the STRICT to be immediately after 
> INSERT, UPDATE or DELETE.

What about before it, ie
STRICT UPDATE ...

This should dodge the problem of possible conflict with table names,
and it seems to me to read more naturally too.
        regards, tom lane

2012/12/21 Tom Lane <tgl@sss.pgh.pa.us>:
> Marko Tiikkaja <pgmail@joh.to> writes:
>> Another idea would be to force the STRICT to be immediately after
>> INSERT, UPDATE or DELETE.
>
> What about before it, ie
>
>         STRICT UPDATE ...
>
> This should dodge the problem of possible conflict with table names,
> and it seems to me to read more naturally too.

+1

Pavel



>
>                         regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On 12/21/12 5:09 PM, Christopher Browne wrote:
> I could use GET DIAGNOSTICS to determine if nothing got altered, and
> it seems likely to me that expressly doing this via IF/ELSE/END IF would
> be easier to read in function code than a somewhat magic STRICT
> side-effect.

STRICT is used in INTO, so PL/PgSQL users should already have an idea 
what it's going to do outside of INTO.

> I certainly appreciate that brevity can make things more readable, it's
> just
> that I'm not sure that is much of a help here.
>
> This is adding specific syntax for what seems like an unusual case to me,
> which seems like an unworthwhile complication.

A quick grep suggests that our (the company I work for) code base has 
160 occurrences of INSERT/UPDATE/DELETE followed by IF NOT FOUND THEN 
RAISE EXCEPTION.  So it doesn't seem like an unusual case to me.

Of course, some of them couldn't use STRICT because they are expected to 
happen (in which case they can send a more descriptive error message), 
but most of them could.


Regards,
Marko Tiikkaja

On 12/21/12 5:22 PM, Tom Lane wrote:
> Marko Tiikkaja <pgmail@joh.to> writes:
>> Another idea would be to force the STRICT to be immediately after
>> INSERT, UPDATE or DELETE.
>
> What about before it, ie
>
>     STRICT UPDATE ...
>
> This should dodge the problem of possible conflict with table names,
> and it seems to me to read more naturally too.

Yeah, putting STRICT after the command wouldn't work for UPDATE.

I like this one best so far, so I'm going with this syntax for the next 
version of the patch.



Regards,
Marko Tiikkaja

On Fri, Dec 21, 2012 at 4:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> What about before it, ie
>
>         STRICT UPDATE ...

+1 from me too.
This feature would be awesome.

Christopher Browne <cbbrowne@gmail.com> writes:
> This is adding specific syntax for what seems like an unusual case to me,
> which seems like an unworthwhile complication.

That was my first reaction too, but Marko's followon examples seem to
make a reasonable case for it.  There are many situations where you
expect an UPDATE or DELETE to hit exactly one row.  Often, programmers
won't bother to add code to check that it did ... but if a one-word
addition to the command can provide such a check, it seems more likely
that they would add the check.
        regards, tom lane

2012/12/21 Tom Lane <tgl@sss.pgh.pa.us>:
> Christopher Browne <cbbrowne@gmail.com> writes:
>> This is adding specific syntax for what seems like an unusual case to me,
>> which seems like an unworthwhile complication.
>
> That was my first reaction too, but Marko's followon examples seem to
> make a reasonable case for it.  There are many situations where you
> expect an UPDATE or DELETE to hit exactly one row.  Often, programmers
> won't bother to add code to check that it did ... but if a one-word
> addition to the command can provide such a check, it seems more likely
> that they would add the check.

and it can be used for optimization - it can be optimized for fast first row

Regards

Pavel

>
>                         regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Dec 21, 2012 at 4:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> That was my first reaction too, but Marko's followon examples seem to
> make a reasonable case for it.  There are many situations where you
> expect an UPDATE or DELETE to hit exactly one row.  Often, programmers
> won't bother to add code to check that it did ... but if a one-word
> addition to the command can provide such a check, it seems more likely
> that they would add the check.

Very true.

When I was a PL/PgSQL beginner a few years ago I did exactly that, I
didn't check if the update actually updated any row, I didn't know it
could fail, and felt extremely worried and stupid when I realised
this. I spent an entire day going through all functions fixing this
problem at all places. The fix was not beautiful and it bugged me
there was not a prettier way to fix it.

On Fri, 21 Dec 2012 16:14:19 +0100, I wrote:
> I wrote a patch
> which allows you to add STRICT into PERFORM and INSERT/UPDATE/DELETE
> without specifying an INTO clause.

Here's the second version of the patch, addressing the syntax issues.  I
also couldn't make the grammar work with PERFORM STRICT, so I allowed
STRICT SELECT  instead.

Any feedback welcome.


Regards,
Marko Tiikkaja

On 1/26/13 11:28 AM, Marko Tiikkaja wrote:
> On Fri, 21 Dec 2012 16:14:19 +0100, I wrote:
>> I wrote a patch
>> which allows you to add STRICT into PERFORM and INSERT/UPDATE/DELETE
>> without specifying an INTO clause.
> 
> Here's the second version of the patch, addressing the syntax issues.

I think the new syntax is horribly ugly.  The actual command name should
always come first, not options.  What will happen if people add more
options along this line?

> I also couldn't make the grammar work with PERFORM STRICT, so I allowed 
> STRICT SELECT  instead.

I don't quite understand the reason for distinguishing PERFORM and
SELECT, but what you are proposing will make this even more confusing.


That said, I don't quite believe in the premise for this patch to begin
with.  The supposed analogy is with INTO STRICT.  But that is
effectively a variable assignment and checks whether that assignment was
performed correctly.  So for scalar variables, this checks that exactly
one value was returned.  I'd imagine if we allowed a syntax like ...
INTO (a, b, c), (d, e, f) it would check that exactly two rows were
returned.  So this clause basically just ensures that the run-time
behavior is consistent with the appearance of the source code.

What you are now proposing is that STRICT means "one", but that's just
an opinion.  The SQL standard recognizes that updating or deleting
nothing is a noteworthy condition, so I could get partially behind this
patch if it just errored out when zero rows are affected, but insisting
on one is just arbitrary.

(Note also that elsewhere STRICT means something like "not null", so the
term is getting a bit overloaded.)

On Fri, 01 Feb 2013 18:11:13 +0100, Peter Eisentraut <peter_e@gmx.net>  
wrote:
> On 1/26/13 11:28 AM, Marko Tiikkaja wrote:
>> Here's the second version of the patch, addressing the syntax issues.
>
> I think the new syntax is horribly ugly.  The actual command name should
> always come first, not options.  What will happen if people add more
> options along this line?

WITH foo AS (..) SELECT ..;  doesn't have the command first either.

I don't really see what other plpgsql-specific options we would add..

>> I also couldn't make the grammar work with PERFORM STRICT, so I allowed
>> STRICT SELECT  instead.
>
> I don't quite understand the reason for distinguishing PERFORM and
> SELECT, but what you are proposing will make this even more confusing.
>
>
> That said, I don't quite believe in the premise for this patch to begin
> with.  The supposed analogy is with INTO STRICT.  But that is
> effectively a variable assignment and checks whether that assignment was
> performed correctly.  So for scalar variables, this checks that exactly
> one value was returned.  I'd imagine if we allowed a syntax like ...
> INTO (a, b, c), (d, e, f) it would check that exactly two rows were
> returned.  So this clause basically just ensures that the run-time
> behavior is consistent with the appearance of the source code.

Fine, I can see why you see it that way.

> What you are now proposing is that STRICT means "one", but that's just
> an opinion.  The SQL standard recognizes that updating or deleting
> nothing is a noteworthy condition, so I could get partially behind this
> patch if it just errored out when zero rows are affected, but insisting
> on one is just arbitrary.

*shrug*

To me, this makes the most sense.  In my experience if you know something  
should be there, it's exactly one row and not "one or more".  Not throwing  
an error on "more than one" would make this feature a lot less useful in  
practice.



Regards,
Marko Tiikkaja

Hi,

If I'm counting correctly, we have four votes for this patch and two votes  
against it.

Any other opinions?


Regards,
Marko Tiikkaja

"Marko Tiikkaja" <marko@joh.to> writes:
> If I'm counting correctly, we have four votes for this patch and two votes  
> against it.

> Any other opinions?

FWIW, I share Peter's poor opinion of this syntax.  I can see the
appeal of not having to write an explicit check of the rowcount
afterwards, but that appeal is greatly weakened by the strange syntax.
(IOW, if you were counting me as a + vote, that was only a vote for
the concept --- on reflection I don't much like this implementation.)
        regards, tom lane

> FWIW, I share Peter's poor opinion of this syntax.  I can see the
> appeal of not having to write an explicit check of the rowcount
> afterwards, but that appeal is greatly weakened by the strange syntax.
> (IOW, if you were counting me as a + vote, that was only a vote for
> the concept --- on reflection I don't much like this implementation.)

I agree with other commentators that it would be useful, but that the
word STRICT should be near the word INTO, making it clear that the
STRICTness is tied to the variable assignment.

I do think we can deal with the "more than one" case once PL/pgSQL INTO
actually supports assigning more than one row to a variable, which
currently it doesn't.  At that time, we'll just have to remember to
update the code for STRICT as well.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com