Merge pull request #49593 from ClickHouse/rs/qc-empty-chunk

Query Cache: Safeguard against empty chunks

docs/en/operations/settings/settings.md

@@ -1514,7 +1514,7 @@ Default value: `0`.
 
 ## query_cache_max_size_in_bytes {#query-cache-max-size-in-bytes}
 
-The maximum amount of memory (in bytes) the current user may allocate in the query cache. 0 means unlimited.
+The maximum amount of memory (in bytes) the current user may allocate in the [query cache](../query-cache.md). 0 means unlimited.
 
 Possible values:
 
@@ -1524,7 +1524,7 @@ Default value: 0 (no restriction).
 
 ## query_cache_max_entries {#query-cache-max-entries}
 
-The maximum number of query results the current user may store in the query cache. 0 means unlimited.
+The maximum number of query results the current user may store in the [query cache](../query-cache.md). 0 means unlimited.
 
 Possible values:
 

src/Interpreters/Cache/QueryCache.cpp

@@ -206,6 +206,15 @@ void QueryCache::Writer::buffer(Chunk && chunk, ChunkType chunk_type)
     if (skip_insert)
         return;
 
+    /// Reading from the query cache is implemented using processor `SourceFromChunks` which inherits from `ISource`.
+    /// The latter has logic which finishes processing (= calls `.finish()` on the output port + returns `Status::Finished`)
+    /// when the derived class returns an empty chunk. If this empty chunk is not the last chunk,
+    /// i.e. if it is followed by non-empty chunks, the query result will be incorrect.
+    /// This situation should theoretically never occur in practice but who knows...
+    /// To be on the safe side, writing into the query cache now rejects empty chunks and thereby avoids this scenario.
+    if (chunk.empty())
+        return;
+
     std::lock_guard lock(mutex);
 
     switch (chunk_type)