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 <andres@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
