Bug or stupidity

Collapse
This topic is closed.
X
X
 
  • Time
  • Show
Clear All
new posts
  • Philip Hofstetter

    Bug or stupidity

    Hello,

    I think, I found a bug, but maybe it's just my stupidity. Granted: What
    I did was an error on my part, but I still think, PostgreSQL should not
    do what it does.

    I've already created a simple testcase:


    popscan_light=> create table a (id serial, name varchar(10), primary
    key(id)) without oids;
    NOTICE: CREATE TABLE will create implicit sequence "a_id_seq" for
    "serial" column "a.id"
    NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "a_pkey"
    for table "a"
    CREATE TABLE
    popscan_light=> create table b (id int4 references a (id) on delete
    cascade, name2 varchar(15), primary key (id)) without oids;
    NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "b_pkey"
    for table "b"
    CREATE TABLE
    popscan_light=> insert into a (name) values ('gnegg');
    INSERT 0 1
    popscan_light=> insert into a (name) values ('blepp');
    INSERT 0 1
    popscan_light=> insert into b values (1, 'gnegglink');
    INSERT 0 1
    popscan_light=> insert into b values (2, 'blepplink');
    INSERT 0 1
    popscan_light=> select a.name, b.name2 from a left join b using (id)
    order by b.name2;
    name | name2
    -------+-----------
    blepp | blepplink
    gnegg | gnegglink
    (2 rows)

    popscan_light=> select aliasa.name, aliasb.name2 from a aliasa left join
    b aliasb using (id) order by b.name2;
    NOTICE: adding missing FROM-clause entry for table "b"
    name | name2
    -------+-----------
    gnegg | gnegglink
    blepp | blepplink
    gnegg | gnegglink
    blepp | blepplink
    (4 rows)

    popscan_light=> select aliasa.name, aliasb.name2 from a aliasa left join
    b aliasb using (id) order by aliasb.name2;
    name | name2
    -------+-----------
    blepp | blepplink
    gnegg | gnegglink
    (2 rows)

    popscan_light=> \q
    fangorn ~ $ psql --version
    psql (PostgreSQL) 7.4.3
    contains support for command-line editing

    In the second "SELECT"-Query I've ordered the result set by the
    name-column of the second table, but I have not used the alias "aliasb"
    I created, but I used the full table name. I know this is not really
    correct, but I'd still like to know why Postgres throws 4 results at me.

    If I use the correct column in the order by clause, I get the correctly
    joined result.

    Looking at my second query, I think the false "order by" seems to pull
    in another copy of table b joining it without a proper condition. I
    don't think, this is the right thing to do.

    Or ist it?

    Anyone?

    Philip

    ---------------------------(end of broadcast)---------------------------
    TIP 9: the planner will ignore your desire to choose an index scan if your
    joining column's datatypes do not match

  • Martijn van Oosterhout

    #2
    Re: Bug or stupidity

    On Sat, Oct 23, 2004 at 02:17:16PM +0000, Philip Hofstetter wrote:[color=blue]
    > Hello,
    >
    > I think, I found a bug, but maybe it's just my stupidity. Granted: What
    > I did was an error on my part, but I still think, PostgreSQL should not
    > do what it does.[/color]

    .... snip ...
    [color=blue]
    > popscan_light=> select aliasa.name, aliasb.name2 from a aliasa left join
    > b aliasb using (id) order by b.name2;
    > NOTICE: adding missing FROM-clause entry for table "b"
    > name | name2
    > -------+-----------
    > gnegg | gnegglink
    > blepp | blepplink
    > gnegg | gnegglink
    > blepp | blepplink
    > (4 rows)[/color]

    See that NOTIVCE? It's telling you that it's converted your query to:

    select aliasa.name, aliasb.name2 from b, a aliasa left join
    b aliasb using (id) order by b.name2;

    Since you now have an unconstrained join on the B table, you get twice
    as many rows.

    It basically comes down to, if you make an alias, you have to use the
    alias. You can't use the original table name *and* the alias. The
    reference to the original table is becomes another copy of the same
    table.

    As for what's SQL standard, I think by a strict definition your query
    shouldn't be allowed at all, referencing an undefined table.

    Hope this helps,
    --
    Martijn van Oosterhout <kleptog@svana. org> http://svana.org/kleptog/[color=blue]
    > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
    > tool for doing 5% of the work and then sitting around waiting for someone
    > else to do the other 95% so you can sue them.[/color]

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.0.6 (GNU/Linux)
    Comment: For info see http://www.gnupg.org

    iD8DBQFBemr/Y5Twig3Ge+YRAlI YAKC6DjO0oikssf d+WdC0LIQTlngvx QCeMntm
    jtYktqJ/FuAfqLShKD7Sga0 =
    =xKg9
    -----END PGP SIGNATURE-----

    Comment

    • Philip Hofstetter

      #3
      Re: Bug or stupidity

      Hi,

      Martijn van Oosterhout wrote:[color=blue][color=green]
      >>popscan_light => select aliasa.name, aliasb.name2 from a aliasa left join
      >>b aliasb using (id) order by b.name2;
      >>NOTICE: adding missing FROM-clause entry for table "b"
      >> name | name2
      >>-------+-----------
      >> gnegg | gnegglink
      >> blepp | blepplink
      >> gnegg | gnegglink
      >> blepp | blepplink
      >>(4 rows)[/color]
      >
      >
      > See that NOTIVCE? It's telling you that it's converted your query to:[/color]

      actually, I've overseen it. But then, my assumption in my mail was
      correct anyway.
      [color=blue]
      > select aliasa.name, aliasb.name2 from b, a aliasa left join
      > b aliasb using (id) order by b.name2;[/color]
      [color=blue]
      > Since you now have an unconstrained join on the B table, you get twice
      > as many rows.[/color]

      This is what I thought.
      [color=blue]
      > As for what's SQL standard, I think by a strict definition your query
      > shouldn't be allowed at all, referencing an undefined table.[/color]

      This is exactly what I think too. I mean: I know I made an error in my
      query. It would just have been easier to find if PostgreSQL actually had
      told me so (I'm not getting those NOTICEs from PHP...).

      If it's wrong, it should be disallowed, not made worse by assuming a
      completely wrong thing.

      Thanks for your fast response anyway.

      Philip

      ---------------------------(end of broadcast)---------------------------
      TIP 1: subscribe and unsubscribe commands go to majordomo@postg resql.org

      Comment

      • Martijn van Oosterhout

        #4
        Re: Bug or stupidity

        On Sat, Oct 23, 2004 at 02:35:20PM +0000, Philip Hofstetter wrote:[color=blue][color=green]
        > >As for what's SQL standard, I think by a strict definition your query
        > >shouldn't be allowed at all, referencing an undefined table.[/color]
        >
        > This is exactly what I think too. I mean: I know I made an error in my
        > query. It would just have been easier to find if PostgreSQL actually had
        > told me so (I'm not getting those NOTICEs from PHP...).
        >
        > If it's wrong, it should be disallowed, not made worse by assuming a
        > completely wrong thing.[/color]

        Maybe, ofcourse, this exact same construct is used heavily in DELETEs.
        Look at the syntax of the delete command:

        DELETE FROM [ ONLY ] table [ WHERE condition ]

        You can't declare extra tables or define aliases. Every other table
        used in the query is by strict definitions "undefined" . Should they all
        be declared illegal too?

        Perhaps you could argue that using undeclared tables is allowed for
        DELETEs but not for SELECTs. But why make a distiction if you don't
        need to.

        Hope this helps,

        --
        Martijn van Oosterhout <kleptog@svana. org> http://svana.org/kleptog/[color=blue]
        > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
        > tool for doing 5% of the work and then sitting around waiting for someone
        > else to do the other 95% so you can sue them.[/color]

        -----BEGIN PGP SIGNATURE-----
        Version: GnuPG v1.0.6 (GNU/Linux)
        Comment: For info see http://www.gnupg.org

        iD8DBQFBem2yY5T wig3Ge+YRAqrsAJ 0Vt8H67qHEBFreX ltZCMxGUB0W2ACg yLCj
        i6U+ec/FqTlIqGPoFPaQ32 E=
        =4GA1
        -----END PGP SIGNATURE-----

        Comment

        • Stephan Szabo

          #5
          Re: Bug or stupidity

          On Sat, 23 Oct 2004, Philip Hofstetter wrote:
          [color=blue][color=green]
          > > As for what's SQL standard, I think by a strict definition your query
          > > shouldn't be allowed at all, referencing an undefined table.[/color]
          >
          > This is exactly what I think too. I mean: I know I made an error in my
          > query. It would just have been easier to find if PostgreSQL actually had
          > told me so (I'm not getting those NOTICEs from PHP...).
          >
          > If it's wrong, it should be disallowed, not made worse by assuming a
          > completely wrong thing.[/color]

          It's enabled in large part for backwards compatibility. There's a runtime
          option that controls the behavior (add_missing_fr om).


          ---------------------------(end of broadcast)---------------------------
          TIP 3: if posting/reading through Usenet, please send an appropriate
          subscribe-nomail command to majordomo@postg resql.org so that your
          message can get through to the mailing list cleanly

          Comment

          • Karim Nassar

            #6
            Re: Bug or stupidity

            On Sat, 2004-10-23 at 07:35, Philip Hofstetter wrote:[color=blue]
            > <snip> It would just have been easier to find if PostgreSQL actually had
            > told me so (I'm not getting those NOTICEs from PHP...).[/color]

            As far as I can tell, Apache or PHP snarfs up all the messages that
            postgres generates before they can get to the postgres log.

            In order to see them, these are my entries from php.ini:

            1. error_reporting = E_ALL
            2. display_errors = Off
            3. log_errors = On
            4. log_errors_max_ len = 2048

            In english:

            1. Every freakin message you see
            2. don't put em on the web page
            3. just my log file
            4. and show me all my long queries.

            On my system, everything ends up in apache's error_log.

            HTH,
            \<.


            ---------------------------(end of broadcast)---------------------------
            TIP 9: the planner will ignore your desire to choose an index scan if your
            joining column's datatypes do not match

            Comment

            • Thomas Hallgren

              #7
              Re: Bug or stupidity

              Stephan Szabo wrote:
              [color=blue]
              > It's enabled in large part for backwards compatibility. There's a[/color]
              runtime[color=blue]
              > option that controls the behavior (add_missing_fr om).
              >[/color]
              IMHO, it would be a more natural choice to have the add_missing_fro m
              disabled by default. Why would anyone *ever* want faulty SQL being
              magically "patched up" by the dbms?

              Ok, so some older installations might break when this is changed but the
              option is still there. Let applications that depend on this somewhat
              magical behavior enable it rather than have everyone else potentially
              run into the same problem as Philip.

              Regards,

              Thomas Hallgren


              ---------------------------(end of broadcast)---------------------------
              TIP 3: if posting/reading through Usenet, please send an appropriate
              subscribe-nomail command to majordomo@postg resql.org so that your
              message can get through to the mailing list cleanly

              Comment

              • Steven Klassen

                #8
                Re: Bug or stupidity

                * Thomas Hallgren <thhal@mailbloc ks.com> [2004-10-25 15:52:20 +0200]:
                [color=blue]
                > IMHO, it would be a more natural choice to have the add_missing_fro m
                > disabled by default. Why would anyone *ever* want faulty SQL being
                > magically "patched up" by the dbms?[/color]

                That assumes that developers will implement queries in their code
                without testing them. Unfortunately, that's probably not too far from
                reality. I've thought of it as a nice "debugging" feature while I'm
                trying to hammer out a complicated query for the first time.

                --
                Steven Klassen - Lead Programmer
                Command Prompt, Inc. - http://www.commandprompt.com/
                PostgreSQL Replication & Support Services, (503) 667-4564

                ---------------------------(end of broadcast)---------------------------
                TIP 7: don't forget to increase your free space map settings

                Comment

                • Stephan Szabo

                  #9
                  Re: Bug or stupidity

                  On Mon, 25 Oct 2004, Thomas Hallgren wrote:
                  [color=blue]
                  > Stephan Szabo wrote:
                  >[color=green]
                  > > It's enabled in large part for backwards compatibility. There's a[/color]
                  > runtime[color=green]
                  > > option that controls the behavior (add_missing_fr om).
                  > >[/color]
                  > IMHO, it would be a more natural choice to have the add_missing_fro m
                  > disabled by default. Why would anyone *ever* want faulty SQL being[/color]

                  In general, when we add a backwards compatibility option, we give a couple
                  of versions before the default is changed. In addition, until we have a
                  form of delete which allows a "from" list, there are some queries which
                  are really more naturally written in a form similar to add_missing_fro m
                  (although "from" lists would be better).
                  [color=blue]
                  > magically "patched up" by the dbms?[/color]

                  I think that many people do, even if they don't realize it. Pretty much
                  almost any extension to the spec is faulty SQL, from != and use of column
                  aliases in some places they technically aren't allowed to DISTINCT ON and
                  UPDATE FROM.

                  ---------------------------(end of broadcast)---------------------------
                  TIP 9: the planner will ignore your desire to choose an index scan if your
                  joining column's datatypes do not match

                  Comment

                  • Thomas Hallgren

                    #10
                    Re: Bug or stupidity

                    Steven,
                    [color=blue]
                    > That assumes that developers will implement queries in their code
                    > without testing them. Unfortunately, that's probably not too far from
                    > reality. I've thought of it as a nice "debugging" feature while I'm
                    > trying to hammer out a complicated query for the first time.[/color]


                    I don't see how that makes a difference really. As a developer, I'd
                    rather prefer if I get an explanatory error result rather than a notice
                    (often invisible) and an incorrect result when testing. If I don't test
                    at all (God forbid) I want the same thing to happen the first time the
                    code is deployed. Anything else is really scary. I don't see how it can
                    be the dbms responsibility to correct erroneous SQL ever. It's
                    comparable to having a compiler that magically adds undeclared (or
                    misspelled) variables in your code. Shrug...

                    Is the variable settable in a session? If so, that would be good for the
                    purpose you mention.

                    Regards,
                    Thomas Hallgren


                    ---------------------------(end of broadcast)---------------------------
                    TIP 1: subscribe and unsubscribe commands go to majordomo@postg resql.org

                    Comment

                    • Steven Klassen

                      #11
                      Re: Bug or stupidity

                      * Thomas Hallgren <thhal@mailbloc ks.com> [2004-10-25 19:06:40 +0200]:
                      [color=blue]
                      > I don't see how that makes a difference really.[/color]

                      /me notes the timestamp on his post and vows never to post before 8am
                      again.

                      --
                      Steven Klassen - Lead Programmer
                      Command Prompt, Inc. - http://www.commandprompt.com/
                      PostgreSQL Replication & Support Services, (503) 667-4564

                      ---------------------------(end of broadcast)---------------------------
                      TIP 2: you can get off all lists at once with the unregister command
                      (send "unregister YourEmailAddres sHere" to majordomo@postg resql.org)

                      Comment

                      • Thomas Hallgren

                        #12
                        Re: Bug or stupidity

                        Stephan,
                        [color=blue]
                        > In general, when we add a backwards compatibility option, we give
                        > a couple of versions before the default is changed.
                        >[/color]
                        Perhaps the 8.0 would be a perfect time since it's a change of the major
                        number.
                        [color=blue]
                        > In addition, until we have a form of delete which allows a "from"
                        > list, there are some queries which are really more naturally written
                        > in a form similar to add_missing_fro m
                        > (although "from" lists would be better).
                        >[/color]
                        Still, if the query is incorrect, I want to know about it. I don't ever
                        want an incorrect behavior as a result of some behind the scenes magic.
                        For me, there's no exception to that rule and my guess is that very few
                        people would disagree if they think about it more in depth. This option
                        helps no one. It only adds to the confusion.
                        [color=blue]
                        > I think that many people do, even if they don't realize it.
                        >[/color]
                        If people write incorrect SQL because "this looks like the natural way
                        of doing it", don't you think it's fair if they find out about the error
                        ASAP? Catching errors early in the development process is generally
                        considered a good thing. When this option is enabled, errors might be
                        hidden (you get the notification that not everyone will pay attention
                        to, or even see). I consider that a very *bad* thing.

                        It's perhaps OK that the option exists so that old legacy system can
                        keep on running, but to have it enabled by default is not good at all.

                        Regards,
                        Thomas Hallgren


                        ---------------------------(end of broadcast)---------------------------
                        TIP 4: Don't 'kill -9' the postmaster

                        Comment

                        • Mike Mascari

                          #13
                          Re: Bug or stupidity

                          Thomas Hallgren wrote:[color=blue]
                          > Steven,
                          >[color=green]
                          > > That assumes that developers will implement queries in their code
                          > > without testing them. Unfortunately, that's probably not too far from
                          > > reality. I've thought of it as a nice "debugging" feature while I'm
                          > > trying to hammer out a complicated query for the first time.[/color]
                          >
                          > I don't see how that makes a difference really. As a developer, I'd
                          > rather prefer if I get an explanatory error result rather than a notice
                          > (often invisible) and an incorrect result when testing. If I don't test
                          > at all (God forbid) I want the same thing to happen the first time the
                          > code is deployed.[/color]

                          This particular error may be less than obvious even during crude testing
                          because the number of tuples in the relation in question may be zero or
                          one, so the cross-product wouldn't produce anything unusual.

                          Mike Mascari

                          ---------------------------(end of broadcast)---------------------------
                          TIP 1: subscribe and unsubscribe commands go to majordomo@postg resql.org

                          Comment

                          • Stephan Szabo

                            #14
                            Re: Bug or stupidity

                            On Mon, 25 Oct 2004, Thomas Hallgren wrote:
                            [color=blue]
                            > Stephan,
                            >[color=green]
                            > > In general, when we add a backwards compatibility option, we give
                            > > a couple of versions before the default is changed.
                            > >[/color]
                            > Perhaps the 8.0 would be a perfect time since it's a change of the major
                            > number.[/color]

                            Maybe, but I think it'll be a hard sell without a replacement for the
                            delete form that works when it's off.
                            [color=blue][color=green]
                            > > In addition, until we have a form of delete which allows a "from"
                            > > list, there are some queries which are really more naturally written
                            > > in a form similar to add_missing_fro m
                            > > (although "from" lists would be better).
                            > >[/color]
                            > Still, if the query is incorrect, I want to know about it. I don't ever[/color]

                            But, is the query incorrect? It does what PostgreSQL says it will.
                            That's not what the spec says it'll do, but the same is true of most of
                            the extensions, and I don't think people generally consider queries using
                            those as incorrect.

                            ---------------------------(end of broadcast)---------------------------
                            TIP 6: Have you searched our list archives?



                            Comment

                            • Martijn van Oosterhout

                              #15
                              Re: Bug or stupidity

                              On Tue, Oct 26, 2004 at 05:25:57PM +0200, Thomas Hallgren wrote:[color=blue]
                              > If the WHERE clause that defines the criteria for deletion involves more
                              > than one table, then you'd use a sub select and that has a FROM clause
                              > of its own.[/color]

                              Sure, that's what you could do, but it makes the query rather more
                              complex than it needs to be. For example, consider this statement:

                              DELETE FROM x WHERE x.a = table.a and x.b > table.b and table.c = 4;

                              Look, "table" is not declared anywhere, but I can't think of a way to
                              rewrite this in strict SQL. Maybe:

                              DELETE FROM x WHERE x.id IN
                              (SELECT x.id FROM x, table where x.a = table.a and x.b > table.b and table.c = 4);

                              Seems like a lot of extra work for no gain. Hope id is never NULL.
                              Maybe EXISTS would work better?
                              [color=blue]
                              > I haven't seen any other extension that, when enabled, attempts to
                              > "improve" badly written SQL in a way that potentially gives incorrect
                              > query results. As I said in another post, this is like having a compiler
                              > that instead of complaining about a misspelled variable, adds a new one.[/color]

                              transform_equal s_null comes to mind. It's a hack to make 'x = NULL'
                              work the way people coming from Oracle expect. It "fixes" it to be 'x
                              IS NULL'.

                              That is arguably something that could cause unexpected results.
                              [color=blue]
                              > So all your tests run fine. You ship to your customers. The customers
                              > starts adding data to tables and finds some strange behavior. It turns
                              > out that everything is caused by tables being added to the FROM clause.
                              > You didn't see the problem in your test because there, the added table
                              > had less than 2 tuples in it.[/color]

                              It has to be exactly one tuple. If there are zero tuples you get zero
                              output. Cross-joining with an empty table produces no output. You're
                              shipping a product where people expect to be able to add more rows to a
                              table, but you never test that?
                              [color=blue]
                              > As I said before, I don't object to the presence of this "option" so
                              > that people that really knows _why_ they enable it can do so, but I
                              > strongly object to having this option enabled by default. I suggest that:
                              >
                              > 1. Have this option disabled by default.
                              > 2. Print WARNING's rather than notifications when tables are added.[/color]

                              If you're not seeing NOTICEs now, what makes you think you'll see
                              WARNINGs? Every DB interface I've used so far displays the notices
                              where I can see them. This notice is one of the less useful, there
                              are other more useful warnings which are much more handy to see...
                              --
                              Martijn van Oosterhout <kleptog@svana. org> http://svana.org/kleptog/[color=blue]
                              > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
                              > tool for doing 5% of the work and then sitting around waiting for someone
                              > else to do the other 95% so you can sue them.[/color]

                              -----BEGIN PGP SIGNATURE-----
                              Version: GnuPG v1.0.6 (GNU/Linux)
                              Comment: For info see http://www.gnupg.org

                              iD8DBQFBfnK5Y5T wig3Ge+YRAg+zAK DPKCFYbgT8lRqsE 9jlg+Uwu48iGQCf bL3l
                              p9CACUptj0Olemr PzOYnYko=
                              =wiqT
                              -----END PGP SIGNATURE-----

                              Comment

                              Working...