FAQ
Applied.

On Fri, 16 Jun 2000, Ryan Kirkpatrick wrote:

I will download the snapshot today at work (where I have "real"
bandwidth :) and test things out this weekend. I should have a report by
Monday. Maybe I will even have a patch that can be safely applied to
the development source tree. :) TTYL.
Ok, I have tested the new snapshot. I have good news and I have
bad news...
First of all, to build pgsql correctly from the snapshot tarball
(dated Fri, Jun 16th), I had to run a 'make distclean' first. There were
some config.status files laying around that were confusing the configure
run. Don't know if that is just par for pgsql devel snapshots or if there
was mistake somewhere in the building of the snapshot.
Once I got past that, I realized that the only non-fmgr related
Linux/Alpha patch was a adding a single line to the linux_alpha template
file. If I had realized that earlier, I would have submitted that patch
--
Bruce Momjian | http://www.op.net/~candle
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Search Discussions

  • Tom Lane at Jun 19, 2000 at 5:17 am

    Ryan Kirkpatrick writes:
    First of all, to build pgsql correctly from the snapshot tarball
    (dated Fri, Jun 16th), I had to run a 'make distclean' first. There were
    some config.status files laying around that were confusing the configure
    run.
    That's odd, the snapshot is supposed to have been made from a
    distcleaned directory tree. Maybe something busted in the snapshot
    generation script? Marc?
    I tracked the problem down to the following suspect functions. I
    found these as they were functions listed in the Linux/Alpha patches (as
    needing Datum datatypes for function params), but did not appear to have
    been rewritten from the new fmgr (i.e. no PG_FUNCTION_ARGS in
    declaration). These functions are all in src/backend/utils/adt/nabstime.c.
    abstime2tm
    tm2abstime
    AbsoluteTimeIsBefore
    AbsoluteTimeIsAfter
    reltime2tm
    If these are converted to the new fmgr format, then I think the regression
    tests mentioned above should pass on Linux/Alpha.
    Hmm. I did not touch these because they aren't fmgr-callable (and in
    fact I feel fairly safe in saying that AbsoluteTimeIsAfter isn't called
    period, since it's ifdef'd out...). As best I can tell, the other four
    are only called from places that see valid prototypes for them, so if
    your compiler is failing to call them correctly then your compiler is
    broken.

    I suspect that the real problem is not call sequences, but something
    else that happens to be affecting these routines (and maybe related code
    that doesn't get exercised by the regression tests). Maybe something
    like macro-constants that are declared with not quite the right type
    (unsigned or not, long or not) and need to be casted to match whatever
    they're being compared to. We've seen that before.

    Could you dig a little more and try to identify exactly what's
    going wrong?

    Anyway, it sure sounds like we've broken the back of the problems.
    Couple more bug fixes and we'll be there. That's good news indeed!

    regards, tom lane
  • Ryan Kirkpatrick at Jun 20, 2000 at 11:15 pm

    On Mon, 19 Jun 2000, Tom Lane wrote:

    declaration). These functions are all in src/backend/utils/adt/nabstime.c.
    abstime2tm
    tm2abstime
    AbsoluteTimeIsBefore
    AbsoluteTimeIsAfter
    reltime2tm
    Hmm. I did not touch these because they aren't fmgr-callable (and in ...
    I suspect that the real problem is not call sequences, but something
    else that happens to be affecting these routines (and maybe related code ...
    they're being compared to. We've seen that before.
    Could you dig a little more and try to identify exactly what's
    going wrong?
    Will do. I ran out of time last weekend to actually test if these
    functions were the cause of the problem or not. They just looked suspcious
    given the patches I had. I will did deeper and see what I can find, but it
    will probably not happen until next weekend. Will post when I have found
    something.
    Anyway, it sure sounds like we've broken the back of the problems.
    Couple more bug fixes and we'll be there. That's good news indeed!
    Yes, very good news indeed! :)

    ---------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ---------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | http://www.rkirkpat.net/ |
    ---------------------------------------------------------------------------
  • Tatsuo Ishii at Jun 25, 2000 at 2:41 am

    On Mon, 19 Jun 2000, Tom Lane wrote:
    declaration). These functions are all in src/backend/utils/adt/nabstime.c.
    abstime2tm
    tm2abstime
    AbsoluteTimeIsBefore
    AbsoluteTimeIsAfter
    reltime2tm
    Hmm. I did not touch these because they aren't fmgr-callable (and in ...
    I suspect that the real problem is not call sequences, but something
    else that happens to be affecting these routines (and maybe related code ...
    they're being compared to. We've seen that before.
    Could you dig a little more and try to identify exactly what's
    going wrong?
    Will do. I ran out of time last weekend to actually test if these
    functions were the cause of the problem or not. They just looked suspcious
    given the patches I had. I will did deeper and see what I can find, but it
    will probably not happen until next weekend. Will post when I have found
    something.
    Tamotsu Nakagawa has posted a fix for this to a local mail list in
    Japan. Can someone comment on this? According to him, with the patch
    now only the geometry test fails.

    void
    -abstime2tm(AbsoluteTime time, int *tzp, struct tm * tm, char *tzn)
    +abstime2tm(AbsoluteTime _time, int *tzp, struct tm * tm, char *tzn)
    {
    + time_t time = (time_t) _time;
    #ifdef USE_POSIX_TIME
    struct tm *tx;
  • Tom Lane at Jun 25, 2000 at 4:16 am

    Tatsuo Ishii writes:
    Tamotsu Nakagawa has posted a fix for this to a local mail list in
    Japan. Can someone comment on this? According to him, with the patch
    now only the geometry test fails.
    void
    -abstime2tm(AbsoluteTime time, int *tzp, struct tm * tm, char *tzn)
    +abstime2tm(AbsoluteTime _time, int *tzp, struct tm * tm, char *tzn)
    {
    + time_t time = (time_t) _time;
    #ifdef USE_POSIX_TIME
    struct tm *tx;
    Hmm, that makes all kinds of sense if time_t is not the same size as
    AbsoluteTime --- which wouldn't surprise me at all on a 64-bit system.
    time_t *ought* to be 64-bits on such a machine. The casts in that
    routine,
    tx = localtime((time_t *) &time);
    are obviously bogus if so. Can anyone with an Alpha comment?

    What surprises me more is the implication that this is the only place
    that makes such a bogus assumption about the size of time_t. I'd have
    guessed there are more places...

    regards, tom lane
  • Thomas Lockhart at Jun 25, 2000 at 4:34 am

    Hmm, that makes all kinds of sense if time_t is not the same size as
    AbsoluteTime --- which wouldn't surprise me at all on a 64-bit system.
    time_t *ought* to be 64-bits on such a machine. The casts in that
    routine,
    tx = localtime((time_t *) &time);
    are obviously bogus if so. Can anyone with an Alpha comment?
    I haven't had an Alpha for a couple of years, but I *strongly* recall
    that time_t is 64 bits on that machine.

    - Thomas
  • Adriaan Joubert at Jun 25, 2000 at 7:22 am

    Thomas Lockhart wrote:

    Hmm, that makes all kinds of sense if time_t is not the same size as
    AbsoluteTime --- which wouldn't surprise me at all on a 64-bit system.
    time_t *ought* to be 64-bits on such a machine. The casts in that
    routine,
    tx = localtime((time_t *) &time);
    are obviously bogus if so. Can anyone with an Alpha comment?
    I haven't had an Alpha for a couple of years, but I *strongly* recall
    that time_t is 64 bits on that machine.
    In <sys/types.h> time_t is defined as an int4, i.e. 4 bytes. To
    double-check I wrote a program to print sizeof:

    sizeof(time_t)=4 (DU 4.0F, cc)

    So I guess it is 32 bits. On the whole they have stuck to traditional sizes
    for traditional types -- it would just have broken too many programmes
    otherwise. Of course they are going to have to make time_t 64 bits within
    the next 30 years ....

    Adriaan
  • Ryan Kirkpatrick at Jun 25, 2000 at 3:32 pm

    On Sun, 25 Jun 2000, Tom Lane wrote:

    Tatsuo Ishii <t-ishii@sra.co.jp> writes:
    Tamotsu Nakagawa has posted a fix for this to a local mail list in
    Japan. Can someone comment on this? According to him, with the patch
    now only the geometry test fails.
    void
    -abstime2tm(AbsoluteTime time, int *tzp, struct tm * tm, char *tzn)
    +abstime2tm(AbsoluteTime _time, int *tzp, struct tm * tm, char *tzn)
    {
    + time_t time = (time_t) _time;
    #ifdef USE_POSIX_TIME
    struct tm *tx;
    Hmm, that makes all kinds of sense if time_t is not the same size as
    AbsoluteTime --- which wouldn't surprise me at all on a 64-bit system.
    time_t *ought* to be 64-bits on such a machine. The casts in that
    routine,
    tx = localtime((time_t *) &time);
    are obviously bogus if so. Can anyone with an Alpha comment?
    On Linux/Alpha, time_t is indeed 64 bit (i.e.
    printf("%d\n",sizeof(time_t)) results in an '8'). If AbsoluteTime is only
    32 bit, then that would definitely cause a problem.
    What surprises me more is the implication that this is the only place
    that makes such a bogus assumption about the size of time_t. I'd have
    guessed there are more places...
    I will apply the above patch, check for other instances of time_t
    size assumptions, and see what the result is this afternoon. TTYL.

    ---------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ---------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | http://www.rkirkpat.net/ |
    ---------------------------------------------------------------------------
  • Ryan Kirkpatrick at Jun 25, 2000 at 10:09 pm

    void
    -abstime2tm(AbsoluteTime time, int *tzp, struct tm * tm, char *tzn)
    +abstime2tm(AbsoluteTime _time, int *tzp, struct tm * tm, char *tzn)
    {
    + time_t time = (time_t) _time;
    #ifdef USE_POSIX_TIME
    struct tm *tx;
    Ok, the above patch does indeed solve the problem. And this
    appears to be the only place AbsoluteTime needs to be copied to a time_t
    variable. I can't find any other casts of AbsoluteTime to time_t, and
    with this patch applied all regression tests pass just fine (save for
    geometry of course with its standard off by one in nth decimal place
    difference).
    Additionally, I do not see how this patch could break other
    platforms. At worst, it is a minor slow down that might even be optimized
    out by some compiliers when they see that sizeof(AbsoluteTime) ==
    sizeof(time_t). I will defer to the core developers on how you want to
    apply this patch to the source tree (i.e. with #ifdef alpha && linux or as
    above). Though probably best to add a bit of a comment beside it so
    someone does not remove it later thinking they are "optimizing" the code.
    :)
    At this point, Linux/Alpha should actually run out of the box! Let
    me know when this patch is applied (in what ever form it ends up as) and I
    will download a new snapshot and test it.
    TTYL.

    ---------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ---------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | http://www.rkirkpat.net/ |
    ---------------------------------------------------------------------------
  • Tom Lane at Jun 26, 2000 at 3:32 am

    Ryan Kirkpatrick writes:
    Ok, the above patch does indeed solve the problem. And this
    appears to be the only place AbsoluteTime needs to be copied to a time_t
    variable. I can't find any other casts of AbsoluteTime to time_t, Great!
    and
    with this patch applied all regression tests pass just fine (save for
    geometry of course with its standard off by one in nth decimal place
    difference).
    Probably we should write that off as a platform issue and create an
    Alpha-specific expected-output file for geometry. See the documentation
    about platform-specific files, and please send along a patch to add one.
    Additionally, I do not see how this patch could break other
    platforms. At worst, it is a minor slow down that might even be optimized
    out by some compiliers when they see that sizeof(AbsoluteTime) ==
    sizeof(time_t). I will defer to the core developers on how you want to
    apply this patch to the source tree (i.e. with #ifdef alpha && linux or as
    above).
    No, we should just apply it as is, no #ifdef. There are going to be
    more and more platforms with 64-bit time_t.

    regards, tom lane
  • Peter Eisentraut at Jun 26, 2000 at 12:29 pm

    On Sun, 25 Jun 2000, Tom Lane wrote:

    Probably we should write that off as a platform issue and create an
    Alpha-specific expected-output file for geometry.
    I noticed the other day that the geometry output differs with the
    compiler optimization level. That can't be good.


    --
    Peter Eisentraut Sernanders väg 10:115
    peter_e@gmx.net 75262 Uppsala
    http://yi.org/peter-e/ Sweden
  • Ryan Kirkpatrick at Jun 27, 2000 at 12:36 am

    On Sun, 25 Jun 2000, Tom Lane wrote:

    with this patch applied all regression tests pass just fine (save for
    geometry of course with its standard off by one in nth decimal place
    difference).
    Probably we should write that off as a platform issue and create an
    Alpha-specific expected-output file for geometry. See the documentation
    about platform-specific files, and please send along a patch to add one.
    I remember finding a geometry.out file in the expected directory
    that did match. Seems to me it was a Solaris/Sun one.... Anyway, I will
    look into it and generate a patch to rid ourselves of that annoyance.
    Probably will be next Sunday before it happens.
    As for the comments about different geometry outputs with
    different optimization levels, the linux_alpha template is set to use -O2,
    which seems pretty standard for safe, yet useful optimization. Though I
    have had reports that newer versions of gcc (from Mandrake 7.1) break
    pgsql (spinlocks get stuck) with that optimization level. My alpha is
    still running Debian 2.1 and so my gcc is a little old. Probably will
    upgrade before the end of the summer and have to deal with that issue
    then. Yuck. :( Of course, if some one wants to beat me to it, feel free :)
    Additionally, I do not see how this patch could break other
    platforms. At worst, it is a minor slow down that might even be optimized
    out by some compiliers when they see that sizeof(AbsoluteTime) ==
    sizeof(time_t). I will defer to the core developers on how you want to
    apply this patch to the source tree (i.e. with #ifdef alpha && linux or as
    above).
    No, we should just apply it as is, no #ifdef. There are going to be
    more and more platforms with 64-bit time_t.
    No problem here with that, go ahead and apply it.

    ---------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ---------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | http://www.rkirkpat.net/ |
    ---------------------------------------------------------------------------
  • Peter Eisentraut at Jun 26, 2000 at 1:35 am

    Tom Lane writes:

    What surprises me more is the implication that this is the only place
    that makes such a bogus assumption about the size of time_t. I'd have
    guessed there are more places...
    A lot of those were fixed for 7.0.

    --
    Peter Eisentraut Sernanders väg 10:115
    peter_e@gmx.net 75262 Uppsala
    http://yi.org/peter-e/ Sweden
  • Bruce Momjian at Jun 27, 2000 at 6:09 pm
    Applied as you suggested.

    On Mon, 19 Jun 2000, Tom Lane wrote:

    declaration). These functions are all in src/backend/utils/adt/nabstime.c.
    abstime2tm
    tm2abstime
    AbsoluteTimeIsBefore
    AbsoluteTimeIsAfter
    reltime2tm
    Hmm. I did not touch these because they aren't fmgr-callable (and in ...
    I suspect that the real problem is not call sequences, but something
    else that happens to be affecting these routines (and maybe related code ...
    they're being compared to. We've seen that before.
    Could you dig a little more and try to identify exactly what's
    going wrong?
    Will do. I ran out of time last weekend to actually test if these
    functions were the cause of the problem or not. They just looked suspcious
    given the patches I had. I will did deeper and see what I can find, but it
    will probably not happen until next weekend. Will post when I have found
    something.
    Tamotsu Nakagawa has posted a fix for this to a local mail list in
    Japan. Can someone comment on this? According to him, with the patch
    now only the geometry test fails.

    void
    -abstime2tm(AbsoluteTime time, int *tzp, struct tm * tm, char *tzn)
    +abstime2tm(AbsoluteTime _time, int *tzp, struct tm * tm, char *tzn)
    {
    + time_t time = (time_t) _time;
    #ifdef USE_POSIX_TIME
    struct tm *tx;

    --
    Bruce Momjian | http://www.op.net/~candle
    pgman@candle.pha.pa.us | (610) 853-3000
    + If your life is a hard drive, | 830 Blythe Avenue
    + Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-ports @
categoriespostgresql
postedJun 19, '00 at 12:51a
activeJun 27, '00 at 6:09p
posts14
users8
websitepostgresql.org
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase