Repository navigation
Expose the X-ClickHouse-Summary header as last_summary - #6
NYDrewReynolds wants to merge 3 commits into
Conversation
ClickHouse reports what a statement read and wrote in the X-ClickHouse-Summary response header. raw_execute hands the response to processed_response, which returns only the parsed body, so the header was reachable only inside exec_delete, which builds its own request. Record it per thread, keyed the same way as response_format_stack, and read it back through last_summary. exec_delete now shares the parser rather than repeating it; that also stops a response with no summary header raising TypeError from JSON.parse(nil), which the inline rescue did not cover. Callers want written_rows: an INSERT ... SELECT that matched no rows and one that wrote millions are otherwise indistinguishable from the response body.
raw_execute now reads the summary off the response, so the doubles that stand in for one have to expose header as the real class does.
json 3.0 takes one argument to JSON.parse, ActiveSupport's JSON.decode still passes two, and every spec touching a JSON column fails on deserialize. The gem has no lockfile, so CI resolves the newest json on each run and master is red for the same reason.
| end | ||
|
|
||
| # Rows and bytes the most recent statement on this thread read and wrote, from X-ClickHouse-Summary. | ||
| def last_summary |
There was a problem hiding this comment.
this gets overwritten by any later statement on the same thread, including ones the adapter runs itself like do_system_execute, so the export has to read it on the line right after the insert
when the swarm side lands, should that be a small wrapper that runs the statement and reads the summary in one place? curious your thoughts
| retry if (retries -= 1) > 0 # rubocop:disable Style/NumericPredicate | ||
| raise | ||
| end | ||
| record_summary(response) |
There was a problem hiding this comment.
execute_to_file builds its own request so it never reaches this, and a raise after the retries skips it too. both leave the previous statement's summary in place, so should those clear it as well?
| expect(connection.last_summary['written_rows'].to_i).to eq(0) | ||
| end | ||
|
|
||
| it 'keeps one thread summary out of another' do |
There was a problem hiding this comment.
nothing covers the no-header case where record_summary rescues and this comes back nil, and exec_delete's new return path isn't asserted anywhere either (model_spec only checks that delete_all doesn't raise). worth a stubbed-response spec for both?
Why
ClickHouse reports what a statement read and wrote in the
X-ClickHouse-Summaryresponse header.raw_executepasses the response toprocessed_response, which returns only the parsed body, so the header was reachable only insideexec_delete— which builds its own request and keeps the raw response.swarm needs
written_rowsfor the nightly S3 events export. AnINSERT ... SELECTthat matched no rows and one that wrote millions are indistinguishable from the response body alone, so a run that exported nothing currently records the same "succeeded" as one that delivered 16M rows.What
raw_execute, keyed the same way as the existingresponse_format_stack, and read it back throughlast_summary.exec_deletenow shares the parser instead of repeating it.One behaviour change worth calling out: the old inline parse was
JSON.parse(res.header['x-clickhouse-summary'])rescuing onlyJSON::ParserError. A response with no summary header passesniland raisesTypeError, which that rescue does not cover. The shared parser coerces with.to_sfirst, so a missing header now yieldsnilrather than raising.A statement that reports no summary clears the previous one, so a stale value can't be misread as belonging to the current statement.
Verified
INSERT ... SELECTagainst a local ClickHouse returns:and a plain
SELECTreturns"written_rows":"0". Values are strings, so callers need.to_i.Specs added in
spec/single/last_summary_spec.rbcover written rows on insert, zero on select, read rows, that the summary tracks the latest statement, and that it does not leak across threads.