Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Database: explicitly define table in where clause to make queries unambiguous #605

Closed
wants to merge 1 commit into from

3 participants

@juzna

Problem explained:

I've got these three tables joined by foreign keys:

DB schema

When I want to select requests added by me which belong to a movie (transitive dependence via subtitles), I would use:
$this->db->table('requests')->where(array('addedBy' => $userId, 'subtitle.movieId' => $movieId)).
Look at this command. It is obvious that I want to have a condition on addedBy column in requests table and you probably won't even realize there is another column of the same name. And even if you do so, that column is somewhere else and I'm not using it in the db command at all. So I think this command is absolutely clear.

However, Nette Db will fail because of issuing ambiguous SQL query to the database:
SELECT requests.* FROM requests INNER JOIN subtitles AS subtitle ON requests.subtitleid = subtitle.id WHERE (addedBy = ?) AND (subtitle.movieId = ?). Notice the WHERE clause - it says addedBy which is ambiguous for sql server.

I believe Nette Db should make it clear and tell the database what we wanted, but it has to translate it into the database language. Therefore it must make the sql query unambiguous, as what we said to Nette Db was clear and unambiguous. Therefore the sql query should be:
SELECT requests.* FROM requests INNER JOIN subtitles AS subtitle ON requests.subtitleid = subtitle.id WHERE (requests.addedBy = ?) AND (subtitle.movieId = ?) (explicitly define the table name in where clause).

(Discussion expected)

@hrach

Awesome, please add test. :)

@hrach

maybe use # as delimeter since it's used across the framework.

@juzna

I took a shower and looked at it again, and perhaps it's not solved in the best place. Explicit table name in SQL is not really needed when one selects only from one table. Also, decision whether the table name is needed in generated SQL query should probably be within getSql method (or its callees).

@icaine

what if you pass as a condition something like "column < ? OR column > ?"?

@juzna juzna closed this
@juzna

@icaine you're right, that would break it. I don't have any proper solution.

@juzna juzna referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@juzna juzna referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@juzna juzna referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 0 deletions.
  1. +3 −0  Nette/Database/Table/Selection.php
View
3  Nette/Database/Table/Selection.php
@@ -220,6 +220,9 @@ public function where($condition, $parameters = array())
$this->conditions[$hash] = $condition;
$condition = $this->removeExtraTables($condition);
+ if (!preg_match('~[.:]~', $condition)) {
+ $condition = "$this->name.$condition";
+ }
$condition = $this->tryDelimite($condition);
$args = func_num_args();
Something went wrong with that request. Please try again.