Обсуждение: Another bug in pg_operator.h

Поиск
Список
Период
Сортировка

Another bug in pg_operator.h

От
Tom Lane
Дата:
As long as we're doing last-minute changes that force initdb ...

I believe I've just identified another error in pg_operator.h.
Specifically, this tuple is bogus and ought to be deleted:

DATA(insert OID = 619 (  "<"       PGUID 0 b t f 704 704  16   0   0  0  0 intervalct - - ));

This tuple defines "<" on tinterval's (type 704 = TINTERVAL) as
being "intervalct", or interval-contained-in.  Now that doesn't
make a lot of sense.  Furthermore there is already a tuple
that defines "<<" as being intervalct:

DATA(insert OID = 573 (  "<<"       PGUID 0 b t f 704 704  16   0   0   0   0 intervalct - - ));

and there is another tuple that defines "<" on tintervals as
being "intervallt":

DATA(insert OID = 813 (  "<"       PGUID 0 b t f 704 704  16 813   0   0   0 intervallt - - ));

I found this while trying to track down why the tinterval regression
test fails on HPUX (it produces the right tuples, but in the wrong
sort order ... and unlike the rules case, the select does have an
ORDER BY clause).  As far as I can tell from the CVS logs, the bogosity
has been in there for at least a year.

It looks to me like the bogus tuple is masking the correct definition
of "<" and causing tintervals to sort incorrectly.  Why this doesn't
happen on many other machines isn't clear, but maybe it has something
to do with the platform-dependency of qsort that we already identified?

Anyway, I'm a rank newbie at hacking this stuff, so I'm not about
to commit this change without asking someone else to check my work.
But if it is indeed wrong, right now seems like a good time to fix it...
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Wed, 28 Oct 1998, Tom Lane wrote:

> As long as we're doing last-minute changes that force initdb ...

You guys all know I'm going to be a super-prick next release, right?
*grin*  Just a pre-warning... :)


Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
Actually, the darn thing is *crawling* with bugs.

Once I understood what the commute and negate links were all about,
I applied a few simple sanity checks (like, if A says it commutes to B,
does B say it commutes to A?).  This was, um, fruitful.

The comparison operators on booleans, tintervals, and lsegs are all
pretty seriously busted --- they have incorrect commute and/or negate
links.  I am worried about what other similar bugs may be in there,
but short of groveling over the entries by hand, I don't know what else
to do to check the table further.

BTW Bruce's fix of this morning wasn't right either --- he misspelled
the name of the lseg_ne comparison routine.

I am currently running a build and regression test for the attached
diffs, which I propose to commit if the regress passes.

I expect to find that this explains the tinterval regression differences
I've been seeing.  The tinterval expected file is clearly wrong,
because it contains "sorted" lists of tintervals that are obviously not
in order.  I've been getting a different but just as wrong sort order
on HPUX.  With luck this fix will produce a correct sort order.

BTW, is it sufficient to edit pg_operator.h, rebuild, reinstall, and
initdb?  Or is there some additional incantation that needs to be made
when patching pg_operator.h?
        regards, tom lane

PS: Stop the presses .... looks like int8 and int4-vs-int8 comparators are
bogus too ... geez ...


*** pg_operator.h.orig    Wed Oct 28 12:10:48 1998
--- pg_operator.h    Wed Oct 28 18:32:20 1998
***************
*** 95,102 **** DATA(insert OID =  80 ( "<="       PGUID 0 b t f  23  20  16 419 419   0   0 int48le intlesel
intlejoinsel)); DATA(insert OID =  82 ( ">="       PGUID 0 b t f  23  20  16 418 418   0   0 int48ge intgesel
intgejoinsel)); 
 
! DATA(insert OID =  58 ( "<"           PGUID 0 b t f  16  16  16  85  91   0   0 boollt intltsel intltjoinsel ));
! DATA(insert OID =  59 ( ">"           PGUID 0 b t f  16  16  16  85  91   0   0 boolgt intltsel intltjoinsel ));
DATA(insertOID =  85 ( "<>"       PGUID 0 b t f  16  16  16  85  91   0   0 boolne neqsel neqjoinsel )); DATA(insert
OID=  91 ( "="           PGUID 0 b t t  16  16  16  91  85   0   0 booleq eqsel eqjoinsel )); #define
BooleanEqualOperator  91
 
--- 95,102 ---- DATA(insert OID =  80 ( "<="       PGUID 0 b t f  23  20  16 419 419   0   0 int48le intlesel
intlejoinsel)); DATA(insert OID =  82 ( ">="       PGUID 0 b t f  23  20  16 418 418   0   0 int48ge intgesel
intgejoinsel)); 
 
! DATA(insert OID =  58 ( "<"           PGUID 0 b t f  16  16  16  59   0   0   0 boollt intltsel intltjoinsel ));
! DATA(insert OID =  59 ( ">"           PGUID 0 b t f  16  16  16  58   0   0   0 boolgt intltsel intltjoinsel ));
DATA(insertOID =  85 ( "<>"       PGUID 0 b t f  16  16  16  85  91   0   0 boolne neqsel neqjoinsel )); DATA(insert
OID=  91 ( "="           PGUID 0 b t t  16  16  16  91  85   0   0 booleq eqsel eqjoinsel )); #define
BooleanEqualOperator  91
 
***************
*** 191,198 **** DATA(insert OID = 517 (  "<->"       PGUID 0 b t f 600 600 701 517   0   0   0 point_distance intltsel
intltjoinsel)); DATA(insert OID = 518 (  "<>"       PGUID 0 b t f  23  23  16 518  96  0  0 int4ne neqsel neqjoinsel
));DATA(insert OID = 519 (  "<>"       PGUID 0 b t f  21  21  16 519  94  0  0 int2ne neqsel neqjoinsel ));
 
! DATA(insert OID = 520 (  ">"       PGUID 0 b t f  21  21  16  95   0  0  0 int2gt intgtsel intgtjoinsel ));
! DATA(insert OID = 521 (  ">"       PGUID 0 b t f  23  23  16  97   0  0  0 int4gt intgtsel intgtjoinsel ));
DATA(insertOID = 522 (  "<="       PGUID 0 b t f  21  21  16 524 520  0  0 int2le intltsel intltjoinsel )); DATA(insert
OID= 523 (  "<="       PGUID 0 b t f  23  23  16 525 521  0  0 int4le intltsel intltjoinsel )); DATA(insert OID = 524 (
">="       PGUID 0 b t f  21  21  16 522  95  0  0 int2ge intgtsel intgtjoinsel ));
 
--- 191,198 ---- DATA(insert OID = 517 (  "<->"       PGUID 0 b t f 600 600 701 517   0   0   0 point_distance intltsel
intltjoinsel)); DATA(insert OID = 518 (  "<>"       PGUID 0 b t f  23  23  16 518  96  0  0 int4ne neqsel neqjoinsel
));DATA(insert OID = 519 (  "<>"       PGUID 0 b t f  21  21  16 519  94  0  0 int2ne neqsel neqjoinsel ));
 
! DATA(insert OID = 520 (  ">"       PGUID 0 b t f  21  21  16  95 522  0  0 int2gt intgtsel intgtjoinsel ));
! DATA(insert OID = 521 (  ">"       PGUID 0 b t f  23  23  16  97 523  0  0 int4gt intgtsel intgtjoinsel ));
DATA(insertOID = 522 (  "<="       PGUID 0 b t f  21  21  16 524 520  0  0 int2le intltsel intltjoinsel )); DATA(insert
OID= 523 (  "<="       PGUID 0 b t f  23  23  16 525 521  0  0 int4le intltsel intltjoinsel )); DATA(insert OID = 524 (
">="       PGUID 0 b t f  21  21  16 522  95  0  0 int2ge intgtsel intgtjoinsel ));
 
***************
*** 298,305 **** DATA(insert OID = 617 (  "<->"       PGUID 0 b t f 601 603 701   0   0  0  0 dist_sb - - ));
DATA(insertOID = 618 (  "<->"       PGUID 0 b t f 600 602 701   0   0  0  0 dist_ppath - - )); 
 
- DATA(insert OID = 619 (  "<"       PGUID 0 b t f 704 704  16   0   0  0  0 intervalct - - ));
-  DATA(insert OID = 620 (  "="       PGUID 0 b t t  700  700    16 620 621    622 622 float4eq eqsel eqjoinsel ));
DATA(insertOID = 621 (  "<>"       PGUID 0 b t f  700  700    16 621 620    0 0 float4ne neqsel neqjoinsel ));
DATA(insertOID = 622 (  "<"       PGUID 0 b t f  700  700    16 623 625    0 0 float4lt intltsel intltjoinsel ));
 
--- 298,303 ----
***************
*** 400,411 **** DATA(insert OID =  808 (  "?-"       PGUID 0 b t f  600  600     16  808  0 0 0 point_horiz - - ));
DATA(insertOID =  809 (  "?|"       PGUID 0 b t f  600  600     16  809  0 0 0 point_vert - - )); 
 
! DATA(insert OID = 811 (  "="       PGUID 0 b t t 704 704  16 811   0   0   0 intervaleq - - ));
! DATA(insert OID = 812 (  "<>"       PGUID 0 b t f 704 704  16 812   0   0   0 intervalne - - ));
! DATA(insert OID = 813 (  "<"       PGUID 0 b t f 704 704  16 813   0   0   0 intervallt - - ));
! DATA(insert OID = 814 (  ">"       PGUID 0 b t f 704 704  16 814   0   0   0 intervalgt - - ));
! DATA(insert OID = 815 (  "<="       PGUID 0 b t f 704 704  16 815   0   0   0 intervalle - - ));
! DATA(insert OID = 816 (  ">="       PGUID 0 b t f 704 704  16 816   0   0   0 intervalge - - ));  DATA(insert OID =
843(  "*"       PGUID 0 b t f  790  700    790 845   0   0   0 cash_mul_flt4 - - )); DATA(insert OID = 844 (  "/"
PGUID0 b t f  790  700    790   0   0   0   0 cash_div_flt4 - - ));
 
--- 398,409 ---- DATA(insert OID =  808 (  "?-"       PGUID 0 b t f  600  600     16  808  0 0 0 point_horiz - - ));
DATA(insertOID =  809 (  "?|"       PGUID 0 b t f  600  600     16  809  0 0 0 point_vert - - )); 
 
! DATA(insert OID = 811 (  "="       PGUID 0 b t t 704 704  16 811 812   0   0 intervaleq - - ));
! DATA(insert OID = 812 (  "<>"       PGUID 0 b t f 704 704  16 812 811   0   0 intervalne - - ));
! DATA(insert OID = 813 (  "<"       PGUID 0 b t f 704 704  16 814 816   0   0 intervallt - - ));
! DATA(insert OID = 814 (  ">"       PGUID 0 b t f 704 704  16 813 815   0   0 intervalgt - - ));
! DATA(insert OID = 815 (  "<="       PGUID 0 b t f 704 704  16 816 814   0   0 intervalle - - ));
! DATA(insert OID = 816 (  ">="       PGUID 0 b t f 704 704  16 815 813   0   0 intervalge - - ));  DATA(insert OID =
843(  "*"       PGUID 0 b t f  790  700    790 845   0   0   0 cash_mul_flt4 - - )); DATA(insert OID = 844 (  "/"
PGUID0 b t f  790  700    790   0   0   0   0 cash_div_flt4 - - ));
 
***************
*** 597,603 **** DATA(insert OID = 1527 (  "?-|"   PGUID 0 b t f  601  601    16 1527  0 0 0 lseg_perp - - ));
DATA(insertOID = 1528 (  "?-"      PGUID 0 l t f    0  601    16 1528  0 0 0 lseg_horizontal - - )); DATA(insert OID =
1529(  "?|"      PGUID 0 l t f    0  601    16 1529  0 0 0 lseg_vertical - - ));
 
! DATA(insert OID = 1535 (  "="      PGUID 0 b t f  601  601    16 1535  0 0 0 lseg_eq intltsel - )); DATA(insert OID =
1536(  "#"      PGUID 0 b t f  601  601  600 1536  0 0 0 lseg_interpt - - )); DATA(insert OID = 1537 (  "?#"      PGUID
0b t f  601  628    16 1537  0 0 0 inter_sl - - )); DATA(insert OID = 1538 (  "?#"      PGUID 0 b t f  601  603    16
1538 0 0 0 inter_sb - - ));
 
--- 595,601 ---- DATA(insert OID = 1527 (  "?-|"   PGUID 0 b t f  601  601    16 1527  0 0 0 lseg_perp - - ));
DATA(insertOID = 1528 (  "?-"      PGUID 0 l t f    0  601    16 1528  0 0 0 lseg_horizontal - - )); DATA(insert OID =
1529(  "?|"      PGUID 0 l t f    0  601    16 1529  0 0 0 lseg_vertical - - ));
 
! DATA(insert OID = 1535 (  "="      PGUID 0 b t f  601  601    16 1535 1586 0 0 lseg_eq intltsel - )); DATA(insert OID
=1536 (  "#"      PGUID 0 b t f  601  601  600 1536  0 0 0 lseg_interpt - - )); DATA(insert OID = 1537 (  "?#"
PGUID0 b t f  601  628    16 1537  0 0 0 inter_sl - - )); DATA(insert OID = 1538 (  "?#"      PGUID 0 b t f  601  603
16 1538  0 0 0 inter_sb - - ));
 
***************
*** 619,629 **** DATA(insert OID = 1578 (  "##"      PGUID 0 b t f  601  601  600      0  0 0 0 close_lseg - - ));
DATA(insertOID = 1585 (  "/"      PGUID 0 b t f 1186 1186 1186      0  0 0 0 timespan_div - - )); 
 
! DATA(insert OID = 1586 (  "<>"      PGUID 0 b t f  601  601    16 1535  0 0 0 lseg_neq intltsel - ));
! DATA(insert OID = 1587 (  "<"      PGUID 0 b t f  601  601    16 1590  0 0 0 lseg_lt intltsel - ));
! DATA(insert OID = 1588 (  "<="      PGUID 0 b t f  601  601    16 1589  0 0 0 lseg_le intltsel - ));
! DATA(insert OID = 1589 (  ">"      PGUID 0 b t f  601  601    16 1588  0 0 0 lseg_gt intltsel - ));
! DATA(insert OID = 1590 (  ">="      PGUID 0 b t f  601  601    16 1587  0 0 0 lseg_ge intltsel - ));  DATA(insert OID
=1591 (  "@-@"   PGUID 0 l t f 0  601    701    0  0 0 0 lseg_length - - )); 
 
--- 617,627 ---- DATA(insert OID = 1578 (  "##"      PGUID 0 b t f  601  601  600      0  0 0 0 close_lseg - - ));
DATA(insertOID = 1585 (  "/"      PGUID 0 b t f 1186 1186 1186      0  0 0 0 timespan_div - - )); 
 
! DATA(insert OID = 1586 (  "<>"      PGUID 0 b t f  601  601    16 1586 1535 0 0 lseg_ne intltsel - ));
! DATA(insert OID = 1587 (  "<"      PGUID 0 b t f  601  601    16 1589 1590 0 0 lseg_lt intltsel - ));
! DATA(insert OID = 1588 (  "<="      PGUID 0 b t f  601  601    16 1590 1589 0 0 lseg_le intltsel - ));
! DATA(insert OID = 1589 (  ">"      PGUID 0 b t f  601  601    16 1587 1588 0 0 lseg_gt intltsel - ));
! DATA(insert OID = 1590 (  ">="      PGUID 0 b t f  601  601    16 1588 1587 0 0 lseg_ge intltsel - ));  DATA(insert
OID= 1591 (  "@-@"   PGUID 0 l t f 0  601    701    0  0 0 0 lseg_length - - )); 
 




Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Wed, 28 Oct 1998, Tom Lane wrote:

> Actually, the darn thing is *crawling* with bugs.

Guess BETA3 won't be the release, eh? *grin*


Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> This was, um, fruitful.

My *goodness*, that was a good idea.

I have now located and repaired ninety-three distinct bugs in
pg_operator.h, all of the form "operator A has an incorrect com, negate,
or sort link to operator B".  Almost none of them required any semantic
analysis to spot --- I found them by looking for conditions like A links
to B but B doesn't link to A, or A claims B is its commutation but B
doesn't have the right input data types to be that, etc.

The tinterval regress test now produces believable results --- ie,
the order in which allegedly-sorted intervals appear actually agrees
with the length of the intervals.  I would imagine that we can get
rid of the special-case NetBSD expected output for tinterval, as well.

I will shortly commit these changes (as soon as I finish another
build/regress pass).  I will also commit a new regression test script
that looks for all the test conditions that I used to locate these
problems, in hopes that no new bugs of this ilk will creep in.

There is one unfixed bug in pg_operator, which I let go because I didn't
want to make the decision as to what to do with it, but it needs to be
fixed.  Namely, there is a conflict between OIDs 512 (on_ppath) and 754
(pt_contained_path), both of which claim to be the implementation of
"point @ path":

DATA(insert OID = 512 (  "@"       PGUID 0 b t f 600 602  16   0   0   0   0 on_ppath intltsel intltjoinsel ));

DATA(insert OID = 754 (  "@"       PGUID 0 b t f  600  602     16  755  0 0 0 pt_contained_path - - ));

I imagine that which one you actually get is a quasi-random matter...
we need to decide which one is the One True @, and change the oprname
of the other one.

I suggest that it'd be a Good Idea to develop similar check scripts for
the other hand-maintained tables.  But I figure I've done enough for one
day...
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
dg@informix.com (David Gould)
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> > This was, um, fruitful.
> 
> My *goodness*, that was a good idea.
> 
> I have now located and repaired ninety-three distinct bugs in
> pg_operator.h, all of the form "operator A has an incorrect com, negate,
> or sort link to operator B".  Almost none of them required any semantic
> analysis to spot --- I found them by looking for conditions like A links
> to B but B doesn't link to A, or A claims B is its commutation but B
> doesn't have the right input data types to be that, etc.

Bravo! I did this for Illustra a couple years ago. What a pain. But it is
nice to get right answers. Also, it lets the planner generate better plans.
If there is a negator or commutator it can use it instead of generating extra
steps.

-dg

David Gould            dg@informix.com           510.628.3783 or 510.305.9468 
Informix Software  (No, really)         300 Lakeside Drive  Oakland, CA 94612- If simplicity worked, the world would be
overrunwith insects. -
 


Re: [HACKERS] Another bug in pg_operator.h

От
"Thomas G. Lockhart"
Дата:
> Actually, the darn thing is *crawling* with bugs.

OK, sit down and take a deep breath. And another one. OK, do you still
see bugs everywhere? Oh, that's too bad. We'll send someone over right
away. Stay away from sharp objects, OK? *grin*

> The comparison operators on booleans, tintervals, and lsegs are all
> pretty seriously busted --- they have incorrect commute and/or negate
> links.  I am worried about what other similar bugs may be in there,
> but short of groveling over the entries by hand, I don't know what 
> else to do to check the table further.

Well, we should grovel over the entries by hand, for v6.5. I'd love to
get that thing cleaned up. But I'll bet that some of what you're
finding, although inconsistant, has no adverse effect currently. And it
isn't worse than in previous releases (and in some places it is better). 

In particular, the commute and negate links are not used as extensively
as you might think they should be. But I'd like them to be used more in
the future.

I vaguely recall that I do (mis-)use the commute operators for entries
for which the commute operator would not otherwise be meaningful to help
reorder expressions in the parser. I think that for mixed-type operators
I use the commute column to suggest an alternate operator when trying to
find candidates in the new type coersion stuff.

> BTW Bruce's fix of this morning wasn't right either --- he misspelled
> the name of the lseg_ne comparison routine.

Well, fix that for sure...

> I am currently running a build and regression test for the attached
> diffs, which I propose to commit if the regress passes.
> I expect to find that this explains the tinterval regression 
> differences I've been seeing.  The tinterval expected file is clearly 
> wrong, because it contains "sorted" lists of tintervals that are 
> obviously not in order.

tinterval is an "orphan" type. If it is really a useful type for people,
we should adopt it and fix it up.

> BTW, is it sufficient to edit pg_operator.h, rebuild, reinstall, and
> initdb?  Or is there some additional incantation that needs to be made
> when patching pg_operator.h?

Not that I can recall. You shouldn't reassign OIDs, and there is a small
chance that an OID you remove is actually referenced in one of the
indexing tables (I don't recall the exact layout). I've found that I
need to do a full "make clean install", otherwise some things end up in
an inconsistant state, probably due to incomplete Makefile dependencies.

> Stop the presses .... looks like int8 and int4-vs-int8 comparators 
> are bogus too ... geez ...

Is this a robust system or what! You can get half the tables wrong and
still get good results. Those AI guys are green with envy :)

btw, what'd I do wrong on those?
                  - Tom


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> As long as we're doing last-minute changes that force initdb ...
> 
> I believe I've just identified another error in pg_operator.h.
> Specifically, this tuple is bogus and ought to be deleted:
> 
> DATA(insert OID = 619 (  "<"       PGUID 0 b t f 704 704  16   0   0  0  0 intervalct - - ));

Yep.  I have removed the entry.  It didn't even look like it belonged
there.

> 
> This tuple defines "<" on tinterval's (type 704 = TINTERVAL) as
> being "intervalct", or interval-contained-in.  Now that doesn't
> make a lot of sense.  Furthermore there is already a tuple
> that defines "<<" as being intervalct:
> 
> DATA(insert OID = 573 (  "<<"       PGUID 0 b t f 704 704  16   0   0   0   0 intervalct - - ));
> 
> and there is another tuple that defines "<" on tintervals as
> being "intervallt":
> 
> DATA(insert OID = 813 (  "<"       PGUID 0 b t f 704 704  16 813   0   0   0 intervallt - - ));
> 
> I found this while trying to track down why the tinterval regression
> test fails on HPUX (it produces the right tuples, but in the wrong
> sort order ... and unlike the rules case, the select does have an
> ORDER BY clause).  As far as I can tell from the CVS logs, the bogosity
> has been in there for at least a year.

That is an amazing way to find a bad pg_operator entry.

> 
> It looks to me like the bogus tuple is masking the correct definition
> of "<" and causing tintervals to sort incorrectly.  Why this doesn't
> happen on many other machines isn't clear, but maybe it has something
> to do with the platform-dependency of qsort that we already identified?
> 
> Anyway, I'm a rank newbie at hacking this stuff, so I'm not about
> to commit this change without asking someone else to check my work.
> But if it is indeed wrong, right now seems like a good time to fix it...

I have done the dirty deed, so Marc can break my fingers.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> On Wed, 28 Oct 1998, Tom Lane wrote:
> 
> > As long as we're doing last-minute changes that force initdb ...
> 
> You guys all know I'm going to be a super-prick next release, right?
> *grin*  Just a pre-warning... :)

Marc is a Canadian Frenchy.  If they are want to succeed from their own
country, they could do anything.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> On Wed, 28 Oct 1998, Tom Lane wrote:
> 
> > Actually, the darn thing is *crawling* with bugs.
> 
> Guess BETA3 won't be the release, eh? *grin*

I can see an argument for letting it go for 6.5, but if it is broken,
and is showing up in regression testing, I think we have to fix it.

We can't fix it in 6.4.*.  No initdb for that.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
> > This was, um, fruitful.
> 
> My *goodness*, that was a good idea.
> 
> I have now located and repaired ninety-three distinct bugs in
> pg_operator.h, all of the form "operator A has an incorrect com, negate,
> or sort link to operator B".  Almost none of them required any semantic
> analysis to spot --- I found them by looking for conditions like A links
> to B but B doesn't link to A, or A claims B is its commutation but B
> doesn't have the right input data types to be that, etc.

Green light.  Go.

> 
> The tinterval regress test now produces believable results --- ie,
> the order in which allegedly-sorted intervals appear actually agrees
> with the length of the intervals.  I would imagine that we can get
> rid of the special-case NetBSD expected output for tinterval, as well.
> 
> I will shortly commit these changes (as soon as I finish another
> build/regress pass).  I will also commit a new regression test script
> that looks for all the test conditions that I used to locate these
> problems, in hopes that no new bugs of this ilk will creep in.

What I did was to make a file in the include/catalog directory called
template1_check.sql and pg_attribute_check.sql.  These are SQL
statements that check various catalogs and report problems where things
are missing joins.  Perhaps we could put something in there, or move
these to the regression directory and include them in your stuff.  They
are not platform-specific.

Would be nice so we could spot the problems right away.  Or perhaps put
a script in include/catalogs that checks all the system tables, and run
that from the regression directory.  That would be nice too.

> 
> There is one unfixed bug in pg_operator, which I let go because I didn't
> want to make the decision as to what to do with it, but it needs to be
> fixed.  Namely, there is a conflict between OIDs 512 (on_ppath) and 754
> (pt_contained_path), both of which claim to be the implementation of
> "point @ path":
> 
> DATA(insert OID = 512 (  "@"       PGUID 0 b t f 600 602  16   0   0   0   0 on_ppath intltsel intltjoinsel ));
> 
> DATA(insert OID = 754 (  "@"       PGUID 0 b t f  600  602     16  755  0 0 0 pt_contained_path - - ));


I see "@" means "on" sometimes, and "contained" sometimes, and they use
"@" for both uses for the point/path combination.  Looks like a problem
that "@" applies to point/path, and "on" and "contained" are both valid.
However, they seem to mean the same thing.  Are on_ppath and
pt_contained_path doing the same thing.  Thomas could help here.


--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Wed, 28 Oct 1998, Bruce Momjian wrote:

> I have done the dirty deed, so Marc can break my fingers.
One of these days, I'll brave crossing the border into "red neck
cop territory" :)

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Wed, 28 Oct 1998, Bruce Momjian wrote:

> > On Wed, 28 Oct 1998, Tom Lane wrote:
> > 
> > > As long as we're doing last-minute changes that force initdb ...
> > 
> > You guys all know I'm going to be a super-prick next release, right?
> > *grin*  Just a pre-warning... :)
> 
> Marc is a Canadian Frenchy.  If they are want to succeed from their own
> country, they could do anything.
Hey hey, careful there.  I'm against seperatism, for one.  And I
was born in Nova Scotia for two...just got alot of Frenchy blood in me is
all :)

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Wed, 28 Oct 1998, Bruce Momjian wrote:

> > On Wed, 28 Oct 1998, Tom Lane wrote:
> > 
> > > Actually, the darn thing is *crawling* with bugs.
> > 
> > Guess BETA3 won't be the release, eh? *grin*
> 
> I can see an argument for letting it go for 6.5, but if it is broken,
> and is showing up in regression testing, I think we have to fix it.
> 
> We can't fix it in 6.4.*.  No initdb for that.
Bruce, can you and Thomas go through Tom's patch with a fine
toothed comb before its committed?  this close to release (5days!), I'd
rather have a tri-agreement on it, but from what David mentioned, it seems
that it would be "a good thing" to get in before v6.4 is released...
If all three of you feel it is a) right and b) safe, throw it in.
I'll do up a quick BETA4 (release candidate 2) as soon as you tell me
whether or not its being/been committed...
Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
>     Bruce, can you and Thomas go through Tom's patch with a fine
> toothed comb before its committed?  this close to release (5days!), I'd
> rather have a tri-agreement on it, but from what David mentioned, it seems
> that it would be "a good thing" to get in before v6.4 is released...
> 
>     If all three of you feel it is a) right and b) safe, throw it in.
> I'll do up a quick BETA4 (release candidate 2) as soon as you tell me
> whether or not its being/been committed...

Looks like it is already in.  I will check it.  Thomas is checking the
'@' issue right now on IRC.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> > Marc is a Canadian Frenchy.  If they are want to succeed from their own
> > country, they could do anything.
> 
>     Hey hey, careful there.  I'm against seperatism, for one.  And I
> was born in Nova Scotia for two...just got alot of Frenchy blood in me is
> all :)

I just wanted to use the word "Frenchy", that's all.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Thu, 29 Oct 1998, Bruce Momjian wrote:

> > > Marc is a Canadian Frenchy.  If they are want to succeed from their own
> > > country, they could do anything.
> > 
> >     Hey hey, careful there.  I'm against seperatism, for one.  And I
> > was born in Nova Scotia for two...just got alot of Frenchy blood in me is
> > all :)
> 
> I just wanted to use the word "Frenchy", that's all.
*roll eyes* :)

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
"Thomas G. Lockhart"
Дата:
>         Bruce, can you and Thomas go through Tom's patch with a fine
> toothed comb before its committed?  this close to release (5days!)

I'm sure they are good things, and Tom Lane has been careful and
thoughtful about what he does in past patches (and sticks around to pick
up the pieces :). Does the regression test still behave in all other
respects Tom?

I'm a bit worried about keeping up with the docs schedule. So if I look
at these tables, and if my docs get behind, can you consider giving me a
couple of days to recover (only if I need it)? I'm out of town this
weekend too, which doesn't help...
               - Tom


Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
dg@informix.com (David Gould) writes:
> Also, it lets the planner generate
> better plans.  If there is a negator or commutator it can use it
> instead of generating extra steps.

Well, I did *not* go looking for links that should be there and weren't
(except in the very special case that the reverse link existed).  I
just tried to sanity-check the existing links.

I agree that it would be nice to look for missing links that should
be added ... but that is a performance enhancement, not a bug fix,
so I am not eager to do it this close to release.  We should do
another pass over this table after 6.4 is out the door.

(Another reason I didn't try to do that is that I've got no good
idea how to find missing links, short of brain-numbingly tedious
hand examination.  Can you suggest any automated way of finding
missing links, or at least finding likely things to look at?)
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Thu, 29 Oct 1998, Thomas G. Lockhart wrote:

> >         Bruce, can you and Thomas go through Tom's patch with a fine
> > toothed comb before its committed?  this close to release (5days!)
> 
> I'm sure they are good things, and Tom Lane has been careful and
> thoughtful about what he does in past patches (and sticks around to pick
> up the pieces :). Does the regression test still behave in all other
> respects Tom?
Quite frankly, this close to the release, I'd kinda ask the same
if it were any of the other committers too...I'm not really singling out
Tom :)

> I'm a bit worried about keeping up with the docs schedule. So if I look
> at these tables, and if my docs get behind, can you consider giving me a
> couple of days to recover (only if I need it)? I'm out of town this
> weekend too, which doesn't help...
Consider it done...if I hear nothing from you, I'll still consider
Monday a go...

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 



Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> I will also commit a new regression test script
>> that looks for all the test conditions that I used to locate these
>> problems, in hopes that no new bugs of this ilk will creep in.

> What I did was to make a file in the include/catalog directory called
> template1_check.sql and pg_attribute_check.sql.  These are SQL
> statements that check various catalogs and report problems where things
> are missing joins.  Perhaps we could put something in there, or move
> these to the regression directory and include them in your stuff.

I saw those but it wasn't clear to me when they would get applied or
whether they were hand-generated or derived from something else.  So
I went and made a new regression test, because I think I comprehend
those.  If you want to fold the opr_sanity regress test into one of
the sql files in include/catalog, go right ahead.  (Or, maybe those
files should be pushed over to regression testing?  I dunno.)

> I see "@" means "on" sometimes, and "contained" sometimes, and they use
> "@" for both uses for the point/path combination.  Looks like a problem
> that "@" applies to point/path, and "on" and "contained" are both valid.
> However, they seem to mean the same thing.  Are on_ppath and
> pt_contained_path doing the same thing.  Thomas could help here.

I would expect that on_ppath checks to see if the point is on (touches)
the path, whereas pt_contained_path checks to see if the point is within
the area enclosed by the path.  But I haven't looked to see if that's
what the author of the code thought...
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> dg@informix.com (David Gould) writes:
> > Also, it lets the planner generate
> > better plans.  If there is a negator or commutator it can use it
> > instead of generating extra steps.
> 
> Well, I did *not* go looking for links that should be there and weren't
> (except in the very special case that the reverse link existed).  I
> just tried to sanity-check the existing links.
> 
> I agree that it would be nice to look for missing links that should
> be added ... but that is a performance enhancement, not a bug fix,
> so I am not eager to do it this close to release.  We should do
> another pass over this table after 6.4 is out the door.
> 
> (Another reason I didn't try to do that is that I've got no good
> idea how to find missing links, short of brain-numbingly tedious
> hand examination.  Can you suggest any automated way of finding
> missing links, or at least finding likely things to look at?)

Yes.  See include/catalog/template1_check.sql.  Uses SQL to do checks.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
dg@informix.com (David Gould)
Дата:
> 
> dg@informix.com (David Gould) writes:
> > Also, it lets the planner generate
> > better plans.  If there is a negator or commutator it can use it
> > instead of generating extra steps.
> 
> Well, I did *not* go looking for links that should be there and weren't
> (except in the very special case that the reverse link existed).  I
> just tried to sanity-check the existing links.
> 
> I agree that it would be nice to look for missing links that should
> be added ... but that is a performance enhancement, not a bug fix,
> so I am not eager to do it this close to release.  We should do
> another pass over this table after 6.4 is out the door.
> 
> (Another reason I didn't try to do that is that I've got no good
> idea how to find missing links, short of brain-numbingly tedious
> hand examination.  Can you suggest any automated way of finding
> missing links, or at least finding likely things to look at?)
> 
>             regards, tom lane

Well, I have to admit, I did the brain-numbingly tedious hand examination.
But, mostly it is straight forward. If you have op 'lt' for type foo as
function foolt, then it probably has negetor 'ge' as function 'fooge'.

-dg

David Gould            dg@informix.com           510.628.3783 or 510.305.9468 
Informix Software  (No, really)         300 Lakeside Drive  Oakland, CA 94612- If simplicity worked, the world would be
overrunwith insects. -
 


Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Yes.  See include/catalog/template1_check.sql.  Uses SQL to do checks.

Hmm.  OK, that script has some overlap with what I did last night.
But is it run automatically?  I think putting the checks into a
regression test is a better plan.

Also, in playing around with template1_check.sql, I discovered that
pg_operator OID 644

DATA(insert OID = 644 (  "<>"       PGUID 0 b t f  30  30  16 644 649  0  0 oid8ne neqsel neqjoinsel ));

points to a nonexistent operator --- there is no oid8ne in pg_proc, nor
in the code... anyone feel like fixing it?
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
Bruce Momjian
Дата:
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > Yes.  See include/catalog/template1_check.sql.  Uses SQL to do checks.
> 
> Hmm.  OK, that script has some overlap with what I did last night.
> But is it run automatically?  I think putting the checks into a
> regression test is a better plan.

You were right, template1_check.sql is generated by
pgsql/contrib/findoidjoins/make_oidjoin_check.

It is not run automatically, but should be, just as duplicate_oids
should be.

> 
> Also, in playing around with template1_check.sql, I discovered that
> pg_operator OID 644
> 
> DATA(insert OID = 644 (  "<>"       PGUID 0 b t f  30  30  16 644 649  0  0 oid8ne neqsel neqjoinsel ));
> 
> points to a nonexistent operator --- there is no oid8ne in pg_proc, nor
> in the code... anyone feel like fixing it?

Added.

--  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,
Pennsylvania19026
 


Re: [HACKERS] Another bug in pg_operator.h

От
"Thomas G. Lockhart"
Дата:
> points to a nonexistent operator --- there is no oid8ne in pg_proc, 
> nor in the code... anyone feel like fixing it?

You de man! I just committed inet regression test fixes and resolved the
"@" operator problem. I'm back to working on docs for now...
                 - Tom

actually, at the moment I'm back to going to work :)


Re: [HACKERS] Another bug in pg_operator.h

От
Tom Lane
Дата:
"Thomas G. Lockhart" <lockhart@alumni.caltech.edu> writes:
> I just committed inet regression test fixes and resolved the
> "@" operator problem.

Great!  I think that means it's safe to put out a BETA4, Marc.

Leastwise I promise to stop breaking things for a while ;-)
        regards, tom lane


Re: [HACKERS] Another bug in pg_operator.h

От
The Hermit Hacker
Дата:
On Thu, 29 Oct 1998, Tom Lane wrote:

> "Thomas G. Lockhart" <lockhart@alumni.caltech.edu> writes:
> > I just committed inet regression test fixes and resolved the
> > "@" operator problem.
> 
> Great!  I think that means it's safe to put out a BETA4, Marc.
Done, BETA4 is rolled...

Marc G. Fournier                                
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org