Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Sync our copy of the timezone library with IANA tzcode master.

  2. Fix bool/int type confusion

  1. pgsql: Fix bool/int type confusion

    Peter Eisentraut <peter_e@gmx.net> — 2017-09-14T16:09:23Z

    Fix bool/int type confusion

Using ++ on a bool variable doesn't work well when stdbool.h is in use.
The original BSD code appears to use int here, so use that instead.

Reviewed-by: Thomas Munro <thomas.munro@enterprisedb.com>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/0ec2e908babfbfde83a3925680f06b16408739ff

Modified Files
--------------
src/timezone/localtime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

  2. Re: [COMMITTERS] pgsql: Fix bool/int type confusion

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-21T15:46:40Z

    Peter Eisentraut <peter_e@gmx.net> writes:
> Fix bool/int type confusion
> Using ++ on a bool variable doesn't work well when stdbool.h is in use.
> The original BSD code appears to use int here, so use that instead.

I'm fairly unhappy with this approach to fixing this problem, because
localtime.c is not Postgres-maintained code; we try to keep it in sync
with the upstream copy maintained by the IANA timezone crew.  Patching it
like this will interfere with the next sync attempt, and patching it only
in master will cause back-patching of that sync patch to fail.

I checked the tz list archives and discovered that this problem was
already reported to them back in May:
http://mm.icann.org/pipermail/tz/2017-May/024995.html
Although indeed their previous coding had had the variable as "int",
their response was not to change it back to int, but to get rid
of the loop incrementing it, because that was intended to support
the possibility of 2 leap seconds on the same day, which according
to them is a widespread misunderstanding of the applicable standard.
So the code in their git repo still has the variable as bool, but
there's no ++ operator on it anymore.

This also means that the portability problem is purely hypothetical;
given valid tz reference data the code wouldn't ever try to increment
"hit" to 2 anyway.  It's even more hypothetical for us, because we don't
use leap-second-aware zones.

They've not made a new tzcode release since May, due to lack of political
activity in this sphere this year, although I gather that one is likely
by November or so.  However, we have precedent for sometimes syncing with
their git tip to absorb code bug fixes, cf commit 7afafe8af.

Therefore, what I would like to do is revert this commit (0ec2e908b),
and then either leave the problem to be fixed by our next regular sync
with a released tzcode version, or else force a sync with their git tip
to absorb their fix now.  In either case we should keep all our live
branches in sync.  I'm willing to do the code-sync work either way.

Comments?

			regards, tom lane

  3. Re: [COMMITTERS] pgsql: Fix bool/int type confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-09-21T18:23:27Z

    On 9/21/17 11:46, Tom Lane wrote:
> So the code in their git repo still has the variable as bool, but
> there's no ++ operator on it anymore.
> 
> This also means that the portability problem is purely hypothetical;
> given valid tz reference data the code wouldn't ever try to increment
> "hit" to 2 anyway.  It's even more hypothetical for us, because we don't
> use leap-second-aware zones.

It's not so much that there is an actual portability problem.  The
problem is that the compiler issues a warning for this with stdbool.

> Therefore, what I would like to do is revert this commit (0ec2e908b),
> and then either leave the problem to be fixed by our next regular sync
> with a released tzcode version, or else force a sync with their git tip
> to absorb their fix now.  In either case we should keep all our live
> branches in sync.  I'm willing to do the code-sync work either way.

That makes sense.  There is no urgency on the stdbool patch set, so
waiting for this to be resolved properly around November seems reasonable.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: [COMMITTERS] pgsql: Fix bool/int type confusion

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-21T18:58:09Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/21/17 11:46, Tom Lane wrote:
>> This also means that the portability problem is purely hypothetical;
>> given valid tz reference data the code wouldn't ever try to increment
>> "hit" to 2 anyway.  It's even more hypothetical for us, because we don't
>> use leap-second-aware zones.

> It's not so much that there is an actual portability problem.  The
> problem is that the compiler issues a warning for this with stdbool.

Understood.

>> Therefore, what I would like to do is revert this commit (0ec2e908b),
>> and then either leave the problem to be fixed by our next regular sync
>> with a released tzcode version, or else force a sync with their git tip
>> to absorb their fix now.  In either case we should keep all our live
>> branches in sync.  I'm willing to do the code-sync work either way.

> That makes sense.  There is no urgency on the stdbool patch set, so
> waiting for this to be resolved properly around November seems reasonable.

I've just been going through their git commit log to see what else has
changed since tzcode2017b, and I note that there are half a dozen other
portability-ish fixes.  I think that some of them affect only code we
don't use, but I'm not sure that that's the case for all.  So I'm a bit
inclined to go with plan B, that is sync with their current HEAD now.
As far as I've been able to tell, they don't have any special code QA
process that they apply before a release.  They push out releases based
on the need to update the tzdata files, and the tzcode just rides along
with that in whatever state it is in.  So there's not really any
reliability gain from waiting for an official code release --- it just
is a bit easier to document what we synced to.  That being the case,
absorbing their changes in smaller chunks rather than bigger ones seems
easier.  I don't have the sync process totally automated, but it's not
that painful as long as there are not too many changes at once.

			regards, tom lane

  5. Re: [COMMITTERS] pgsql: Fix bool/int type confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-09-22T12:56:39Z

    On 9/21/17 14:58, Tom Lane wrote:
> I've just been going through their git commit log to see what else has
> changed since tzcode2017b, and I note that there are half a dozen other
> portability-ish fixes.  I think that some of them affect only code we
> don't use, but I'm not sure that that's the case for all.  So I'm a bit
> inclined to go with plan B, that is sync with their current HEAD now.

I suppose it might be good to do this after 10.0 final is wrapped?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: [COMMITTERS] pgsql: Fix bool/int type confusion

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-22T13:33:36Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/21/17 14:58, Tom Lane wrote:
>> I've just been going through their git commit log to see what else has
>> changed since tzcode2017b, and I note that there are half a dozen other
>> portability-ish fixes.  I think that some of them affect only code we
>> don't use, but I'm not sure that that's the case for all.  So I'm a bit
>> inclined to go with plan B, that is sync with their current HEAD now.

> I suppose it might be good to do this after 10.0 final is wrapped?

I went and did it already.  I'm not particularly worried about the
impending 10.0 wrap --- the changes are minor as such things go,
and we've generally not worried about giving previous tzcode syncs
more than a few days' buildfarm testing before shipping them.

			regards, tom lane