Tags: BetterStackHQ/ClickHouse

Check for cyclic dependencies with a bounded search instead of a full…

…-graph topological sort

`DatabaseCatalog` checks that a `CREATE`, `ATTACH`, `RENAME` or `EXCHANGE` does
not introduce a cyclic dependency by applying the change to the dependency
graph, calling `hasCyclicDependencies` and reverting. That check is a Kahn
topological sort over every node of the graph, so its cost grows with the number
of tables on the server rather than with the size of the change, and each of the
three checks pays it twice - once for the referential graph and once for the
loading graph. One `CREATE OR REPLACE VIEW` on an `Atomic` database runs six of
them.

The checks themselves maintain the invariant that no statement introduces a
cycle, so in the steady state the graph is acyclic. When the dependencies of a
few tables are replaced in an acyclic graph, every edge of a cycle that avoids
those tables existed before, so any new cycle passes through one of them via one
of its new dependencies. `TablesDependencyGraph::hasCyclicDependenciesAfterReplacing`
answers the question that way: for each changed table it searches forward from
that table's new dependencies, following the replacements of the other changed
tables as well, and stops when it reaches the table it started from. It visits
only the part of the graph reachable from the new dependencies, does not modify
the graph, and does not touch the lazily calculated levels.

The three checks now call it first and return when it finds nothing. Only when
it reports a cycle do they fall back to the previous code - apply, sort,
describe the cycle, revert, throw - so the `INFINITE_LOOP` exception and its
message are unchanged. A `LOGICAL_ERROR` on that path reports a bounded search
which found a cycle the full sort does not see.

One difference in behaviour: `hasCyclicDependencies` answers "does the graph
contain any cycle", so a cycle anywhere in the graph used to fail every
dependency-adding statement on the server, naming unrelated tables. The bounded
search answers "would this statement create a cycle through the tables it
changes", which is what these checks were added for and what their messages say.
A cycle can only pre-exist in metadata written by a server which did not perform
these checks; a unit test pins the narrowing so it is not read as an accident.

The stateless test was renumbered from 05137 on master to 05063, which is free
on this branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WhrWgQCSNRgwYevjG4P1W
(cherry picked from commit 38cfd90)

Do not derive the async insert flush deadline for `break` mode queries

Under `timeout_overflow_mode = 'break'` the query waiting for the flush
stops at its limit without an error, so its data must still be flushed;
deriving a deadline from it would expire the entry after the client was
told the insert succeeded. Such batches keep the previous behaviour.

Also drop the error code declarations left unused by the shared wait
helper, and cover the HTTP wait path in the test.

Drop two settings history entries that name settings which do not exist

The entries for `use_mv_select_plan_cache` and
`mv_select_plan_cache_max_variants_per_view` predate the renaming of those
settings and were removed from this branch's history block; reapplying the
branch onto a newer base brought them back because the removal no longer
matched its context. The settings history must only name settings that are
declared, or the `compatibility` setting cannot be applied.

Release v26.7.6.57-stable

Do not recover a rewritten object into a read made for the old one

The rewrite recovery re-reads the object under its refreshed metadata, which
is correct only when the read takes the whole file. A read whose row groups
were already chosen - by the query condition cache under the cached ETag, or
by a cluster task's assignment - would apply that selection, made for the
generation that is gone, to the generation that replaced it: a partial count
returned as a whole one. Such a read is no longer repeated; it reports the
overwrite, and the refreshed cache entry serves the reads that follow.

Also stop reading a whole-object GET that carries no Content-Length as a
size of zero: the SDK leaves its default in place for a missing header, and
a spurious size mismatch on an unchanged object would be the result.

Recognize a missing object by the error code the pipeline keeps

The error of a read that ran into a missing object arrives in the source as a
plain exception: the pipeline passes an exception between processors by value,
which slices the storage-specific exception carrying the S3 error type down to
the error code that every S3 failure shares. Reading the type back therefore
never succeeded, and an object deleted after its metadata was cached was
reported as a failed read on the stores that report a missing key, rather than
being skipped or reported as missing.

Match the error code alone. The wider match costs a metadata request on a first
read that failed for another reason, and that request is what establishes
whether the object is there in the first place, so the decision it feeds is
unchanged.

The test gains a read of a deleted object with the read-time ETag condition off,
which is the shape that has the storage report the missing key on every store,
and stops asking for server log messages: the storage reports the object as
missing at error level, and the client mirrors those to its stderr, where the
test runner takes any output for a failure.

Include <mutex> for the parse memo lock

`PartColumnsParseMemo::parse` takes a `std::lock_guard` over the memo mutex
before publishing a parsed entry, and none of the headers it includes brings
<mutex> in under -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES.

Correct the object metadata cache gating, accounting and documentation

Keep archives out of the cache. The iterator saw the archive object and would
have stored its metadata under the archive path, while a failed read of a
member invalidates the `archive :: member` path, so a replaced archive could
never recover. The prefix is now empty for archive configurations.

Serve the cache only to callers that can invalidate it. The static `createReader`
is also used by `ObjectStorageQueueSource`, which has its own `generate` and
therefore never drops an entry after a failed read; it now takes an explicit
`allow_object_metadata_cache` argument that only `StorageObjectStorageSource`
passes.

Describe the failure mode honestly. Entries are never revalidated, so a violated
immutability contract can produce stale results and not only errors:
`s3_validate_etag_on_read` sees only reads that reach S3, while reads served from
the filesystem cache, and the row count and schema inference caches, do not, and
Azure and HDFS have no equivalent check. What the error path guarantees is
recovery of the next query.

Also: drop the `ObjectMetadataCacheBytes` metric, which reported the entry count
rather than bytes, and keep `ObjectMetadataCacheEntries`; count an invalidation
only when an entry was really removed; never let a failure to invalidate replace
the error that caused it; and skip the cache for metadata-only iterations, which
neither read objects nor report progress.

Add tests for the cluster read path, where the worker fetches the metadata in
`createReader`, and for the filesystem cache key, which is derived from the ETag
and must be identical whether the metadata came from a request or from the cache.

Skip building dependency-graph trace output when it would not be logged

TablesDependencyGraph::log() described every node unconditionally: it forced a
recalculation of the levels of the whole graph and built two strings per node
before LOG_TRACE decided whether to emit anything. DatabaseCatalog calls it
while holding its global lock on every CREATE statement that adds a dependency,
so that is O(size of the graph) of work per such statement at any log level.
Skip the pass unless a trace message would actually be written, using the same
condition as LOG_IMPL, so a client that asked for trace logs through
send_logs_level still receives every line.

Also initialize the logger in the constructor. It used to be created lazily by
a const method reachable from every read-only getter, which is a data race as
soon as the graph is read concurrently.

Internal release 26.7.4.58.3: histogram aggregate state optimisations