Workflow [PR], commit [563701c]

Summary: ✅

• Performance Comparison: Performance dashboard

AI Review

Summary

This PR makes whole-segment **/ globs match zero or more directory components across the regex helper and the local file / Filesystem listing path. The current code addresses the prior edge cases around brace-containing directory names, non-whole-segment **, adjacent globstar duplicates, and literal suffixes below multiple directory levels; I found no remaining correctness issues that need inline comments.

Final Verdict

Status: ✅ Approve

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

Continued this PR:

• Addressed the AI review 'Request changes' verdict on the local-path deduplication. The matched_paths set in listFilesWithRegexpMatching was declared once and shared across every expandSelectionGlob alternative, so it changed brace-expansion semantics beyond the **/ contract: independent alternatives resolving to the same concrete file (e.g. file('{top,top}.tsv', ...)) were silently collapsed instead of being read once per alternative. Moved the set inside the per-expansion loop (commit 0d86e8d) so deduplication stays scoped to the recursive globstar traversal of a single expanded pattern, while independent brace-expanded alternatives keep their pre-existing behavior.
• Added a regression to 04320_globstar_zero_level_directory asserting that {top,top}.tsv returns the file twice (the per-alternative read is preserved), alongside the existing **/**/*.tsv dedup case.
• Dropped the unnecessary no-parallel tag from the test (commit 569b55e) — it only uses its own USER_FILES_PATH/04320 subtree and does not mutate shared server state.
• Merged master (was 774 commits behind and red) — clean merge, no conflicts; master had not touched StorageFile.cpp/parseGlobs.cpp since the merge base.

Verified all five glob cases against a freshly built binary via clickhouse local: **/*.tsv matches all levels including the top-level file, **/top.tsv matches at zero level, **/braced.tsv matches the braced directory, **/**/*.tsv returns each file exactly once, and {top,top}.tsv returns the top-level file twice.

The two prior CI failures are unrelated to a glob-matching change: the Stress test (arm_release) Logical error query is a fuzzer-found analyzer issue in SortingAggregatedTransform::prepare() for a GROUP BY ... WITH TOTALS query (a distinct STID/PR each occurrence on master), and the BuzzHouse (amd_debug) ERROR is a harness infra failure (status.tsv not found, 7-21/day across distinct PRs on master).

Fixed the only red — the flaky check on 04320_globstar_zero_level_directory (all 26 failures across the four sanitizers were this single test).

Root cause: when the no-parallel tag was dropped in 569b55e, the test was still creating its fixtures under a fixed user_files/04320 subdirectory. The flaky check runs the same test concurrently many times, so independent instances raced on that one directory — one run's rm -rf/mkdir clobbered the files another run was querying, producing intermittently empty/partial results that diverged from the reference.

Fix (cd2d351bc12): use a unique per-invocation subdirectory named after $CLICKHOUSE_DATABASE instead of the fixed 04320. clickhouse-test generates a fresh random test_<...> database for every test execution (including each flaky-check rerun), so the directory is now distinct per run and concurrent instances no longer collide. This is the same isolation pattern already used by e.g. 00309_formats_case_insensitive and 01098_msgpack_format, and keeps the test parallel-safe without re-introducing no-parallel. The reference is unchanged (the directory name never appears in query output).

Verified the path substitution and globbing mechanics end-to-end against a binary (no shell-quoting issues; the unique directory resolves correctly in every file(...) argument). The feature behaviour itself was already green in the non-flaky stateless runs. Did not merge master (merge base is ~2 days old, well under the week boundary, and the red was self-caused rather than a flaky master failure).

Continued this PR.

Addressed the AI review "⚠️ Request changes" verdict

The review flagged that makeRegexpPatternFromGlobs and the local file/Filesystem listing in StorageFile disagreed on **/: the helper rewrote any non-overlapping **/ token into the zero-level form ([^/]*/)* (covering ?**/file.txt, *?**/file.txt), while the local listing applies zero-level semantics only to a path segment that is exactly ** (current_glob == "/**").

I resolved this by narrowing the helper (commit b528b89) rather than complicating the local path: ** is now treated as a globstar only when it forms a whole path segment — bounded by / (or the string boundaries) on the left and by / on the right. This:

• matches conventional glob semantics (Bash globstar, where ** is special only as a complete path component);
• makes the helper consistent with the already-correct local listing;
• leaves every whole-segment case byte-identical (**/file.txt, data/**/file.txt, data/**/sub/**/file.txt, data/**, the ***/a*** runs); only the non-whole-segment ?**/ and *?**/ cases revert to their legacy expansion.

Updated the helper unit tests for the now-legacy ?**//*?**/ cases and added a user-facing stateless regression 04490_globstar_non_whole_segment proving the local file(...) path treats ?**/ as a single-directory-level match (not zero-level), consistently with the helper. The unresolved review thread is resolved.

The branch also carries a previously-unpushed master merge and the test renumber to 04489 from an earlier session.

CI failures (all unrelated to this glob change)

The report was for d50f3fdc; none of the failures touch glob/file/path code:

• Stress test (amd_debug) — Logical error: Unexpected exception in refresh scheduling in RefreshTask::doScheduling (STID 2508-34af; same family as the closed Logical error: 'Unexpected exception in refresh scheduling' (STID: 2508-3e7b) #106737). This is fixed by Stop refreshable MV gracefully on Keeper without MULTI_READ on ATTACH/restore #108441, which the master merge now on this branch brought in but which was not present at d50f3fdc. The Cannot start clickhouse-server / Check failed rows are downstream of that crash, so they should clear on re-run.
• Stress test (arm_msan) — Hung check failed, possible deadlock found: chronic flake Hung check failed, possible deadlock found #107941.
• AST fuzzer (arm_asan_ubsan) — Block structure mismatch in A stream in UnionStep::updateOutputHeader (STID 0993-27f0): tracked master-wide fuzzer flake Logical error: Block structure mismatch in A stream: different columns: (STID: 0993-27f0) #108142 (exact STID match).
• Performance Comparison (arm_release 2/6, 4/6) — cidb.is_ready() assertion / play.clickhouse.com read timeout: CIDB infrastructure, not a performance regression.

Verified the new regex outputs for the changed cases with a faithful standalone reproduction of the final-processing loop (the inputs contain no range/enum/bracket characters, so escaping/expansion is a no-op): ?**/file.txt -> [^/][^/]*[^{}]*/file\.txt, *?**/file.txt -> [^/]*[^/][^{}]*[^{}]*/file\.txt, and all whole-segment cases unchanged.

Continued this PR.

Fixed the only real CI failure: 04490_globstar_non_whole_segment

At b528b8969bb the new test failed on every stateless lane (and triggered "Too many test failures" on the targeted/flaky checks). Its whole-segment case

SELECT * FROM file('<dir>/**/pick.tsv', 'TSV', 'a UInt8, b UInt8') ORDER BY a

matched only the zero- and one-level files (pick.tsv, sub/pick.tsv) but not the two-level sub/subsub/pick.tsv, while the test (and this PR's goal) require **/ to match zero or more directory components.

Root cause (in listFilesWithRegexpMatchingImpl, StorageFile.cpp): when descending into a subdirectory under a **/ segment, the code passed the suffix after ** (here /pick.tsv), dropping the **. For a literal suffix that string contains no glob, so the recursive call took the no-glob exact-match branch and returned without descending further — so only a single intermediate level matched. A glob suffix such as **/*.tsv happened to keep descending because the sticky recursive flag re-applied the glob segment at every level, so only literal suffixes were affected. This also made the local listing disagree with makeRegexpPatternFromGlobs, which rewrites **/ to ([^/]*/)* (any number of levels), and with 02459's **/tex* and 02707's **/*.csv, which already match all depths.

Fix (1 file, +14/-1): when the current segment is exactly ** and a suffix follows, descend into subdirectories keeping the whole **/... pattern, so the globstar keeps matching at every deeper level. The pre-existing zero-level branch applies the post-** suffix at the current level, so the combination matches zero, one, or more directory components. The trailing ** (no suffix) case is unchanged.

Verified against a freshly built binary — all of these pass:

• 04490_globstar_non_whole_segment (whole-segment **/pick.tsv → 0,1,2; non-whole ?**/pick.tsv → one level)
• 04489_globstar_zero_level_directory (**/*.tsv, **/top.tsv, braced dir, **/**/*.tsv dedup, {top,top}.tsv)
• 02459_glob_for_recursive_directory_traversal
• 02707_clickhouse_local_implicit_file_table_function
• 02816_clickhouse_local_table_name_expressions

No new test was needed — 04490 itself is the regression test, and the byte-counting / dedup paths from the earlier commits are untouched.

Other CI

The stateless redness traced entirely to 04490. The non-stateless failures were already triaged on the previous SHAs as unrelated/tracked (Stress amd_debug RefreshTask::doScheduling fixed by #108441 which the master merge brought in; arm_msan Hung check = #107941; AST fuzzer UnionStep::updateOutputHeader STID 0993-27f0 = #108142; perf-comparison = CIDB infra). I did not re-merge master: the merge base is from 2026-06-29 (< 1 day), there are no conflicts, and master has not touched parseGlobs.cpp/StorageFile.cpp since then.

Continue-PR pass (no code change): the PR is fully addressed and only blocked on flaky stress reds + draft status.

Review state

• AI Review: ✅ Approve; 0 unresolved review threads; no outstanding change-requests. LLVM coverage report shows 90/90 (100%) of changed C/C++ lines covered.
• Diff reviewed end-to-end and confirmed sound: makeRegexpPatternFromGlobs rewrites a whole-segment **/ to ([^/]*/)* (legacy character-by-character expansion preserved for non-whole-segment ** such as ?**/, a**, and ***), and listFilesWithRegexpMatchingImpl applies the post-** suffix at the current directory (zero-level branch) and descends keeping the whole **/... pattern, so the globstar matches zero, one, or more directory levels — with an unordered_set dedup scoped per brace-expansion alternative to avoid double-emission from adjacent globstars while preserving per-alternative reads.

CI

The current report (563701cd) has only two reds, both the chronic Hung check failed, possible deadlock found flake — Stress test (amd_tsan) and Stress test (arm_tsan):

• Both are the open tracking issue Hung check failed, possible deadlock found #107941 (the CI summary itself links them there). The hung-thread stacks are generic TCPHandler::run → executeASTFuzzerQueries frames with zero glob/parseGlobs/StorageFile frames — unrelated to this change.
• The workflow run (28458923351) is still in progress, so the failed jobs cannot be re-run yet (This workflow is already running); they should be re-run once the run completes.

Not re-merging master

The branch is 244 commits behind, but the merge base is from 2026-06-29 (< 1 day old, well under the one-week threshold), master has not touched parseGlobs.cpp/StorageFile.cpp since then, and the only reds are the #107941 flake (not fixed on master, so a merge cannot clear it). A fresh full CI run is currently in progress on this head SHA, which a re-merge would discard. Holding off.

This PR is a draft — it appears ready apart from marking it ready for review and re-running the flaky Hung check jobs.

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered: 97/97 (100.00%) · Uncovered code

Full report · Diff report

continue-pr re-check (head 563701cd) — no code change

Nothing actionable on the public side: AI Review ✅ Approve, 0 unresolved review threads, changed-line coverage 100%, mergeable: MERGEABLE / UNSTABLE (all required checks pass). The only reds are the two stress jobs, both the chronic Hung check failed, possible deadlock found flake.

Reconfirmed the hung check is unrelated — verified specifically against this glob change

Since this PR touches the recursive directory descent in listFilesWithRegexpMatchingImpl, the one real failure mode to rule out for a hang is an infinite/degenerate globstar recursion. I downloaded the Stress test (amd_tsan) run logs and checked directly:

• listFilesWithRegexpMatching — the recursive globstar-descent function this PR modifies — is absent from every stress log (0 occurrences across all 8 stress_test_run_*.txt). If the change caused a descent hang, this frame would appear in the hung-thread dump; it does not.
• The StorageFile.cpp frames that do appear are ordinary exception traces from StorageFile::StorageFile / getTableStructureAndFormatFromFileImpl (schema inference), not the listing recursion.
• The captured hung-thread stack is the generic executeQueryImpl → TCPHandler::runImpl → TCPHandler::run fuzzer path, with zero glob / parseGlobs / StorageFile listing frames.

This is the open tracking flake #107941 (the CI summary itself links both jobs there) — master-wide (hundreds of PRs/day, fires on master too), not caused by this change.

Actions this pass

• Re-triggered the two failed stress jobs (run 28458923351 --failed, now queued) — the previous attempt completed and re-hit the same flake, so this is a fresh shot at a green board.
• No re-merge: merge base 0cca85ea1a5 is from 2026-06-29 (< 1 week), and master has not touched parseGlobs.cpp / StorageFile.cpp since then, so a merge cannot clear the flake.
• No code change: no outstanding review feedback to address.

The PR is still a draft — apart from re-running the flaky Hung check jobs, it appears ready for marking ready-for-review.