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