Skip to content

Commit ddf9377

Browse files
committed
refactor(clickhouse): rationalize insertFormat/bulkInsert boundary
bulkInsert() is the recommended bulk-ingest entry point; insertFormat() stays as a lower-level setter for callers that stream the body separately. Both paths now share a single private envelope emitter (compileFormatInsertEnvelope) so they cannot diverge on table quoting, column resolution, or column-name validation. Also aligns insertFormat()+insert() with bulkInsert() by accepting the no-columns envelope form (INSERT INTO t FORMAT name) — ClickHouse treats the column list as optional, and the two paths now match.
1 parent 076fd57 commit ddf9377

4 files changed

Lines changed: 123 additions & 74 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1551,7 +1551,7 @@ $statement = (new Builder())
15511551

15521552
The `Format` enum currently supports `Format::JSONEachRow` and `Format::TabSeparated`. JSONEachRow rows are encoded with `JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE` (slashes and non-ASCII are preserved verbatim); TabSeparated escapes `\\`, `\t`, `\n`, `\r` and emits `\N` for `null`. An empty row iterable produces an empty body, which ClickHouse accepts as a zero-row ingest. The iterable is consumed eagerly — pass a generator if you want to defer row construction, but the serialized body is materialized in full before the statement is returned.
15531553

1554-
For envelopes only (no body — e.g. when streaming the payload from elsewhere), the lower-level `insertFormat()` setter remains available and pairs with `insert()` as before:
1554+
`bulkInsert()` is the recommended entry point for FORMAT-based ingest — it covers the full envelope + body contract in one typed call. The lower-level `insertFormat()` setter pairs with `insert()` for the envelope-only path (returns `body = null`) and is retained for callers that stream the payload separately. Both paths share the same envelope emitter, so the resulting `query` is identical for equivalent inputs:
15551555

15561556
```php
15571557
$statement = (new Builder())

src/Query/Builder/ClickHouse.php

Lines changed: 65 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -141,35 +141,12 @@ public function hint(string $hint): static
141141
}
142142

143143
/**
144-
* Declare a ClickHouse FORMAT pragma for the next INSERT.
145-
*
146-
* When a format is set, `insert()` emits
147-
* `INSERT INTO \`t\` (\`col1\`, \`col2\`) FORMAT <name>` with no VALUES.
148-
* The row payload must be streamed into the HTTP body by the caller.
149-
* Column names are derived from the most recent `set()` call (values are
150-
* ignored). Pass `$columns` to declare them explicitly when no `set()`
151-
* call has been made.
152-
*
153-
* @param list<string> $columns
154-
*/
155-
public function insertFormat(string $format, array $columns = []): static
156-
{
157-
if (!\preg_match('/^[A-Za-z][A-Za-z0-9_]*$/', $format)) {
158-
throw new ValidationException('Invalid ClickHouse INSERT format: ' . $format);
159-
}
160-
161-
$this->insertFormat = $format;
162-
$this->insertFormatColumns = $columns;
163-
164-
return $this;
165-
}
166-
167-
/**
168-
* Build a single statement that carries both the `INSERT INTO <table>
169-
* FORMAT <name>` envelope and the serialized row payload for a
170-
* ClickHouse bulk ingest. Returns a `FormattedInsertStatement` whose
171-
* `->query` is the envelope and whose `->body` is the formatted
172-
* payload to send as the HTTP request body.
144+
* Recommended bulk-ingest entry point. Emits the canonical `INSERT INTO
145+
* <table> [(<cols>)] FORMAT <name>` envelope alongside the serialized row
146+
* payload in a single typed call. The returned `FormattedInsertStatement`
147+
* exposes `->query` (envelope, no bindings) and `->body` (format-specific
148+
* payload) so the caller can ship both to ClickHouse's HTTP interface
149+
* without hand-assembling either side.
173150
*
174151
* The target table must be set via `into()` first. Columns are derived
175152
* from the keys of the first row when `$columns` is omitted; pass
@@ -182,9 +159,6 @@ public function insertFormat(string $format, array $columns = []): static
182159
*/
183160
public function bulkInsert(Format $format, iterable $rows, array $columns = []): FormattedInsertStatement
184161
{
185-
$this->bindings = [];
186-
$this->validateTable();
187-
188162
$materialized = [];
189163
foreach ($rows as $row) {
190164
/** @phpstan-ignore function.alreadyNarrowedType */
@@ -198,22 +172,7 @@ public function bulkInsert(Format $format, iterable $rows, array $columns = []):
198172
$columns = \array_keys($materialized[0]);
199173
}
200174

201-
foreach ($columns as $col) {
202-
if ($col === '') {
203-
throw new ValidationException('Column names for bulkInsert() must be non-empty strings.');
204-
}
205-
}
206-
207-
$wrappedColumns = empty($columns)
208-
? ''
209-
: ' (' . \implode(', ', \array_map(
210-
fn (string $col): string => $this->resolveAndWrap($col),
211-
$columns
212-
)) . ')';
213-
214-
$sql = 'INSERT INTO ' . $this->quote($this->table)
215-
. $wrappedColumns
216-
. ' FORMAT ' . $format->value;
175+
$sql = $this->compileFormatInsertEnvelope($format->value, $columns);
217176

218177
$body = $format->serialize($materialized, empty($columns) ? null : $columns);
219178

@@ -230,6 +189,63 @@ public function bulkInsert(Format $format, iterable $rows, array $columns = []):
230189
);
231190
}
232191

192+
/**
193+
* Lower-level setter for the FORMAT envelope. Use `bulkInsert()` for the
194+
* typed entry point; this method is retained for callers that need to
195+
* stream the body payload separately (e.g. piping a pre-serialized stream
196+
* straight into the HTTP request) — the subsequent `insert()` call emits
197+
* the envelope only, with `body = null`.
198+
*
199+
* Column names are derived from the most recent `set()` call (values are
200+
* ignored). Pass `$columns` to declare them explicitly when no `set()`
201+
* call has been made.
202+
*
203+
* @param list<string> $columns
204+
*/
205+
public function insertFormat(string $format, array $columns = []): static
206+
{
207+
if (!\preg_match('/^[A-Za-z][A-Za-z0-9_]*$/', $format)) {
208+
throw new ValidationException('Invalid ClickHouse INSERT format: ' . $format);
209+
}
210+
211+
$this->insertFormat = $format;
212+
$this->insertFormatColumns = $columns;
213+
214+
return $this;
215+
}
216+
217+
/**
218+
* Build the shared `INSERT INTO <table> [(<cols>)] FORMAT <name>`
219+
* envelope. Validates the table, validates column names, quotes the
220+
* table identifier, and wraps each column via `resolveAndWrap()`.
221+
* Resets bindings so callers don't accumulate stale values from prior
222+
* builder operations.
223+
*
224+
* @param list<string> $columns
225+
*/
226+
private function compileFormatInsertEnvelope(string $format, array $columns): string
227+
{
228+
$this->bindings = [];
229+
$this->validateTable();
230+
231+
foreach ($columns as $col) {
232+
if ($col === '') {
233+
throw new ValidationException('Column names for FORMAT INSERT must be non-empty strings.');
234+
}
235+
}
236+
237+
$wrappedColumns = empty($columns)
238+
? ''
239+
: ' (' . \implode(', ', \array_map(
240+
fn (string $col): string => $this->resolveAndWrap($col),
241+
$columns
242+
)) . ')';
243+
244+
return 'INSERT INTO ' . $this->quote($this->table)
245+
. $wrappedColumns
246+
. ' FORMAT ' . $format;
247+
}
248+
233249
/**
234250
* @param array<string, string> $settings
235251
*/
@@ -685,31 +701,11 @@ public function insert(): Statement
685701
return $this->applyNamedTypedBindings(parent::insert());
686702
}
687703

688-
$this->bindings = [];
689-
$this->validateTable();
690-
691704
$columns = !empty($this->insertFormatColumns)
692705
? $this->insertFormatColumns
693706
: (!empty($this->rows) ? \array_keys($this->rows[0]) : []);
694707

695-
if (empty($columns)) {
696-
throw new ValidationException('No columns specified for FORMAT INSERT. Pass columns to insertFormat() or call set() before insert().');
697-
}
698-
699-
foreach ($columns as $col) {
700-
if ($col === '') {
701-
throw new ValidationException('Column names for FORMAT INSERT must be non-empty strings.');
702-
}
703-
}
704-
705-
$wrappedColumns = \array_map(
706-
fn (string $col): string => $this->resolveAndWrap($col),
707-
$columns
708-
);
709-
710-
$sql = 'INSERT INTO ' . $this->quote($this->table)
711-
. ' (' . \implode(', ', $wrappedColumns) . ')'
712-
. ' FORMAT ' . $format;
708+
$sql = $this->compileFormatInsertEnvelope($format, $columns);
713709

714710
return new FormattedInsertStatement(
715711
$sql,

tests/Query/Builder/Feature/ClickHouse/BulkInsertTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,4 +305,51 @@ public function testInsertFormatEnvelopeStillEmitsNullBodyForBackCompat(): void
305305
$this->assertInstanceOf(FormattedInsertStatement::class, $result);
306306
$this->assertNull($result->body);
307307
}
308+
309+
public function testBulkInsertAndInsertFormatEmitIdenticalEnvelopeForSameInputs(): void
310+
{
311+
$bulk = (new Builder())
312+
->into('events')
313+
->bulkInsert(Format::JSONEachRow, [
314+
['id' => 1, 'event' => 'login'],
315+
]);
316+
317+
$envelope = (new Builder())
318+
->into('events')
319+
->insertFormat('JSONEachRow', ['id', 'event'])
320+
->insert();
321+
322+
$this->assertInstanceOf(FormattedInsertStatement::class, $envelope);
323+
$this->assertSame($bulk->query, $envelope->query);
324+
$this->assertSame($bulk->columns, $envelope->columns);
325+
$this->assertSame($bulk->format, $envelope->format);
326+
}
327+
328+
public function testInsertFormatEnvelopeQuotesTableWithLiteralDot(): void
329+
{
330+
$envelope = (new Builder())
331+
->into('my.namespace')
332+
->insertFormat('JSONEachRow', ['id'])
333+
->insert();
334+
335+
$bulk = (new Builder())
336+
->into('my.namespace')
337+
->bulkInsert(Format::JSONEachRow, [['id' => 1]]);
338+
339+
$this->assertSame(
340+
'INSERT INTO `my`.`namespace` (`id`) FORMAT JSONEachRow',
341+
$envelope->query,
342+
);
343+
$this->assertSame($envelope->query, $bulk->query);
344+
}
345+
346+
public function testInsertFormatRejectsEmptyColumnNameMatchingBulkInsert(): void
347+
{
348+
$this->expectException(ValidationException::class);
349+
350+
(new Builder())
351+
->into('events')
352+
->insertFormat('JSONEachRow', ['id', ''])
353+
->insert();
354+
}
308355
}

tests/Query/Builder/Feature/ClickHouse/InsertFormatTest.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,20 @@ public function testInsertFormatRejectsEmptyColumnName(): void
112112
->insert();
113113
}
114114

115-
public function testInsertFormatRequiresColumns(): void
115+
public function testInsertFormatWithoutColumnsEmitsBareEnvelope(): void
116116
{
117-
$this->expectException(ValidationException::class);
118-
119-
(new Builder())
117+
$result = (new Builder())
120118
->into('events')
121119
->insertFormat('JSONEachRow')
122120
->insert();
121+
122+
$this->assertInstanceOf(FormattedInsertStatement::class, $result);
123+
$this->assertSame(
124+
'INSERT INTO `events` FORMAT JSONEachRow',
125+
$result->query
126+
);
127+
$this->assertSame([], $result->columns);
128+
$this->assertNull($result->body);
123129
}
124130

125131
public function testInsertWithoutFormatStillEmitsValues(): void

0 commit comments

Comments
 (0)