On 10/26/14, 3:46 PM, Tom Lane wrote:
> I wrote:
>>> More generally, it seems like we ought to have a test in the type_sanity
>>> regression script that checks that type I/O functions aren't volatile,
>>> ...
>
>> Actually, the right thing to do if we want to enforce this is for
>> CREATE TYPE to check the marking. We'd still need a type_sanity
>> test to check erroneous declarations of built-in types, but a complaint
>> in CREATE TYPE would cover all the contrib modules as well as third-party
>> code.
>
>> I wouldn't propose back-patching such an error check, but it seems
>> reasonable to add it for 9.5. Any objections?
>
> On the way to fixing this, I was unpleasantly surprised to discover that
> we have one I/O function in contrib that *actually is* volatile, and not
> just thoughtlessly marked that way. That's chkpass_in(), which is
> volatile because it goes to the trouble of using random() to produce a
> new password salt value on every call.
>
> Not entirely sure what to do about this. It seems like for the purposes
> of contrib/chkpass, it's a good thing that chkpass_in() won't reuse the
> same salt. Weak as a 12-bit salt might be nowadays, it's still better
> than no salt. Nonetheless, this behavior is breaking assumptions made
> in places like array_in and record_in.
>
> For the moment I'm tempted to mark chkpass_in as stable (with a comment
> explaining that it isn't really) just so we can put in the error check
> in CREATE TYPE. But I wonder if anyone has a better idea.
The check could explicitly ignore chkpass_in. That's pretty grotty, but perhaps less so than marking it stable when it's not...
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com