Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce From class in QueryBuilder #3833

Merged
merged 1 commit into from
Jan 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Upgrade to 3.0

## BC BREAK: `QueryBuilder::insert()`, `update()` and `delete()` signatures changed

These methods now require the `$table` parameter, and do not support aliases anymore.

## BC BREAK: `OCI8Statement::convertPositionalToNamedPlaceholders()` is removed.

The `OCI8Statement::convertPositionalToNamedPlaceholders()` method has been extracted to an internal utility class.
Expand Down
23 changes: 23 additions & 0 deletions lib/Doctrine/DBAL/Query/From.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Query;

/**
* @internal
*/
final class From
{
/** @var string */
public $table;

/** @var string|null */
public $alias;

public function __construct(string $table, ?string $alias = null)
{
$this->table = $table;
$this->alias = $alias;
}
}
83 changes: 21 additions & 62 deletions lib/Doctrine/DBAL/Query/QueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class QueryBuilder
'select' => [],
'distinct' => false,
'from' => [],
'table' => null,
'join' => [],
'set' => [],
'where' => null,
Expand Down Expand Up @@ -421,7 +422,7 @@ public function add(string $sqlPartName, $sqlPart, bool $append = false) : self
$this->state = self::STATE_DIRTY;

if ($append) {
if ($sqlPartName === 'orderBy' || $sqlPartName === 'groupBy' || $sqlPartName === 'select' || $sqlPartName === 'set') {
if ($sqlPartName === 'orderBy' || $sqlPartName === 'groupBy' || $sqlPartName === 'select' || $sqlPartName === 'set' || $sqlPartName === 'from') {
morozov marked this conversation as resolved.
Show resolved Hide resolved
foreach ($sqlPart as $part) {
$this->sqlParts[$sqlPartName][] = $part;
}
Expand Down Expand Up @@ -528,23 +529,15 @@ public function addSelect($select = null) : self
* ->setParameter(':user_id', 1);
* </code>
*
* @param string $delete The table whose rows are subject to the deletion.
* @param string $alias The table alias used in the constructed query.
* @param string $table The table whose rows are subject to the deletion.
*
* @return $this This QueryBuilder instance.
*/
public function delete(?string $delete = null, ?string $alias = null) : self
public function delete(string $table) : self
{
$this->type = self::DELETE;

if (! $delete) {
return $this;
}

return $this->add('from', [
'table' => $delete,
'alias' => $alias,
]);
return $this->add('table', $table);
}

/**
Expand All @@ -558,23 +551,15 @@ public function delete(?string $delete = null, ?string $alias = null) : self
* ->where('c.id = ?');
* </code>
*
* @param string $update The table whose rows are subject to the update.
* @param string $alias The table alias used in the constructed query.
* @param string $table The table whose rows are subject to the update.
*
* @return $this This QueryBuilder instance.
*/
public function update(?string $update = null, ?string $alias = null) : self
public function update(string $table) : self
{
$this->type = self::UPDATE;

if (! $update) {
return $this;
}

return $this->add('from', [
'table' => $update,
'alias' => $alias,
]);
return $this->add('table', $table);
}

/**
Expand All @@ -592,19 +577,15 @@ public function update(?string $update = null, ?string $alias = null) : self
* );
* </code>
*
* @param string $insert The table into which the rows should be inserted.
* @param string $table The table into which the rows should be inserted.
*
* @return $this This QueryBuilder instance.
*/
public function insert(?string $insert = null) : self
public function insert(string $table) : self
{
$this->type = self::INSERT;

if (! $insert) {
return $this;
}

return $this->add('from', ['table' => $insert]);
return $this->add('table', $table);
}

/**
Expand All @@ -624,10 +605,7 @@ public function insert(?string $insert = null) : self
*/
public function from(string $from, ?string $alias = null)
{
return $this->add('from', [
'table' => $from,
'alias' => $alias,
], true);
return $this->add('from', new From($from, $alias), true);
}

/**
Expand Down Expand Up @@ -1125,17 +1103,14 @@ private function getFromClauses() : array
$knownAliases = [];

// Loop through all FROM clauses
/** @var From $from */
foreach ($this->sqlParts['from'] as $from) {
if ($from['alias'] === null || $from['alias'] === $from['table']) {
$tableSql = $from['table'];

/** @var string $tableReference */
$tableReference = $from['table'];
if ($from->alias === null || $from->alias === $from->table) {
BenMorel marked this conversation as resolved.
Show resolved Hide resolved
$tableSql = $from->table;
$tableReference = $from->table;
} else {
$tableSql = $from['table'] . ' ' . $from['alias'];

/** @var string $tableReference */
$tableReference = $from['alias'];
$tableSql = $from->table . ' ' . $from->alias;
$tableReference = $from->alias;
}

$knownAliases[$tableReference] = true;
Expand Down Expand Up @@ -1172,7 +1147,7 @@ private function isLimitQuery() : bool
*/
private function getSQLForInsert() : string
{
return 'INSERT INTO ' . $this->sqlParts['from']['table'] .
return 'INSERT INTO ' . $this->sqlParts['table'] .
' (' . implode(', ', array_keys($this->sqlParts['values'])) . ')' .
' VALUES(' . implode(', ', $this->sqlParts['values']) . ')';
}
Expand All @@ -1182,15 +1157,7 @@ private function getSQLForInsert() : string
*/
private function getSQLForUpdate() : string
{
$from = $this->sqlParts['from'];

if ($from['alias'] === null || $from['alias'] === $from['table']) {
$table = $from['table'];
} else {
$table = $from['table'] . ' ' . $from['alias'];
}

return 'UPDATE ' . $table
return 'UPDATE ' . $this->sqlParts['table']
. ' SET ' . implode(', ', $this->sqlParts['set'])
. ($this->sqlParts['where'] !== null ? ' WHERE ' . ((string) $this->sqlParts['where']) : '');
}
Expand All @@ -1200,15 +1167,7 @@ private function getSQLForUpdate() : string
*/
private function getSQLForDelete() : string
{
$from = $this->sqlParts['from'];

if ($from['alias'] === null || $from['alias'] === $from['table']) {
$table = $from['table'];
} else {
$table = $from['table'] . ' ' . $from['alias'];
}

return 'DELETE FROM ' . $table . ($this->sqlParts['where'] !== null ? ' WHERE ' . ((string) $this->sqlParts['where']) : '');
return 'DELETE FROM ' . $this->sqlParts['table'] . ($this->sqlParts['where'] !== null ? ' WHERE ' . ((string) $this->sqlParts['where']) : '');
}

/**
Expand Down
79 changes: 7 additions & 72 deletions tests/Doctrine/Tests/DBAL/Query/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -404,65 +404,27 @@ public function testSelectMultipleFrom() : void
}

public function testUpdate() : void
{
$qb = new QueryBuilder($this->conn);
$qb->update('users', 'u')
->set('u.foo', '?')
->set('u.bar', '?');

self::assertEquals(QueryBuilder::UPDATE, $qb->getType());
self::assertEquals('UPDATE users u SET u.foo = ?, u.bar = ?', (string) $qb);
}

public function testUpdateWithoutAlias() : void
{
$qb = new QueryBuilder($this->conn);
$qb->update('users')
->set('foo', '?')
->set('bar', '?');

self::assertEquals('UPDATE users SET foo = ?, bar = ?', (string) $qb);
}

public function testUpdateWithMatchingAlias() : void
{
$qb = new QueryBuilder($this->conn);
$qb->update('users', 'users')
->set('foo', '?')
->set('bar', '?');

self::assertEquals(QueryBuilder::UPDATE, $qb->getType());
self::assertEquals('UPDATE users SET foo = ?, bar = ?', (string) $qb);
}

public function testUpdateWhere() : void
{
$qb = new QueryBuilder($this->conn);
$qb->update('users', 'u')
->set('u.foo', '?')
->where('u.foo = ?');

self::assertEquals('UPDATE users u SET u.foo = ? WHERE u.foo = ?', (string) $qb);
}

public function testEmptyUpdate() : void
{
$qb = new QueryBuilder($this->conn);
$qb2 = $qb->update();
$qb->update('users')
->set('foo', '?')
->where('foo = ?');

self::assertEquals(QueryBuilder::UPDATE, $qb->getType());
self::assertSame($qb2, $qb);
self::assertEquals('UPDATE users SET foo = ? WHERE foo = ?', (string) $qb);
}

public function testDelete() : void
{
$qb = new QueryBuilder($this->conn);
$qb->delete('users', 'u');

self::assertEquals(QueryBuilder::DELETE, $qb->getType());
self::assertEquals('DELETE FROM users u', (string) $qb);
}

public function testDeleteWithoutAlias() : void
{
$qb = new QueryBuilder($this->conn);
$qb->delete('users');
Expand All @@ -471,31 +433,13 @@ public function testDeleteWithoutAlias() : void
self::assertEquals('DELETE FROM users', (string) $qb);
}

public function testDeleteWithMatchingAlias() : void
{
$qb = new QueryBuilder($this->conn);
$qb->delete('users', 'users');

self::assertEquals(QueryBuilder::DELETE, $qb->getType());
self::assertEquals('DELETE FROM users', (string) $qb);
}

public function testDeleteWhere() : void
{
$qb = new QueryBuilder($this->conn);
$qb->delete('users', 'u')
$qb->delete('users')
->where('u.foo = ?');

self::assertEquals('DELETE FROM users u WHERE u.foo = ?', (string) $qb);
}

public function testEmptyDelete() : void
{
$qb = new QueryBuilder($this->conn);
$qb2 = $qb->delete();

self::assertEquals(QueryBuilder::DELETE, $qb->getType());
self::assertSame($qb2, $qb);
self::assertEquals('DELETE FROM users WHERE u.foo = ?', (string) $qb);
}

public function testInsertValues() : void
Expand Down Expand Up @@ -559,15 +503,6 @@ public function testInsertValuesSetValue() : void
self::assertEquals('INSERT INTO users (foo, bar) VALUES(?, ?)', (string) $qb);
}

public function testEmptyInsert() : void
{
$qb = new QueryBuilder($this->conn);
$qb2 = $qb->insert();

self::assertEquals(QueryBuilder::INSERT, $qb->getType());
self::assertSame($qb2, $qb);
}

public function testGetConnection() : void
{
$qb = new QueryBuilder($this->conn);
Expand Down