From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org>, 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-06 21:34:54 |
Message-ID: | b75a627c-71f0-449d-b58a-2a7544f8c643@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Peter,
I took a look at this patch today. I think it makes sense to do this,
and I haven't found any major issues with any of the three patches. A
couple minor comments:
0001
----
1) I think this bit in ALTER STATISTICS docs is wrong:
- <term><replaceable class="parameter">new_target</replaceable></term>
+ <term><literal>SET STATISTICS { <replaceable
class="parameter">integer</replaceable> | DEFAULT }</literal></term>
because it means we now have list entries for name, ..., new_name,
new_schema, and then suddenly "SET STATISTICS { integer | DEFAULT }".
That's a bit weird.
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?
3) I find it a bit tedious that making the stxstattarget field nullable
means we now have to translate NULL to -1 in so many places. I know why
it's that way, but it's ... not very convenient :-(
0002
----
1) I think InsertPgAttributeTuples comment probably needs to document
what the new tupdesc_extra parameter does.
0003
----
no comment, seems fine
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Matthias van de Meent | 2024-03-06 21:46:00 | Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan |
Previous Message | Tomas Vondra | 2024-03-06 20:58:48 | Re: Shared detoast Datum proposal |