Re: plpgsql versus domains

I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Thu, Feb 26, 2015 at 1:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> To some extent this is a workaround for the fact that plpgsql does type
>>> conversions the way it does (ie, by I/O rather than by using the parser's
>>> cast mechanisms).  We've talked about changing that, but people seem to
>>> be afraid of the compatibility consequences, and I'm not planning to fight
>>> two compatibility battles concurrently ;-)

>> A sensible plan, but since we're here:

>> - I do agree that changing the way PL/pgsql does those type
>> conversions is scary.  I can't predict what will break, and there's no
>> getting around the scariness of that.

>> - On the other hand, I do think it would be good to change it, because
>> this sucks:

>> rhaas=# do $$ declare x int; begin x := (3.0::numeric)/(1.0::numeric); end $$;
>> ERROR:  invalid input syntax for integer: "3.0000000000000000"
>> CONTEXT:  PL/pgSQL function inline_code_block line 1 at assignment

> If we did want to open that can of worms, my proposal would be to make
> plpgsql type conversions work like so:
> * If there is a cast pathway known to the core SQL parser, use that
>   mechanism.
> * Otherwise, attempt to convert via I/O as we do today.
> This seems to minimize the risk of breaking things, although there would
> probably be corner cases that work differently (for instance I bet boolean
> to text might turn out differently).  In the very long run we could perhaps
> deprecate and remove the second phase.

Since people didn't seem to be running away screaming, here is a patch to
do that.  I've gone through the list of existing casts, and as far as I
can tell the only change in behavior in cases that would have worked
before is the aforementioned boolean->string case.  Before, if you made
plpgsql convert a bool to text, you got 't' or 'f', eg

regression=# do $$declare t text; begin t := true; raise notice 't = %', t; end $$;
NOTICE:  t = t
DO

Now you get 'true' or 'false', because that's what the cast does, eg

regression=# do $$declare t text; begin t := true; raise notice 't = %', t; end $$;
NOTICE:  t = true
DO

This seems to me to be a narrow enough behavioral change that we could
tolerate it in a major release.  (Of course, maybe somebody out there
thinks that failures like the one Robert exhibits are a feature, in
which case they'd have a rather longer list of compatibility issues.)

The performance gain is fairly nifty.  I tested int->bigint coercions
like this:

do $$
declare b bigint;
begin
  for i in 1 .. 1000000 loop
    b := i;
  end loop;
end$$;

On my machine, in non-cassert builds, this takes around 750 ms in 9.4,
610 ms in HEAD (so we already did something good, I'm not quite sure
what), and 420 ms with this patch.  Another example is a no-op domain
conversion:

create domain di int;

do $$
declare b di;
begin
  for i in 1 .. 1000000 loop
    b := i;
  end loop;
end$$;

This takes about 760 ms in 9.4, 660 ms in HEAD, 380 ms with patch.

Comments?

                        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