Skip to content

Treat known non-defect query failures as expected, not as CI failures - #3

Merged
litkhai merged 1 commit into
masterfrom
fix/expected-failures
Aug 11, 2026
Merged

Treat known non-defect query failures as expected, not as CI failures#3
litkhai merged 1 commit into
masterfrom
fix/expected-failures

Conversation

@litkhai

@litkhai litkhai commented Aug 11, 2026

Copy link
Copy Markdown
Owner

The clickhouse job was red because tools/verify.sh exited non-zero on any query failure, and ClickHouse has exactly one: q61 divides by a count(*) that the small verification fixture leaves at 0.

clickhouse job 이 빨간 상태였던 이유는 tools/verify.sh 가 쿼리 실패가 하나라도 있으면 non-zero 로 종료했기 때문입니다. ClickHouse 에는 정확히 하나 있습니다. q61 이 검증 픽스처에서 0 이 되는 count(*) 로 나눕니다.

That is a fixture-size artifact, not a query or engine defect — the query is correct and runs on a real dataset. Leaving CI red for something nobody can act on just teaches people to ignore it.

이는 쿼리나 엔진의 결함이 아니라 픽스처 크기에서 오는 현상입니다. 쿼리 자체는 정상이고 실제 데이터셋에서는 실행됩니다. 아무도 조치할 수 없는 이유로 CI 를 빨간 상태로 두면 사람들이 CI 를 무시하게 됩니다.

Change / 변경

verify.sh classifies failures against expected_failures():

Situation / 상황 Result / 결과
failures match the list / 실패가 목록과 일치 run passes, reason printed / 통과, 이유 출력
a failure not on the list / 목록에 없는 실패 run fails, listed as unexpected / 실패, unexpected 로 표시
a listed query now passes / 목록의 쿼리가 통과 run warns the list is stale / 목록이 낡았다고 경고

The known state is pinned and drift is caught in both directions, so the list cannot quietly rot.

알려진 상태를 고정하고 양방향 드리프트를 잡으므로 목록이 조용히 낡지 않습니다.

Corrected numbers / 수치 정정

CI pulled a newer ClickHouse image than my local run had, and q30/q81 passed there. I pulled 26.7.3 and confirmed: they were a 25.11 limitation resolving an outer CTE alias inside a subquery over a second instance of the same CTE, and they are fixed upstream.

CI 가 로컬보다 새 ClickHouse 이미지를 받았고 q30/q81 이 거기서 통과했습니다. 26.7.3 을 직접 받아 확인했습니다. 25.11 의 제약이었고 상류에서 수정되었습니다.

Engine Before Now
PostgreSQL 103/103 103/103
ClickHouse ⚠️ 100/103 (25.11) 102/103 (+1 expected, 26.7)
StarRocks 103/103 103/103

Docs updated accordingly, with the 25.11 behaviour kept as a historical note.

Portability note / 이식성 참고

The first version of this used mapfile, which does not exist in bash 3.2. tools/verify.sh runs on the host, so it must work on macOS bash 3.2 as well as CI's bash 5 — the mirror image of the RETURN trap bug in #2. It now uses space-separated strings, and both bash versions parse it.

이 변경의 첫 버전은 bash 3.2 에 없는 mapfile 을 사용했습니다. tools/verify.sh호스트에서 실행되므로 CI 의 bash 5 뿐 아니라 macOS 의 bash 3.2 에서도 동작해야 합니다. #2RETURN 트랩 버그와 정반대 방향의 문제입니다. 공백 구분 문자열로 바꾸었고 두 bash 버전 모두 파싱합니다.

🤖 Generated with Claude Code

The clickhouse job was red because tools/verify.sh exited non-zero on any
query failure, and ClickHouse has exactly one: q61 divides by a count(*)
that the small verification fixture leaves at 0. That is a fixture-size
artifact, not a query or engine defect — the query is correct and runs on
a real dataset. Leaving CI red for something nobody can act on just
teaches people to ignore it.

verify.sh now classifies failures against expected_failures():
  - failures matching the list      → run still passes, reason printed
  - any failure not on the list     → run fails, listed as unexpected
  - a listed query that now passes  → run warns that the list is stale

So the known state is pinned and drift is caught in both directions.

Also corrects the ClickHouse numbers. CI pulled a newer image than the
local run had and q30/q81 passed there, so I pulled 26.7.3 and confirmed:
they were a 25.11 limitation resolving an outer CTE alias inside a
subquery over a second instance of the same CTE, and they are fixed
upstream. ClickHouse is 102/103, not 100/103.

  postgres    103/103
  clickhouse  102/103 (+1 expected)
  starrocks   103/103

One portability note while writing this: the first version used mapfile,
which does not exist in bash 3.2. tools/verify.sh runs on the HOST, so it
has to work on macOS bash 3.2 as well as CI's bash 5 — the mirror image of
the RETURN trap bug in the previous commit. It now uses space-separated
strings, and both bash versions parse it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@litkhai
litkhai merged commit 63ee712 into master Aug 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant