Re: [sqlsmith] Crash on GUC serialization

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Sat, Nov 19, 2016 at 9:31 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Thanks for the report! Looks like the serialization code has overlooked
>> the fact that string-valued GUCs can be NULL. Surprising we didn't
>> find that before ...

> I was half-way through it when you sent your email. It seems to me
> that we need to take care only of the case for PGC_STRING, per the
> attached.

Meh; that might be stopping the crash for you but you didn't consider what
to do in serialize_variable. Some machines would dump core there instead,
and others would print "(null)" which wouldn't fit in the allocated space.

The bigger problem here is that set_config_option cannot be used to set
a string GUC's value to NULL --- it will treat value == NULL as a request
to set to the reset_val. The best we can do is to convert the value
into an empty string, which isn't quite the same thing, though it may be
close enough (compare e45e990e4).

I'm also pretty unexcited about serializing float variables this way ---
assuming that sprintf("%.17g") will reconstruct doubles exactly is
just asking for trouble IMO.

So I can't escape the itchy feeling that this entire chunk of code
needs to be thrown away and rewritten differently. But it'd take
some refactoring of the code under set_config_option to do it nicely,
which is more work than we probably want to put in now.

regards, tom lane