Insert query builder allows to use table alias in INSERT ... SELECT ... construction about database HOT 17 CLOSED

@kwn thanks for the very clear bug report! @enov not sure how/where to triage this.

from database.

Should be the else body in Kohana_Database_Query_Builder_Insert::compile()

from database.

@shadowhand It should go to 3.4/develop because it adds an exception to the method.

@kemo is this where the fix should go? I did not try this, but my understanding is that INSERT statements does not support alias for tables (except for T-SQL). The $this->_table in the code excerpt @kwn quoted belongs to the INSERT statement (not subsequent SELECT queries). Better to throw the exception earlier (and it should be a Kohana_Exception).

So if you don't mind, @kwn, kindly elaborate more about how did you build your query and the SQL syntax error you received. My argument is that this is a bug that might affect any INSERT statement, not specifically an INSERT INTO ... SELECT construct, but I might be wrong.

from database.

@enov I was just adding the reference to what @kwn is aiming at, I agree it can be done elsewhere.

Also, Kohana_Database_Query_Builder_Insert::table() isn't used to set the table name in constructor so if the check exception will be in that method - it might be clever to start using it in the constructor just to keep things consistent.

from database.

Also, Kohana_Database_Query_Builder_Insert::table() isn't used to set the table name in constructor so if the check exception will be in that method - it might be clever to start using it in the constructor just to keep things consistent.

Hmm, that's a good catch @kemo .

from database.

Hi guys

@enov I'll just put a test that I created for query builder module I'm creating. You'll find what's wrong. I abandoned Kohana framework a couple of months ago, and I found this bug by accident while extracting query builder from database module. Wish you all the best with Kohana reactivation:)

    public function testExceptionWhenUsingTableAliasInInsertSelectConstruction()
    {
        $qb = new \Quark\Database\Query\Builder\Select(array(
            'u.name'  => 'name',
            'u.email' => 'email'
        ));

        $select = $qb
            ->from(array('users', 'u'))
            ->join(array('posts', 'p'), 'LEFT')
            ->on('p.user_id', '=', 'u.id')
            ->where('u.name', '=', 'test')
            ->having_open()
            ->having('u.age', '>', '10')
            ->or_having('u.age', '<', '14')
            ->having_close()
            ->order_by('u.age', 'DESC');

        try {
            $query = $this
                ->queryBuilder
                ->table(array('users', 'u'))
                ->columns(array('u.name', 'u.email'))
                ->select($select)
                ->compile();

            $this->assertTrue(false, 'Exception not thrown');
        } catch (\Quark\Exception\QuarkException $e) {
            $this->assertTrue(true, 'Exception thrown');
        }
    }

ps. I know that this query doesn't have any sense. I'm just checking string compilation correctness.

from database.

Assuming that:

• \Quark\Database\Query\Builder\Select has same API as Database_Query_Builder_Select
• and later $this->queryBuilder is a Database_Query_Builder_Insert, or compatible

I made the following query based on yours:

        $select = DB::select(array(
              'u.name' => 'name',
              'u.email' => 'email'
          ))->from(array('users', 'u'))
          ->join(array('posts', 'p'), 'LEFT')
          ->on('p.user_id', '=', 'u.id')
          ->where('u.name', '=', 'test')
          ->having_open()
          ->having('u.age', '>', '10')
          ->or_having('u.age', '<', '14')
          ->having_close()
          ->order_by('u.age', 'DESC');


        $insert = DB::insert()
          ->table(array('users', 'u'))
          ->columns(array('u.name', 'u.email'))
          ->select($select);

• If I do echo $insert->compile() I receive:

INSERT INTO users AS u (u.name, u.email) ErrorException [ 8 ]: Undefined offset: 1 ~ MODPATH/database/classes/Kohana/Database.php [ 507 ]

• If I do echo $select->compile() I receive:
  

So, it seems the problem is from the select statement.

Referring to the manual, it says:

It is also possible to create AS aliases when selecting, by passing an array as each parameter

http://kohanaframework.org/3.3/guide/database/query/builder#select-as-column-aliases

If I change the first part of select to DB::select(array('u.name', 'name'), array('u.email', 'email')), I will have this:

        $select = DB::select(
              array('u.name', 'name'),
              array('u.email', 'email')
          )->from(array('users', 'u'))
          ->join(array('posts', 'p'), 'LEFT')
          ->on('p.user_id', '=', 'u.id')
          ->where('u.name', '=', 'test')
          ->having_open()
          ->having('u.age', '>', '10')
          ->or_having('u.age', '<', '14')
          ->having_close()
          ->order_by('u.age', 'DESC');


        $insert = DB::insert()
          ->table(array('users', 'u'))
          ->columns(array('u.name', 'u.email'))
          ->select($select);

This will correctly compile to:

INSERT INTO users AS u (u.name, u.email) SELECT u.name AS name, u.email AS email FROM users AS u LEFT JOIN posts AS p ON (p.user_id = u.id) WHERE u.name = 'test' HAVING (u.age > '10' OR u.age < '14') ORDER BY u.age DESC

So basically, I guess that's your issue. You need to pass an array for each parameter.

Now, It will correctly compile... but I guess there is the other SQL bug regarding INSERT statements that should not have aliases.

Let me know if I followed you correctly.

from database.

@kwn could you confirm if the issue is settled when you pass an array for each parameter?

@kemo do we need to do something regarding the aliasing of an INSERT table? It's legit for T-SQL and throws a database exception after all, in case of other providers.

from database.

@enov
Yep, you are right.

but I guess there is the other SQL bug regarding INSERT statements that should not have aliases.

True, as well.

from database.

Thanks for the confirmation @kwn .

@kemo could you share your thoughts regarding INSERT table whether we should allow aliasing? Or maybe we should throw an exception?

Should I open a separate issue for that?

from database.

@enov I wouldn't allow aliasing, agreed that exception makes more sense there

from database.

Alright, @kemo.

Do you have time for a PR or you think I should do it?
(yea, yea... that question you just knew coming... 😜 )

from database.

@kemo, would you mind to review #84. I did not mean to pressure you after all. Thanks!

from database.

@enov just saw this mate, I wouldn't mind sending the fix

from database.

Closed by #84

from database.

The same situation is in Delete::table() method guys.

from database.

It seems Postgres supports delete table aliasing:

http://www.postgresql.org/docs/9.3/static/sql-delete.html

from database.