Skip to content

Quote Select, join and GROUP BY column names as identifiers #241

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.

Sign up for GitHub

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

Open
simon-mundy wants to merge 2 commits into
base: 0.6.x
Choose a base branch
from
+257 −0
Open

Quote Select, join and GROUP BY column names as identifiers #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c91004
Quote Select, join and GROUP BY column names as identifiers
simon-mundy Oct 13, 2026
8499687
Keep fragment quoting for column strings it already renders as valid SQL
simon-mundy Oct 13, 2026
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
82 changes: 82 additions & 0 deletions src/Sql/Select.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@
namespace PhpDb\Sql;

use Closure;
use Override;
use PhpDb\Adapter\Driver\DriverInterface;
use PhpDb\Adapter\ParameterContainer;
use PhpDb\Adapter\Platform\PlatformInterface;
use PhpDb\Sql\Predicate\PredicateInterface;

use function array_key_exists;
use function array_key_first;
use function array_map;
use function count;
use function explode;
use function gettype;
use function implode;
use function is_array;
use function is_int;
use function is_numeric;
Expand All @@ -29,13 +32,18 @@
use function strtoupper;
use function trim;

use const PREG_SPLIT_DELIM_CAPTURE;
use const PREG_SPLIT_NO_EMPTY;

/**
* @api
*
* @property ?Where $where
* @property ?Having $having
* @property ?Join $joins
* @psalm-import-type Specification from AbstractSql
* @psalm-import-type ColumnReference from AbstractSql
* @psalm-import-type ColumnValue from AbstractSql
* @psalm-import-type PredicateValue from Predicate\PredicateSet
* @psalm-import-type JoinName from Join
* @psalm-type TableReference = string|TableIdentifier|array<string, string|TableIdentifier|Select>
Expand Down Expand Up @@ -748,6 +756,27 @@ protected function processWhere(
];
}

/**
* Quote a select, join or group column string as an identifier chain
*
* @param ColumnReference|ColumnValue $column
* @throws Exception\RuntimeException
*/
#[Override]
protected function resolveColumnValue(
Select|array|string|int|float|bool|ExpressionInterface|null $column,
PlatformInterface $platform,
?DriverInterface $driver = null,
?ParameterContainer $parameterContainer = null,
?string $namedParameterPrefix = null,
): string {
if (is_array($column) && ($column['isIdentifier'] ?? false) && is_string($column['column'])) {
return ($column['fromTable'] ?? '') . $this->quoteColumnIdentifier($column['column'], $platform);
}

return parent::resolveColumnValue($column, $platform, $driver, $parameterContainer, $namedParameterPrefix);
}

private function getHaving(): Having
{
return $this->having ??= new Having();
Expand All @@ -763,13 +792,66 @@ private function getWhere(): Where
return $this->where ??= new Where();
}

/**
* Whether fragment quoting renders a column string as valid SQL
*
* It does when each piece between the safe delimiters (space, `.` and `*`) is quoted as one
* whole name and no two pieces are separated by spaces alone.
*/
private function isFragmentQuotable(string $column, PlatformInterface $platform): bool
{
$previousIsName = false;
$pieces = preg_split('/([ .*])/', $column, -1, PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY);
foreach (false === $pieces ? [] : $pieces as $piece) {
if (' ' === $piece) {
continue;
}

$isName = '.' !== $piece && self::SQL_STAR !== $piece;
if (
$isName
&& (
$previousIsName
|| $platform->quoteIdentifierInFragment($piece) !== $platform->quoteIdentifier($piece)
)
) {
return false;
}

$previousIsName = $isName;
}

return true;
}

private function prefixFromTable(?string $fromTable, PlatformInterface $platform): string
{
return $this->prefixColumnsWithTable && null !== $fromTable && '' !== $fromTable
? $fromTable . $platform->getIdentifierSeparator()
: '';
}

/**
* Quote a column string as an identifier chain, splitting only on the table qualifier dot
*
* A `*` part is left bare. A string holding an inline ` AS ` alias, or one that fragment quoting
* already renders as valid SQL, keeps fragment quoting.
*/
private function quoteColumnIdentifier(string $column, PlatformInterface $platform): string
{
if (stripos($column, needle: ' as ') !== false || $this->isFragmentQuotable($column, $platform)) {
return $platform->quoteIdentifierInFragment($column);
}

return implode(
$platform->getIdentifierSeparator(),
array_map(
static fn(string $part): string => self::SQL_STAR === $part ? $part : $platform->quoteIdentifier($part),
explode('.', $column),
),
);
}

/**
* @param Select|TableReference|null $table
* @return array{0: string|null, 1: string}
Expand Down
175 changes: 175 additions & 0 deletions test/unit/Sql/SelectColumnNameTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
<?php

declare(strict_types=1);

namespace PhpDbTest\Sql;

use PhpDb\Sql\Expression;
use PhpDb\Sql\Select;
use PhpDbTest\TestAsset\TrustingSql92Platform;
use PHPUnit\Framework\Attributes\CoversMethod;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\TestCase;

#[Group('unit')]
#[CoversMethod(Select::class, 'processSelect')]
#[CoversMethod(Select::class, 'processGroup')]
#[CoversMethod(Select::class, 'resolveColumnValue')]
#[CoversMethod(Select::class, 'quoteColumnIdentifier')]
#[CoversMethod(Select::class, 'isFragmentQuotable')]
final class SelectColumnNameTest extends TestCase
{
/**
* @return array<string, array{string|Expression, string}>
*/
public static function groupColumnProvider(): array
{
return [
'plain' => ['bar', '"bar"'],
'dotted' => ['t.bar', '"t"."bar"'],
'hyphen' => ['x-y', '"x-y"'],
'dotted hyphen' => ['t-1.x-y', '"t-1"."x-y"'],
'space' => ['my col', '"my col"'],
'quote character' => ['a"b', '"a\'b"'],
'function call' => ['YEAR(created)', '"YEAR(created)"'],
'arithmetic' => ['a * b', '"a" * "b"'],
'arithmetic without spaces' => ['a*b', '"a"*"b"'],
'expression' => [new Expression('YEAR(created)'), 'YEAR(created)'],
];
}

/**
* @return array<string, array{array<int|string, string|Expression>, string}>
*/
public static function joinColumnProvider(): array
{
return [
'plain' => [['bar'], '"zac"."bar" AS "bar"'],
'plain aliased' => [['b' => 'bar'], '"zac"."bar" AS "b"'],
'dotted' => [['t.bar'], '"zac"."t"."bar" AS "t.bar"'],
'star' => [['*'], '"zac".*'],
'qualified star' => [['t.*'], '"zac"."t".* AS "t.*"'],
'hyphen' => [['x-y'], '"zac"."x-y" AS "x-y"'],
'hyphen aliased' => [['a' => 'x-y'], '"zac"."x-y" AS "a"'],
'space' => [['my col'], '"zac"."my col" AS "my col"'],
'space aliased' => [['a' => 'my col'], '"zac"."my col" AS "a"'],
'quote character' => [['a"b'], '"zac"."a\'b" AS "a\'b"'],
'quote character aliased' => [['a' => 'a"b'], '"zac"."a\'b" AS "a"'],
'arithmetic' => [['price * qty'], '"zac"."price" * "qty" AS "price * qty"'],
'arithmetic without spaces' => [['a*b'], '"zac"."a"*"b" AS "a*b"'],
'inline alias' => [['bar AS baz'], '"zac"."bar" AS "baz" AS "bar AS baz"'],
'expression' => [[new Expression('COUNT(*)')], 'COUNT(*) AS Expression1'],
'expression aliased' => [['c' => new Expression('COUNT(*)')], 'COUNT(*) AS "c"'],
];
}

/**
* @return array<string, array{array<int|string, string|Expression>, bool, string}>
*/
public static function selectColumnProvider(): array
{
return [
'plain' => [['bar'], true, 'SELECT "foo"."bar" AS "bar" FROM "foo"'],
'plain aliased' => [['b' => 'bar'], true, 'SELECT "foo"."bar" AS "b" FROM "foo"'],
'dotted' => [['t.bar'], true, 'SELECT "foo"."t"."bar" AS "t.bar" FROM "foo"'],
'dotted without prefix' => [['t.bar'], false, 'SELECT "t"."bar" AS "t.bar" FROM "foo"'],
'star' => [['*'], true, 'SELECT "foo".* FROM "foo"'],
'star without prefix' => [['*'], false, 'SELECT * FROM "foo"'],
'qualified star' => [['t.*'], true, 'SELECT "foo"."t".* AS "t.*" FROM "foo"'],
'qualified star without prefix' => [['t.*'], false, 'SELECT "t".* AS "t.*" FROM "foo"'],
'hyphen' => [['x-y'], true, 'SELECT "foo"."x-y" AS "x-y" FROM "foo"'],
'hyphen aliased' => [['a' => 'x-y'], true, 'SELECT "foo"."x-y" AS "a" FROM "foo"'],
'hyphen without prefix' => [['x-y'], false, 'SELECT "x-y" AS "x-y" FROM "foo"'],
'space' => [['my col'], true, 'SELECT "foo"."my col" AS "my col" FROM "foo"'],
'space aliased' => [['a' => 'my col'], true, 'SELECT "foo"."my col" AS "a" FROM "foo"'],
'quote character' => [['a"b'], true, 'SELECT "foo"."a\'b" AS "a\'b" FROM "foo"'],
'quote character aliased' => [['a' => 'a"b'], true, 'SELECT "foo"."a\'b" AS "a" FROM "foo"'],
'dotted hyphen' => [['t-1.x-y'], false, 'SELECT "t-1"."x-y" AS "t-1.x-y" FROM "foo"'],
'dotted space aliased' => [
['a' => 't 1.my col'],
false,
'SELECT "t 1"."my col" AS "a" FROM "foo"',
],
'function call' => [['COUNT(id)'], false, 'SELECT "COUNT(id)" AS "COUNT(id)" FROM "foo"'],
'arithmetic' => [
['price * qty'],
true,
'SELECT "foo"."price" * "qty" AS "price * qty" FROM "foo"',
],
'arithmetic without prefix' => [
['price * qty'],
false,
'SELECT "price" * "qty" AS "price * qty" FROM "foo"',
],
'arithmetic without spaces' => [['a*b'], false, 'SELECT "a"*"b" AS "a*b" FROM "foo"'],
'arithmetic aliased' => [
['total' => 'price * qty'],
false,
'SELECT "price" * "qty" AS "total" FROM "foo"',
],
'dotted arithmetic' => [
['t.price * t.qty'],
false,
'SELECT "t"."price" * "t"."qty" AS "t.price * t.qty" FROM "foo"',
],
'arithmetic with hyphenated name' => [['x-y * 2'], false, 'SELECT "x-y * 2" AS "x-y * 2" FROM "foo"'],
'inline alias' => [['bar AS baz'], true, 'SELECT "foo"."bar" AS "baz" FROM "foo"'],
'inline alias lower case' => [['bar as baz'], true, 'SELECT "foo"."bar" as "baz" FROM "foo"'],
'inline alias without prefix' => [['t.bar AS baz'], false, 'SELECT "t"."bar" AS "baz" FROM "foo"'],
'expression' => [
[new Expression('COUNT(id)')],
true,
'SELECT COUNT(id) AS Expression1 FROM "foo"',
],
'expression aliased' => [
['c' => new Expression('COUNT(id)')],
true,
'SELECT COUNT(id) AS "c" FROM "foo"',
],
];
}

#[Test]
#[DataProvider('groupColumnProvider')]
public function quotesGroupColumnStringsAsIdentifierChains(string|Expression $column, string $expected): void
{
$select = (new Select('foo'))->group($column);

self::assertSame(
"SELECT \"foo\".* FROM \"foo\" GROUP BY {$expected}",
$select->getSqlString(new TrustingSql92Platform()),
);
}

/**
* @param array<int|string, string|Expression> $columns
*/
#[Test]
#[DataProvider('joinColumnProvider')]
public function quotesJoinColumnStringsAsIdentifierChains(array $columns, string $expected): void
{
$select = (new Select('foo'))->columns([])->join('zac', 'm = n', $columns);

self::assertSame(
"SELECT {$expected} FROM \"foo\" INNER JOIN \"zac\" ON \"m\" = \"n\"",
$select->getSqlString(new TrustingSql92Platform()),
);
}

/**
* @param array<int|string, string|Expression> $columns
*/
#[Test]
#[DataProvider('selectColumnProvider')]
public function quotesSelectColumnStringsAsIdentifierChains(
array $columns,
bool $prefixColumnsWithTable,
string $expected,
): void {
$select = (new Select('foo'))->columns($columns, $prefixColumnsWithTable);

self::assertSame($expected, $select->getSqlString(new TrustingSql92Platform()));
}
}
Loading