Thread: BUG #13805: plpgsql execute using expression evaluate wrong

The following bug has been logged on the website:

Bug reference:      13805
Logged by:          Jov
Email address:      amutu@amutu.com
PostgreSQL version: 9.4.5
Operating system:   CentOS 6
Description:

test case：

CREATE OR REPLACE FUNCTION public.test()
RETURNS void
LANGUAGE plpgsql
AS $function$
declare
n int = 4;
rt timestamp[];
rt2 interval;
begin
execute $$select array_agg(g.i) from generate_series(now() - ('$1
month')::interval,now(),'1 month') g(i)$$ into r
t using n;
raise notice 'rt : %',rt;

execute $$select array_agg(g.i) from generate_series(now() - ($1::varchar ||
' month')::interval,now(),'1 month')
g(i)$$ into rt using n;
raise notice 'rt : %',rt;

execute $$select ('$1 month')::interval $$ into rt2 using n;
raise notice 'rt2 : %',rt2;

execute $$select ($1::varchar || ' month')::interval $$ into rt2 using n;
raise notice 'rt2 : %',rt2;
end;

$function$
~
~
No changes
db_001=# select * from test();
NOTICE: rt : {"2015-11-08 11:46:28.500811","2015-12-08
11:46:28.500811"}-----wrong answer，should 4 elements
NOTICE: rt : {"2015-08-08 11:46:28.500811","2015-09-08
11:46:28.500811","2015-10-08 11:46:28.500811","2015-11-08
11:46:28.500811","2015-12-08 11:46:28.500811"}
NOTICE: rt2 : 1 mon-----wrong，should 4 mons
NOTICE: rt2 : 4 mons
test
------

(1 row)

On Tue, 08 Dec 2015 03:54:44 +0000
amutu@amutu.com wrote:


> execute $$select ('$1 month')::interval $$ into rt2 using n;
> raise notice 'rt2 : %',rt2;
>=20
> execute $$select ($1::varchar || ' month')::interval $$ into rt2 using n;
> raise notice 'rt2 : %',rt2;
> end;

> NOTICE: rt2 : 1 mon-----wrong=EF=BC=8Cshould 4 mons
> NOTICE: rt2 : 4 mons

You may have found a bug, but if so, it is not where you think it is. The
expression: '$1 month' is a text literal. PL/PGsql does not interpolate $n
like a shell, it only does the substitution where a variable could
ordinarily exist.

However, it does seem a little odd that the literal syntax for
intervals accepts the '$':

postgres=3D# select '$1 month'::interval, '$4 month'::interval;
 interval | interval=20
----------+----------
 1 mon    | 4 mons
(1 row)

I would have expected it to raise an error. The documentation does not shed
any light on this. Anyone?

-dg=20

--=20
David Gould              510 282 0869         daveg@sonic.net
If simplicity worked, the world would be overrun with insects.

Yes,I think this trigger an error is reasonable.It take me 2 hours to find
the problem,not friendly.

Jov
blog: http:amutu.com/blog <http://amutu.com/blog>

2015-12-08 14:08 GMT+08:00 David Gould <daveg@sonic.net>:

> On Tue, 08 Dec 2015 03:54:44 +0000
> amutu@amutu.com wrote:
>
>
> > execute $$select ('$1 month')::interval $$ into rt2 using n;
> > raise notice 'rt2 : %',rt2;
> >
> > execute $$select ($1::varchar || ' month')::interval $$ into rt2 using =
n;
> > raise notice 'rt2 : %',rt2;
> > end;
>
> > NOTICE: rt2 : 1 mon-----wrong=EF=BC=8Cshould 4 mons
> > NOTICE: rt2 : 4 mons
>
> You may have found a bug, but if so, it is not where you think it is. The
> expression: '$1 month' is a text literal. PL/PGsql does not interpolate $=
n
> like a shell, it only does the substitution where a variable could
> ordinarily exist.
>
> However, it does seem a little odd that the literal syntax for
> intervals accepts the '$':
>
> postgres=3D# select '$1 month'::interval, '$4 month'::interval;
>  interval | interval
> ----------+----------
>  1 mon    | 4 mons
> (1 row)
>
> I would have expected it to raise an error. The documentation does not sh=
ed
> any light on this. Anyone?
>
> -dg
>
> --
> David Gould              510 282 0869         daveg@sonic.net
> If simplicity worked, the world would be overrun with insects.
>

David Gould <daveg@sonic.net> writes:
> However, it does seem a little odd that the literal syntax for
> intervals accepts the '$':
> ...
> I would have expected it to raise an error. The documentation does not shed
> any light on this. Anyone?

The datetime input parser tends to consider most non-alphanumeric
characters as being insignificant except as field separators.  We could
tighten that up, but I think we should tread pretty carefully for fear of
breaking cases that used to work.  A trivial example:

regression=# select '1 mon 1 day'::interval;
  interval
-------------
 1 mon 1 day
(1 row)

regression=# select '1 mon, 1 day'::interval;
  interval
-------------
 1 mon 1 day
(1 row)

If we started to reject the second case, we'd likely get complaints.
But the parser doesn't see that as any different from

regression=# select '1 mon$ 1 day'::interval;
  interval
-------------
 1 mon 1 day
(1 row)

Trying to decide which characters are legitimate noise and which
aren't seems like a tarbaby best not to get stuck to :-(

            regards, tom lane

On Tue, 08 Dec 2015 01:36:39 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> The datetime input parser tends to consider most non-alphanumeric
> characters as being insignificant except as field separators.  We could
> tighten that up, but I think we should tread pretty carefully for fear of
> breaking cases that used to work.  A trivial example:
...
> regression=# select '1 mon$ 1 day'::interval;
>   interval
> -------------
>  1 mon 1 day
> (1 row)
>
> Trying to decide which characters are legitimate noise and which
> aren't seems like a tarbaby best not to get stuck to :-(

# select '#!1!#month,?=~&(12$[day*() '::interval;
   interval
---------------
 1 mon 12 days
(1 row)

I feel so ... dirty....

-dg

--
David Gould              510 282 0869         daveg@sonic.net
If simplicity worked, the world would be overrun with insects.

> On 08 Dec 2015, at 07:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>=20
> Trying to decide which characters are legitimate noise and which
> aren't seems like a tarbaby best not to get stuck to :-(

I'd like to argue that if you need PostgreSQL-native time from a string, and=
 want more control and verification, that's probably something that's best l=
eft to the application anyway.

I fail to see how PostgreSQL can reasonably be made to be "perfect" for all p=
ossible cases of dirty input being casted to interval.

The application is in a much better position to deal with this, presumably k=
nowing more about the input, and being a cleaner place to put any parsing an=
d handling.=20

=46rom there, it can be passed to PostgreSQL either as a native interval obj=
ect, or a cleaned up string.=20

In other words, I agree about not changing the behavior.=20

That said, I do wonder if it could be worthwhile to consider having an optio=
n to trigger a notice or error upon dirty bytes getting skipped, as a way of=
 declaring "all input should be clean (now), please accept nothing less". Th=
e again, that certainly falls outside the scope of -bugs@.=20

Terje=