JSONCompactWithProgress query output format#66205
JSONCompactWithProgress query output format#66205Algunenano merged 31 commits intoClickHouse:masterfrom
Conversation
alexkorep
added
the
can be tested
|
This is an automated comment for commit 1eb7cdb with description of existing statuses. It's updated for the latest CI running ✅ Click here to open a full report in a separate page Successful checks
|
alexkorep
changed the title
Algunenano
left a comment
Algunenano
left a comment
There was a problem hiding this comment.
In general it looks fine, although I haven't reviewed in detail yet. I've left some comments about issues I found when testing it. I'll review again in more detail once it's addressed
Algunenano
left a comment
Algunenano
left a comment
There was a problem hiding this comment.
Really nicely done.
Minor things about CI failures (TLDR, please merge with master):
- binary_amd64_compat build: Seems unrelated and probably an infra issue. Let's merge with master.
- 01168_mutations_isolation: Unrelated
[a timeout in 02735_parquet_encoder.
And a possible improvement for the future, but it's unrelated to the changes and affects all progress reports: result_bytes and result_rows seem to be always 0 even if we've already started sending data to the client. It should either match the rows sent or be removed. E.g:
{"meta": [{"name":"number", "type":"UInt64"}, {"name":"a", "type":"Int64"}, {"name":"'\"'", "type":"String"}]}
{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}}
{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"100","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}}
{"progress":{"read_rows":"10","read_bytes":"80","written_rows":"0","written_bytes":"0","total_rows_to_read":"100","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}}
{"data":["0", "0", "\""]}
{"data":["1", "1", "\""]}
{"data":["2", "2", "\""]}
{"data":["3", "3", "\""]}
{"data":["4", "4", "\""]}
{"data":["5", "5", "\""]}
{"data":["6", "6", "\""]}
{"data":["7", "7", "\""]}
{"data":["8", "8", "\""]}
{"data":["9", "9", "\""]}
{"progress":{"read_rows":"20","read_bytes":"160","written_rows":"0","written_bytes":"0","total_rows_to_read":"100","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}}
{"data":["10", "10", "\""]}
{"data":["11", "11", "\""]}
{"data":["12", "12", "\""]}
{"data":["13", "13", "\""]}
{"data":["14", "14", "\""]}
{"data":["15", "15", "\""]}
{"data":["16", "16", "\""]}
{"data":["17", "17", "\""]}
{"data":["18", "18", "\""]}
{"data":["19", "19", "\""]}
{"progress":{"read_rows":"30","read_bytes":"240","written_rows":"0","written_bytes":"0","total_rows_to_read":"100","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}}
...
|
Same for As it's unrelated to the PR, we can leave those improvements for the future and just create a ticket to remember about them. WDYT? |
|
@Algunenano I've created #69137 to follow-up the problem with zero values in elapsed_ns, result_bytes and result_rows. |
|
@Algunenano Looks like I don't have permissions to merge. |
|
It's not possible to merge with failing checks. Please merge master to confirm the build issue goes away |
I did it twice, in both cases the tests failed, but it looks like they failed in different places. The build issue went away though. |
Merging master to retry an infrastructure issue is fine, doing it to pass flaky test is frown upon since it just kicks the problem away until it becomes too big to do anything else. There is no retry, you can review the tests that failed and report them if they are not reported already. Then either help investigating and fixing them or wait until somebody else does it. |
|
Hi, @Algunenano, looks like this PR has passed all the checks, could you please merge if for me? |
5 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Introduced JSONCompactWithProgress format where ClickHouse outputs each row as a newline-delimited JSON object, including metadata, data, progress, totals, and statistics.
Documentation entry for user-facing changes
CI Settings (Only check the boxes if you know what you are doing):