From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Make attstattarget nullable |
Date: | 2024-03-17 12:51:39 |
Message-ID: | 645bc1a5-646c-4b24-9f4a-3e11bee96172@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 14.03.24 15:46, Tomas Vondra wrote:
>>>>> 2) The newtarget handling in AlterStatistics seems rather confusing.
>>>>> Why
>>>>> does it get set to -1 just to ignore the value later? For a while I was
>>>>> 99% sure ALTER STATISTICS ... SET STATISTICS DEFAULT will set the field
>>>>> to -1. Maybe ditching the first if block and directly checking
>>>>> stmt->stxstattarget before setting repl_val/repl_null would be better?
>>>>
>>>> But we also need to continue accepting -1 for default on input. The
>>>> current code achieves that, the proposed variant would not.
>>>
>>> OK, I did not realize that. But then maybe this should be explained in a
>>> comment before the new "if" block, because people won't realize why it
>>> needs to be this way.
>>
>> In the new version, I tried to write this more explicitly, and updated
>> tablecmds.c to match.
>
> WFM. It still seems a bit hard to read, but I don't know how to do it
> better. I guess it's how it has to be to deal with multiple default
> values in a backwards-compatible way. Good thing is it's localized in
> two places.
I have committed this patch series. Thanks.
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2024-03-17 13:07:42 | Re: Simplify backtrace_functions GUC code |
Previous Message | Christophe Pettus | 2024-03-17 10:33:52 | Re: Regression tests fail with musl libc because libpq.so can't be loaded |