Skip to content

Commit dcb0c3e

Browse files
committed
Revert Quantifier/Table::prepare/public-field changes that regressed perf
The public field visibility changes on Columns and From, combined with the Quantifier simplification and Table::prepare inlining, caused a regression from 19μs to 28μs on the complex select benchmark. Reverting to the state after the ColumnRef/OrderSpec commit. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
1 parent 71f12fd commit dcb0c3e

File tree

4 files changed

+68
-24
lines changed

4 files changed

+68
-24
lines changed

src/Sql/Part/Columns.php

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,15 @@ class Columns extends AbstractPart
3131
/** @var array Raw columns for getRawState() backward compatibility */
3232
private array $rawColumns = [Select::SQL_STAR];
3333

34-
public bool $prefixColumnsWithTable = true;
34+
private bool $prefixColumnsWithTable = true;
3535

36-
/** @var string Resolved table prefix (e.g. "table".) — set by Table::prepare() */
37-
public string $fromTablePrefix = '';
36+
/**
37+
* Set during preparePartsForBuild -- the resolved table prefix (e.g. "table".)
38+
*/
39+
private string $fromTablePrefix = '';
3840

39-
/** @var JoinSpec[] Join specs — set by Table::prepare() */
40-
public array $joinSpecs = [];
41+
/** @var JoinSpec[] Join specs for column resolution */
42+
private array $joinSpecs = [];
4143

4244
public function __construct()
4345
{
@@ -166,6 +168,26 @@ public function getPrefixColumnsWithTable(): bool
166168
return $this->prefixColumnsWithTable;
167169
}
168170

171+
/**
172+
* Set the table prefix for column resolution (e.g. "table".)
173+
*/
174+
public function setFromTablePrefix(string $prefix): static
175+
{
176+
$this->fromTablePrefix = $prefix;
177+
return $this;
178+
}
179+
180+
/**
181+
* Set join specs for column resolution during rendering.
182+
*
183+
* @param JoinSpec[] $specs
184+
*/
185+
public function setJoinSpecs(array $specs): static
186+
{
187+
$this->joinSpecs = $specs;
188+
return $this;
189+
}
190+
169191
/**
170192
* Normalize the raw columns array into ColumnRef[].
171193
*/

src/Sql/Part/From.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*/
1515
class From extends AbstractPart
1616
{
17-
public ?TableRef $ref = null;
17+
private ?TableRef $ref = null;
1818

1919
private ?string $resolvedTable = null;
2020

src/Sql/Part/Quantifier.php

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,32 @@
44

55
namespace PhpDb\Sql\Part;
66

7+
use PhpDb\Sql\Argument\Literal;
8+
use PhpDb\Sql\Argument\Select as SelectArgument;
9+
use PhpDb\Sql\ArgumentInterface;
10+
use PhpDb\Sql\ArgumentType;
711
use PhpDb\Sql\ExpressionInterface;
12+
use ValueError;
813

9-
use function is_string;
10-
14+
/**
15+
* Holds and renders a SELECT quantifier (DISTINCT, ALL, or expression).
16+
* Normalizes input to ArgumentInterface at set time.
17+
*/
1118
class Quantifier extends AbstractPart
1219
{
13-
private ExpressionInterface|string|null $quantifier = null;
20+
private ?ArgumentInterface $quantifier = null;
1421

1522
public function toSql(SqlProcessor $processor): ?string
1623
{
1724
if ($this->quantifier === null) {
1825
return null;
1926
}
2027

21-
if (is_string($this->quantifier)) {
22-
return $this->quantifier;
23-
}
24-
25-
return $processor->renderExpression($this->quantifier, 'quantifier');
28+
return match ($this->quantifier->getType()) {
29+
ArgumentType::Literal => $this->quantifier->getValue(),
30+
ArgumentType::Select => $processor->renderExpression($this->quantifier->getValue(), 'quantifier'),
31+
default => throw new ValueError('Unexpected ArgumentType: ' . $this->quantifier->getType()->name),
32+
};
2633
}
2734

2835
public function isEmpty(): bool
@@ -32,12 +39,25 @@ public function isEmpty(): bool
3239

3340
public function set(string|ExpressionInterface|null $quantifier): static
3441
{
35-
$this->quantifier = $quantifier;
42+
if ($quantifier === null) {
43+
$this->quantifier = null;
44+
} elseif ($quantifier instanceof ExpressionInterface) {
45+
$this->quantifier = new SelectArgument($quantifier);
46+
} else {
47+
$this->quantifier = new Literal($quantifier);
48+
}
3649
return $this;
3750
}
3851

52+
/**
53+
* Reconstruct the original format for getRawState() compatibility.
54+
*/
3955
public function get(): string|ExpressionInterface|null
4056
{
41-
return $this->quantifier;
57+
if ($this->quantifier === null) {
58+
return null;
59+
}
60+
61+
return $this->quantifier->getValue();
4262
}
4363
}

src/Sql/Part/Table.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class Table
2222
{
2323
private From $from;
2424
private Columns $columns;
25-
private ?Joins $joins = null;
25+
private ?Joins $joins = null;
2626
private ?int $preparedPlatformId = null;
2727

2828
/** Backward-compat model exposed via getRawState / __get */
@@ -50,7 +50,7 @@ public function getFrom(): string|array|TableIdentifier|Select|null
5050

5151
public function hasFrom(): bool
5252
{
53-
return ! $this->from->isEmpty();
53+
return !$this->from->isEmpty();
5454
}
5555

5656
public function resetFrom(): static
@@ -106,12 +106,12 @@ public function join(
106106

107107
public function hasJoins(): bool
108108
{
109-
return $this->joins !== null && ! $this->joins->isEmpty();
109+
return $this->joins !== null && !$this->joins->isEmpty();
110110
}
111111

112112
public function resetJoins(): static
113113
{
114-
$this->joins = null;
114+
$this->joins = null;
115115
$this->preparedPlatformId = null;
116116
return $this;
117117
}
@@ -125,10 +125,12 @@ public function prepare(SqlProcessor $processor): void
125125
return;
126126
}
127127

128-
$this->columns->fromTablePrefix = $this->columns->prefixColumnsWithTable && $this->from->ref !== null
129-
? $this->from->getQuotedPrefix($processor)
130-
: '';
131-
$this->columns->joinSpecs = $this->joins?->getSpecs() ?? [];
128+
if ($this->columns->getPrefixColumnsWithTable() && !$this->from->isEmpty()) {
129+
$this->columns->setFromTablePrefix($this->from->getQuotedPrefix($processor));
130+
} else {
131+
$this->columns->setFromTablePrefix('');
132+
}
133+
$this->columns->setJoinSpecs($this->joins !== null ? $this->joins->getSpecs() : []);
132134
$this->preparedPlatformId = $platformId;
133135
}
134136

0 commit comments

Comments
 (0)