goodybag / mongo-sql

An extensible SQL generation library for JavaScript with a focus on introspectibility
321 stars 72 forks source link

Alias in sub query bug #150

Closed PaulGrimshaw closed 6 years ago

PaulGrimshaw commented 7 years ago

There appears to be a problem with alias in a sub query:

Example below:

let moQuery = {
    type:  "select",
    table: {
        type: "select",
        table: "myTable",
        alias: "mt"
    },
    alias: "mt2"
}
console.log(builder.sql(moQuery).toString())

Result is:

select "mt2".* from (select "myTable".* from "myTable") "mt" "mt2"

But should be:

select "mt2".* from (select "mt".* from "myTable" "mt") "mt2"

PaulGrimshaw commented 7 years ago

Digging into the source, this seems to be a problem with the way sub query aliases are handled. The libarary seems to expect the alias to be defined within the sub query, where logically this should be outside the sub query.

This can be seen on the existing sub query test (/test/select.js):

it ('allow sub-queries on table', function(){
      var query = builder.sql({
        type: 'select'
      , table: {
          type: 'select'
        , table: 'users'
        , alias: 'u'
        }
      });

      assert.equal(
        query.toString()
      , 'select "u".* from (select "users".* from "users") "u"'
      );
    });

The "alias: 'u'" part of the query should be moved up a level in my opinion, and if it is defined where it is that alias should apply within the sub query, not outside.

jrf0110 commented 7 years ago

@PaulGrimshaw Whenever I implemented this way back when, I had a very specific use-case. I had never covered the case of needing to also alias the inner-most query. So table aliases refer to the context in which they are embedded, which I realize is counter-intuitive:

See http://mosql.j0.hn/#/snippets/1d

I'd be happy to accept a PR that makes this behave like you posted (and we minor or major bump accordingly).

I'd start looking here:

I apologize in advance for the messy code :)

PaulGrimshaw commented 7 years ago

I've submitted a pull request with the changes. Updated the tests, but not the docs... wasn't sure how to find the relevant docs to update.