Re: plpgsql.consistent_into

From: Jim Nasby <jim(at)nasby(dot)net>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Florian Pflug <fgp(at)phlo(dot)org>
Cc: Marko Tiikkaja <marko(at)joh(dot)to>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: plpgsql.consistent_into
Date: 2014-01-13 21:49:45
Message-ID: 52D45F79.5060107@nasby.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/13/14, 1:44 AM, Pavel Stehule wrote:
>
>
>
> 2014/1/12 Florian Pflug <fgp(at)phlo(dot)org <mailto:fgp(at)phlo(dot)org>>
>
> On Jan12, 2014, at 22:37 , Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com <mailto:pavel(dot)stehule(at)gmail(dot)com>> wrote:
> > There is GUC for variable_conflict already too. In this case I would to
> > enable this functionality everywhere (it is tool how to simply eliminate
> > some kind of strange bugs) so it needs a GUC.
> >
> > We have GUC for plpgsql.variable_conflict three years and I don't know
> > about any problem.
>
> I must say I hate behaviour-changing GUCs with quite some passion. IMHO
> they tend to cause bugs, not avoid them, in the long run. The pattern
> usually is
>
> 1) Code gets written, depends on some particular set of settings
> to work correctly
>
> 2) Code gets reused, with little further testing since it's supposed
> to be battle-proven anyway. Settings get dropped.
>
> 3) Code blows up for those corner-cases where the setting actually
> matter. Debugging is hell, because you effectively have to go
> over the code line-by-line and check if it might be affected by
> some GUC or another.
>
> Only a few days ago I spent more than an hour tracking down a bug
> which, as it turned out, was caused by a regex which subtly changed its
> meaning depending on whether standard_conforming_strings is on or off.
>
> Some GUCs are unavoidable - standard_conforming_strings, for example
> probably still was a good idea, since the alternative would have been
> to stick with the historical, non-standard behaviour forever.
>
> But in this case, my feeling is that the trouble such a GUC may cause
> out-weights the potential benefits. I'm all for having a directive like
> #consistent_into (though I feel that the name could convey the
> meaning better). If we *really* think that this ought to be the default
> from 9.4 onward, then we should
>
> *) Change it to always complain, except if the function explictly
> specifies "#consistent_into on" or whatever.
>
> *) Have pg_dump add that to all plpgsql functions if the server
> version is < 9.4 or whatever major release this ends up in
>
> That's all just my opinion of course.
>
>
> I am thinking so GUC and plpgsql option can live together. If you like to accent a some behave, then you can use a plpgsql option. On second hand, I would to use a some functionality, that is safe, but I don't would to dirty source code by using repeated options. But I have to check (and calculate with risk) a GUC settings.
>
> One idea: required GUC? Can be nice a possibility to ensure some GUC setting, and restore ensure these values or raises warning.
>
> Back to main topic. Required and described feature doesn't change a behave of INTO clause. I can enable or disable this functionality and well written code should to work without change (and problems). When check is disabled, then execution is just less safe. So in this case, a impact of GUC is significantly less than by you described issues. Does know anybody a use case where this check should be disabled?
>
> Probably we have a different experience about GUC. I had a problem with standard_conforming_strings and bytea format some years ago. Now I prepare document about required setting. But I can see (from my experience from Czech area) more often problems related to effective_cache_size or from_collapse_limit and similar GUC. These parameters are behind knowledge (and visibility) typical user.

ISTM that in this case, it should be safe to make the new default behavior STRICT; if you forget to set the GUC to disable than you'll get an error that points directly at the problem, at which point you'll go "Oh, yeah... I forgot to set X..."

Outside of the GUC, I believe the default should definitely be STRICT. If your app is relying on non-strict then you need to be made aware of that. We should be able to provide a DO block that will change this setting for every function you've got if someone isn't happy with STRICT mode.
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2014-01-13 21:55:42 Re: Disallow arrays with non-standard lower bounds
Previous Message David Fetter 2014-01-13 21:45:08 Re: Disallow arrays with non-standard lower bounds