Hi, I've a patch making .data variables constant, fwiw. Will post in a bit. Just so we don't duplicate work too much.
On 2018-10-15 19:41:49 -0400, Tom Lane wrote: > I wrote: > > Andres Freund <and...@anarazel.de> writes: > >> 0000000008560224 0000000000023440 b tzdefrules_s > >> 0000000008536704 0000000000023440 b gmtmem.7009 > > > I think that tzdefrules_s is not used in common cases (though I could be > > wrong about that), so we could win by alloc-on-first-use. The same might > > be true for gmtmem, but there's a sticking point: there is no provision > > for failure there, so I'm unsure how we avoid crashing on OOM. > > So my intuition was exactly backwards on this. > > 1. tzdefrules_s is filled in the postmaster, and if it's not filled there > it'd be filled by every child process, so there's zero point in converting > it to malloc-on-the-fly style. This is because tzparse() always wants > it filled. That's actually a tad annoying, because it looks to me like > with many timezone settings the data will not get used, but I'm hesitant > to mess with the IANA code's logic enough to improve that. Maybe I'll try > submitting an upstream patch and see what they think of it first. Ok, that makes sense. > 2. gmtmem, on the other hand, is only used if somebody calls pg_gmtime, > which already has a perfectly good error-report convention, cf gmtime(3). > We have exactly one caller, which was not bothering to test for error :-( > and would dump core on failure. And that caller isn't reached in most > processes, at least not in the regression tests. So this side of it is > easy to improve. > > Hence I propose the attached patch for point 2, which I'd want to > backpatch to keep our copies of the IANA code in sync. That makes sense, too. Greetings, Andres Freund
