> > I am not sure to get the point about some keys removed in the future. > First, that sounds very unlikely. Even if it were the case, isn't > pg_dump going to filter them out anyway? Forcing a stricter check in > the restore function to force a set of keys to exist still sounds like > a better option, as we have only backward-compatibility requirements > in pg_dump, not in the backend restore function at a fixed version. >
There's something that feels incongruous about holding these expressions pg_statistic rows to a higher level of correctness than we do for regular attribute stats, but it's hard to articulate and I'm not sure it actually matters. > > We still could get a situation where all > > the exprs are empty (i.e. [null,null,null]). There is no simple test to > map > > that to just a plain null, but even if there were the SQL is already > > drifting into "clever" territory and I know that pg_dump likes to keep > > things very simple. > > With a set of three expressions [null,null,null] is IMO a valid thing, > we have no valid data. Yeah, I'm content to let that one be as-is. > Another case is when we have a non-NULL > element with all the keys present, but all their values are NULL. To be clear, pg_dump would filter that out to a non-object NULL element, but nothing stops somebody from creating an object like that, and if they did they'd get the the unmodified statatt_get_empty_stat_tuple. > If > my test of the patch is right, the restore function accepts this case, > and decides to set the following fields: > "avg_width": "0", > "null_frac": "0", > "n_distinct": "0" > statatt_get_empty_stat_tuple is just assigning the defaults. > jbv_string_get_cstr() is IMO incorrect in deciding that, no? It > sounds to me that some specific NULL checks would be in order? > Any explicit null parameters would be handled by the loop that fills out the found[] and val[] arrays, and explicit jbvNull is treated the same as if the parameter wasn't specified in the first place. So I think the null checks you seek are already there, but the existence of defaults in the pg_statistic tuple (which has no nullable columns, btw) is throwing you off.
