FAQ
In the regression test rules.sql there is this SQL command

update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;

Which causes my alpha port to go core. The above line can be reduced to:

update rtest_v1 set a = rtest_t3.a + 20 ;

which also causes the same problem. It seems that the 64 bit address
((Expr*)nodeptr)->oper gets truncated ( high 32 bits ) somewhere along the way.

I was able to locate the errant code in rewriteManip.c:712. but There seems to be a
bigger problem other than eraseing the upper 32bit address. It seems that
FindMatchingNew() returns a node of type T_Expr, rather than the expected type of
T_Var. Once u realize this then u can see why the now MISCAST "(Var *)
*nodePtr)->varlevelsup = this_varlevelsup" will cause a problem. On my alpha this erases
a portion in the address in the T_Expr. On the redhat 5.2/i386 this code seems to be
benign, BUT YOU ARE ERASEING SOMETHING that doesn't belong to to T_Expr !

So what gives?
gat
Maybe an assert() will help in finding some of the miscast returned types? Wuddya think?
sure would catch some of the boo-boo's hanging around

rewriteManip.c:
if (this_varno == info->new_varno &&
this_varlevelsup == sublevels_up)
{
n = FindMatchingNew(targetlist,
((Var *) node)->varattno);
if (n == NULL)
{
if (info->event == CMD_UPDATE)
{
*nodePtr = n = copyObject(node);
((Var *) n)->varno = info->current_varno;
((Var *) n)->varnoold = info->current_varno;
}
else
*nodePtr = make_null(((Var *) node)->vartype);
}
else
{
*nodePtr = copyObject(n);
((Var *) *nodePtr)->varlevelsup = this_varlevelsup; /* This
line zaps the address */
}
}

Search Discussions

  • Tom Lane at Jul 20, 1999 at 2:15 am

    Uncle George writes:
    In the regression test rules.sql there is this SQL command
    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;
    Which causes my alpha port to go core.
    Yeah. This was reported by Pedro Lobo on 11 June, and we've been
    patiently waiting for Jan to decide what to do about it :-(

    You could stop the coredump by putting a test into ResolveNew:

    {
    *nodePtr = copyObject(n);
    + if (IsA(*nodePtr, Var))
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
    }

    but what's not so clear is what's supposed to happen when the
    replacement item *isn't* a Var. I tried to convince myself that nothing
    needed to happen in that case, but wasn't successful. (Presumably the
    replacement expression contains no instances of the variable being
    replaced, so recursing into it with ResolveNew shouldn't be needed
    --- but maybe its varlevelsup values need adjusted?)

    regards, tom lane
  • Bruce Momjian at Sep 23, 1999 at 10:02 pm
    This seems to be the detail on the bug report.

    Uncle George <gatgul@voicenet.com> writes:
    In the regression test rules.sql there is this SQL command
    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;
    Which causes my alpha port to go core.
    Yeah. This was reported by Pedro Lobo on 11 June, and we've been
    patiently waiting for Jan to decide what to do about it :-(

    You could stop the coredump by putting a test into ResolveNew:

    {
    *nodePtr = copyObject(n);
    + if (IsA(*nodePtr, Var))
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
    }

    but what's not so clear is what's supposed to happen when the
    replacement item *isn't* a Var. I tried to convince myself that nothing
    needed to happen in that case, but wasn't successful. (Presumably the
    replacement expression contains no instances of the variable being
    replaced, so recursing into it with ResolveNew shouldn't be needed
    --- but maybe its varlevelsup values need adjusted?)

    regards, tom lane

    --
    Bruce Momjian | http://www.op.net/~candle
    maillist@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
  • Bruce Momjian at Nov 29, 1999 at 10:49 pm
    Any ideas on this one?
    Uncle George <gatgul@voicenet.com> writes:
    In the regression test rules.sql there is this SQL command
    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;
    Which causes my alpha port to go core.
    Yeah. This was reported by Pedro Lobo on 11 June, and we've been
    patiently waiting for Jan to decide what to do about it :-(

    You could stop the coredump by putting a test into ResolveNew:

    {
    *nodePtr = copyObject(n);
    + if (IsA(*nodePtr, Var))
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
    }

    but what's not so clear is what's supposed to happen when the
    replacement item *isn't* a Var. I tried to convince myself that nothing
    needed to happen in that case, but wasn't successful. (Presumably the
    replacement expression contains no instances of the variable being
    replaced, so recursing into it with ResolveNew shouldn't be needed
    --- but maybe its varlevelsup values need adjusted?)

    regards, tom lane

    --
    Bruce Momjian | http://www.op.net/~candle
    maillist@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
  • Uncle George at Nov 30, 1999 at 12:57 am
    I'm sorry, the resultant node as returned is not expected. The solution, as
    provided, did stop the insidious erasure of a field in a structure it did not own.
    I'm content ( but i dont know any better )
    If ur asking me what one is suppose to do at this point - I dunno.
    gat

    Bruce Momjian wrote:
    Any ideas on this one?
    Uncle George <gatgul@voicenet.com> writes:
    In the regression test rules.sql there is this SQL command
    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;
    Which causes my alpha port to go core.
    Yeah. This was reported by Pedro Lobo on 11 June, and we've been
    patiently waiting for Jan to decide what to do about it :-(

    You could stop the coredump by putting a test into ResolveNew:

    {
    *nodePtr = copyObject(n);
    + if (IsA(*nodePtr, Var))
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
    }

    but what's not so clear is what's supposed to happen when the
    replacement item *isn't* a Var. I tried to convince myself that nothing
    needed to happen in that case, but wasn't successful. (Presumably the
    replacement expression contains no instances of the variable being
    replaced, so recursing into it with ResolveNew shouldn't be needed
    --- but maybe its varlevelsup values need adjusted?)

    regards, tom lane
    --
    Bruce Momjian | http://www.op.net/~candle
    maillist@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

    ************
  • Tom Lane at Nov 30, 1999 at 2:21 am

    Bruce Momjian writes:
    Any ideas on this one?
    Uncle George <gatgul@voicenet.com> writes:
    In the regression test rules.sql there is this SQL command
    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;
    Which causes my alpha port to go core.
    Yeah. This was reported by Pedro Lobo on 11 June, and we've been
    patiently waiting for Jan to decide what to do about it :-(

    You could stop the coredump by putting a test into ResolveNew:

    {
    *nodePtr = copyObject(n);
    + if (IsA(*nodePtr, Var))
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup;
    }

    but what's not so clear is what's supposed to happen when the
    replacement item *isn't* a Var. I tried to convince myself that nothing
    needed to happen in that case, but wasn't successful. (Presumably the
    replacement expression contains no instances of the variable being
    replaced, so recursing into it with ResolveNew shouldn't be needed
    --- but maybe its varlevelsup values need adjusted?)

    That code currently reads like:

    /* Make a copy of the tlist item to return */
    n = copyObject(n);
    if (IsA(n, Var))
    {
    ((Var *) n)->varlevelsup = this_varlevelsup;
    }
    /* XXX what to do if tlist item is NOT a var?
    * Should we be using something like apply_RIR_adjust_sublevel?
    */
    return n;

    so it won't coredump when the tlist item is not a Var, but I'm not
    convinced it does the right thing either. Jan is the only man who
    understands that code well enough to say what needs to be done about
    it...

    regards, tom lane
  • Ryan Kirkpatrick at Jul 30, 1999 at 3:33 pm

    On Fri, 30 Jul 1999, Thomas Lockhart wrote:

    As I see it, these are the following things that need to be added
    to 6.5.1 to make it alpha ready:
    * Uncle G's Alpha patches { which I have }.
    * Makefile conditionals for Linux/Alpha
    * Bruce's alignment patches { which I do not have }.
    Bruce, if you could get me your alignment patches, then I will try and
    apply the above to 6.5.1, and make a patch that bring 6.5.1 up to alpha
    ready state.
    I *love* this plan. Great!
    And I'll go one better: v6.5.x is not "dead", in the sense that Tom
    Lane has been faithfully applying relevant patches for his fixes in
    case a v6.5.2 is released. I'll guess that the Intel problems noted
    with the main tree are not present in the v6.5.x tree, so any new
    problems noted would be due to the upcoming Alpha patches.
    So, if I understand this correctly, the snapshot available on the
    FTP site is from the unstable tree, and there is a "stable 6.5.x" tree
    that can only be access by cvs{up}? And that this stable tree should not
    have quite as much delta from 6.5.1 as the snapshots do? Or did I miss
    something?
    Let's develop patches on 6.5.x (I'll post snapshots when we want them)
    and Lamar and I can test the Intel behavior.
    Ok, patches are in progress. Regression tests passed, now pounding
    on it with some of my own applications.
    We can publish an Alpha candidate tree so the debian folks can look at
    it, and we can build a RPM for someone (Uncle George?) to test on a
    RedHat box.
    Sounds good.
    v6.5.2 might be possible yet ;)
    Hmm... I don't think other people want to roll in the alpha
    patches into the stable tree (with good reason). I think we are best off
    with just an alpha only version of pgsql via patches on 6.5.1, and leave
    integration of the alpha patches into the full pgsql source tree for 6.6.
    My two cents.

    ----------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ----------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | rkirkpat@nag.cs.colorado.edu |
    ----------------------------------------------------------------------------
    http://www-ugrad.cs.colorado.edu/~rkirkpat/ |
    ----------------------------------------------------------------------------
  • Bruce Momjian at Jul 30, 1999 at 4:04 pm

    And I'll go one better: v6.5.x is not "dead", in the sense that Tom
    Lane has been faithfully applying relevant patches for his fixes in
    case a v6.5.2 is released. I'll guess that the Intel problems noted
    with the main tree are not present in the v6.5.x tree, so any new
    problems noted would be due to the upcoming Alpha patches.
    So, if I understand this correctly, the snapshot available on the
    FTP site is from the unstable tree, and there is a "stable 6.5.x" tree
    that can only be access by cvs{up}? And that this stable tree should not
    have quite as much delta from 6.5.1 as the snapshots do? Or did I miss
    something?
    Yes. This is correct.


    --
    Bruce Momjian | http://www.op.net/~candle
    maillist@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
  • The Hermit Hacker at Jul 30, 1999 at 4:08 pm

    On Fri, 30 Jul 1999, Ryan Kirkpatrick wrote:

    So, if I understand this correctly, the snapshot available on the
    FTP site is from the unstable tree, and there is a "stable 6.5.x" tree
    that can only be access by cvs{up}? And that this stable tree should not
    have quite as much delta from 6.5.1 as the snapshots do? Or did I miss
    something?
    this is correct...
    Hmm... I don't think other people want to roll in the alpha
    patches into the stable tree (with good reason). I think we are best off
    with just an alpha only version of pgsql via patches on 6.5.1, and leave
    integration of the alpha patches into the full pgsql source tree for 6.6.
    My two cents.
    We are going to be rolling a v6.5.2, and .3, and .4 ... basically, until
    v6.6 is released, v6.5.x is our stable release, and, from a commercial
    perspective, has to be maintained.

    I don't expect anyone working on -current to maintain it, I'm going to
    work on it, but I do hope that if someone fixes a bug in -current that
    exists in -stable, and that can be *easily* fixed, that we get the fix in
    there also...

    Marc G. Fournier ICQ#7615664 IRC Nick: Scrappy
    Systems Administrator @ hub.org
    primary: scrappy@hub.org secondary: scrappy@{freebsd|postgresql}.org
  • Ryan Kirkpatrick at Jul 30, 1999 at 4:32 pm

    On Fri, 30 Jul 1999, The Hermit Hacker wrote:
    On Fri, 30 Jul 1999, Ryan Kirkpatrick wrote:
    Hmm... I don't think other people want to roll in the alpha
    patches into the stable tree (with good reason). I think we are best off
    with just an alpha only version of pgsql via patches on 6.5.1, and leave
    integration of the alpha patches into the full pgsql source tree for 6.6.
    My two cents.
    We are going to be rolling a v6.5.2, and .3, and .4 ... basically, until
    v6.6 is released, v6.5.x is our stable release, and, from a commercial
    perspective, has to be maintained.
    I understand that. It is just that from what time I have spent
    looking at the alpha patches, they do a lot more than just "maintenance".
    So while there may indeed by 6.5.2, .3, etc.. releases, none of them
    should include the alpha patches in the source tree (instead have a new
    set of "after release" alpha specific patches, or stick them in contrib).
    I don't want to put the alpha patches in until after I have a chance to
    review them (for compatiblity to other platforms), which will probably
    take a few weeks to a few months.
    I don't expect anyone working on -current to maintain it, I'm going to
    work on it, but I do hope that if someone fixes a bug in -current that
    exists in -stable, and that can be *easily* fixed, that we get the fix in
    there also...
    Sounds good... Only the alpha fixes don't fall under the heading
    of "*easily* fixed in -stable", so they ought to stay out of there for
    now.
    Otherwise your seperation of stable from current trees is a good
    idea, and I now have a better understanding of development and release of
    pgsql, especially in relation to the alpha patches. Thanks.

    ----------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ----------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | rkirkpat@nag.cs.colorado.edu |
    ----------------------------------------------------------------------------
    http://www-ugrad.cs.colorado.edu/~rkirkpat/ |
    ----------------------------------------------------------------------------
  • The Hermit Hacker at Jul 30, 1999 at 4:56 pm

    On Fri, 30 Jul 1999, Ryan Kirkpatrick wrote:

    Sounds good... Only the alpha fixes don't fall under the heading
    of "*easily* fixed in -stable", so they ought to stay out of there for
    now.
    If there are even pieces of the alpha patches that can be applied to the
    central repository, please submit them for inclusion...#ifdef __alpha__
    works quite well :) Reducing the size of the patch for each release is
    always a good thing...

    Marc G. Fournier ICQ#7615664 IRC Nick: Scrappy
    Systems Administrator @ hub.org
    primary: scrappy@hub.org secondary: scrappy@{freebsd|postgresql}.org
  • Ryan Kirkpatrick at Jul 30, 1999 at 5:26 pm

    On Fri, 30 Jul 1999, The Hermit Hacker wrote:
    On Fri, 30 Jul 1999, Ryan Kirkpatrick wrote:

    Sounds good... Only the alpha fixes don't fall under the heading
    of "*easily* fixed in -stable", so they ought to stay out of there for
    now.
    If there are even pieces of the alpha patches that can be applied to the
    central repository, please submit them for inclusion...#ifdef __alpha__
    works quite well :) Reducing the size of the patch for each release is
    always a good thing...
    That is what I plan to do in the coming months as I review the
    alpha patches chagne by change. And yes, #ifdefs are always good to use!
    :)

    ----------------------------------------------------------------------------
    "For to me to live is Christ, and to die is gain." |
    --- Philippians 1:21 (KJV) |
    ----------------------------------------------------------------------------
    Ryan Kirkpatrick | Boulder, Colorado | rkirkpat@nag.cs.colorado.edu |
    ----------------------------------------------------------------------------
    http://www-ugrad.cs.colorado.edu/~rkirkpat/ |
    ----------------------------------------------------------------------------
  • Bruce Momjian at Sep 23, 1999 at 10:01 pm
    Can anyone address the status of this bug?

    In the regression test rules.sql there is this SQL command

    update rtest_v1 set a = rtest_t3.a + 20 where b = rtest_t3.b;

    Which causes my alpha port to go core. The above line can be reduced to:

    update rtest_v1 set a = rtest_t3.a + 20 ;

    which also causes the same problem. It seems that the 64 bit address
    ((Expr*)nodeptr)->oper gets truncated ( high 32 bits ) somewhere along the way.

    I was able to locate the errant code in rewriteManip.c:712. but There seems to be a
    bigger problem other than eraseing the upper 32bit address. It seems that
    FindMatchingNew() returns a node of type T_Expr, rather than the expected type of
    T_Var. Once u realize this then u can see why the now MISCAST "(Var *)
    *nodePtr)->varlevelsup = this_varlevelsup" will cause a problem. On my alpha this erases
    a portion in the address in the T_Expr. On the redhat 5.2/i386 this code seems to be
    benign, BUT YOU ARE ERASEING SOMETHING that doesn't belong to to T_Expr !

    So what gives?
    gat
    Maybe an assert() will help in finding some of the miscast returned types? Wuddya think?
    sure would catch some of the boo-boo's hanging around

    rewriteManip.c:
    if (this_varno == info->new_varno &&
    this_varlevelsup == sublevels_up)
    {
    n = FindMatchingNew(targetlist,
    ((Var *) node)->varattno);
    if (n == NULL)
    {
    if (info->event == CMD_UPDATE)
    {
    *nodePtr = n = copyObject(node);
    ((Var *) n)->varno = info->current_varno;
    ((Var *) n)->varnoold = info->current_varno;
    }
    else
    *nodePtr = make_null(((Var *) node)->vartype);
    }
    else
    {
    *nodePtr = copyObject(n);
    ((Var *) *nodePtr)->varlevelsup = this_varlevelsup; /* This
    line zaps the address */
    }
    }






    --
    Bruce Momjian | http://www.op.net/~candle
    maillist@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
  • Lamar Owen at Sep 23, 1999 at 10:39 pm

    Bruce Momjian wrote:

    Can anyone address the status of this bug?
    [actual bug text snipped...]

    Wow, Bruce. That's an old thread. I'll just say this -- 6.5.1 with
    Uncle George and Ryan Kirkpatrick's patchset applied passes regression
    at RedHat on their alpha development machine (for the RPM distribution).

    Whether the current pre-6.6 tree passes regression or not, I can't say.

    The author of the original message you replied to is gat -- AKA Uncle
    George.

    Lamar OWen
    WGCR Internet Radio
  • Tom Lane at Sep 24, 1999 at 12:32 am

    Bruce Momjian writes:
    Can anyone address the status of this bug?
    AFAIK it hasn't changed since July --- we've been waiting for Jan
    to opine on the proper fix, but he's been busy...

    regards, tom lane

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJul 20, '99 at 12:29a
activeNov 30, '99 at 2:21a
posts15
users7
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase