[CBRD-27251] Normalize histogram string keys by the column type and re-pad CHAR values for LIKE/REGEXP estimates - #7977
Open
soheejung-cs wants to merge 6 commits into
Open
soheejung-cs wants to merge 6 commits into
soheejung-cs wants to merge 6 commits into
Conversation
❌ TC Merge Gate — Merge BlockedOne or more TC PRs are still open. Please merge or close them before merging this PR. TC Repositories & Branches:
Steps to unblock:
|
🧪 TC Test Environment ReadyCircleCI Testing:
TC Repositories & Branches:
Next Steps:
|
soheejung-cs
marked this pull request as ready for review
September 18, 2026 03:38
Contributor
 기능상 차단 결함은 확인되지 않아 병합 가능한 상태로 보이지만, 메모리 소유권 주석을 바로잡고 핵심 정규화 계약에 대한 자동화 회귀 테스트를 보강하는 것이 좋습니다. Reviews (1) · Last reviewed commit: "[CBRD-27251] Fix stale value-origin comm..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
added 2 commits
September 18, 2026 14:15
…e-pad CHAR values for LIKE/REGEXP estimates The sampler stripped trailing spaces when the heap value (= column) type was CHAR, but the client probe stripped when the constant's type was CHAR. The type checker leaves a CHAR literal compared to a VARCHAR column as CHAR, so `varchar_col = 'abcd '` probed the stripped key 'abcd' against unstripped VARCHAR MCVs and matched the wrong entry (sel 0.125 for 100 of 1600 rows, expected 0.0625). - hist::string_key_size_for_column (histogram_reader.hpp): the single definition of the rule (strip iff the COLUMN is CHAR); used by the sampler (extract<std::string>, ndv_hll_hash) and the probe (histogram_extract_key, which now takes the histogram's column type). - histogram_repad_char_value (histogram_cl.cpp): LIKE/REGEXP run on the padded heap value at execution, so re-pad stored CHAR MCV/bucket values to the column precision (in characters, per codeset) before matching. Without data_type on the column node the estimate falls back to the stripped comparison. Histogram blob format is unchanged; no re-collection needed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBMgWWzmrvFiQBSBUiL2YD # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다. # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다.
After histogram_repad_char_value () the matched value may live in pad_buf rather than the histogram blob; say so where the comment described its lifetime (self-review finding, no code change). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBMgWWzmrvFiQBSBUiL2YD # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다. # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다.
soheejung-cs
force-pushed
the
CBRD-27251
branch
from
September 18, 2026 05:16
107011c to
067b319
Compare
db_make_varchar () borrows the buffer instead of copying it (db_make_db_char sets medium.buf to the caller's pointer and leaves need_clear false), so the comment claiming it copies the value out described the ownership contract backwards. The operand is safe because db_string_like () consumes it in the same call, before the next iteration re-pads pad_buf -- say that instead. Review comment from greptile-apps on PR CUBRID#7977. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBMgWWzmrvFiQBSBUiL2YD # 제목: 변경 요약 (50자 이내) (refactor, hotfix, feature) # 본문: 변경 사항에 대한 자세한 설명 # - 주요 변경 사항 1 # - 주요 변경 사항 2 # 참고: 이 라인 아래의 내용은 커밋 메시지에서 제거됩니다.
Contributor
Author
|
/run all |
…not the query http://jira.cubrid.org/browse/CBRD-27251 The CHAR re-padding added for LIKE/REGEXP estimates took the column width and charset from the query's column node (PT_NAME->data_type). Both describe the column the histogram was collected on, not the query text, and the column node is not guaranteed to carry them: a node without data_type yielded precision 0, which disables re-padding, so the estimate silently fell back to matching the stripped stored value. The same two properties were read from two different places -- the value type from the blob, the width and charset from the parse tree. - Width now travels in the blob. Header v2 reserved four bytes at offset 28 that nothing ever read (the builder wrote a constant 0); that field becomes HV2_PRECISION and carries the column's declared width in characters. HistogramReader exposes it as value_precision (). Offsets, header size and format version are unchanged, so old and new binaries read each other's blobs; a blob written before this change reports 0, which means "width unknown" and keeps re-padding off until the statistics are collected again. - The sampler picks the width up from the data it already reads. A heap CHAR value is materialized through its domain (mr_readval_char_internal () inits the DB_VALUE with domain->precision), so the first non-null row of the column reports it; col_collector records it once and passes it down through build_blob () to HistogramBuilder::build (). The parallel path needs one more step: parallel_scan_merge_multi () builds the final collector fresh and only merges peers into it, so merge_peers () takes the width the workers saw. - Charset comes from the collation instead. Both estimate paths already resolve the collation they match under, and a collation determines its charset, so histogram_get_like_selectivity () and histogram_get_rlike_selectivity () read it there. LANG_GET_COLLATION indexes the collation array without a bounds check in release builds, so the LIKE path uses lang_get_collation () with a NULL check, as the REGEXP path already did. After this the re-padding depends only on the blob and the resolved collation. Existing statistics keep working, but their CHAR LIKE/REGEXP estimates stay at the pre-CBRD-27251 behaviour until update statistics runs again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zfELL2BypfaeCFB9fj8Dd
Contributor
Author
|
/run all |
Contributor
Author
|
/run rerun 35686538794 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
http://jira.cubrid.org/browse/CBRD-27251
Purpose
문자열 컬럼의 히스토그램에서 값을 적어 넣는 쪽과 읽어 맞춰보는 쪽이, CHAR 의 자리 채움 공백을 떼어내는 기준을 서로 다르게 잡고 있었습니다.
histogram_sampler_sr.cpp,update statistics가 서버에서 돌리는 샘플러). 힙에 저장된 값을 그대로 읽으므로 판단 기준이 컬럼 타입입니다. 컬럼이 CHAR 이면 채움 공백을 떼고 저장합니다.histogram_cl.cpp의histogram_extract_key,query_planner.c가 플랜을 세우며 선택도를 추정할 때 호출). 여기서는 질의문에 적힌 상수(리터럴)의 타입으로 판단했습니다. 문자열 리터럴은 CHAR 이므로, 컬럼이 VARCHAR 여도 상수에서 공백을 떼어냈습니다.같은 히스토그램을 서로 다른 기준으로 다루니 VARCHAR 컬럼에서 어긋납니다. 타입 검사기는 CHAR↔VARCHAR 비교에서 리터럴을 컬럼 타입으로 강제변환하지 않으므로
varchar_col = 'abcd '의 상수는 CHAR 인 채로 남고, 최적화기만 이를'abcd'로 줄여 엉뚱한 MCV 에 맞췄습니다 (1,600행 재현에서 참값 100행인데 sel 0.125 = 200행).두 쪽이 맞춰야 할 정답은 서버 실행기가 실제로 적용하는 비교 규칙입니다. 매뉴얼 「데이터 타입 > 문자열 > 비교 규칙」이 정한 대로, 후행 공백을 무시하는 것은 양쪽이 모두 고정 길이(CHAR) 일 때뿐이고 한쪽이 가변 길이(VARCHAR) 면 후행 공백을 포함해서 비교합니다 (
db_string_compare, 기본값ignore_trailing_space=no). 즉 VARCHAR 컬럼에서'abcd'와'abcd '는 엄연히 다른 값이고, 히스토그램의 추정도 이 규칙을 따라야 합니다.같은 뿌리에서 나온 두 번째 결함 — CHAR 컬럼의 LIKE / REGEXP. CHAR(8) 컬럼은 매뉴얼대로 선언 길이까지 오른쪽을 공백으로 채워 저장하므로 힙에는
'abcd '가 들어 있고, 실행기의 LIKE / REGEXP 는 이 채워진 값에 패턴을 맞춥니다. 반면 히스토그램에는 공백을 떼어낸'abcd'만 남아 있어LIKE 'abcd %',LIKE 'abcd ',REGEXP '^abcd +$'같이 공백 자리를 요구하는 패턴이 거의 아무것도 맞지 않았습니다 (참값 340행인데 sel 0.003 = 5행).Implementation
hist::string_key_size_for_column (column_type, s, size)(histogram_reader.hpp, inline). 판단 기준을 컬럼 타입 하나로 못박아, CHAR 컬럼이면 채움 공백을 뺀 길이를, 그 외(VARCHAR·BIT)는 바이트를 그대로 돌려줍니다. 서버측 샘플러(extract<std::string>·ndv_hll_hash)와 클라이언트측histogram_extract_key가 같은 함수를 부르므로 두 쪽이 어긋날 수 없습니다.histogram_extract_key에 히스토그램 컬럼 타입 인자를 추가하고(reader.value_type ()), 리터럴 타입으로 분기하던 코드를 없앴습니다. 호출부는histogram_get_equal_selectivity·histogram_get_comp_selectivity두 곳입니다.histogram_repad_char_value (value, column_type, precision, codeset, buf)(histogram_cl.cpp). CHAR 컬럼이면 히스토그램에 저장된 MCV·버킷 경계값을 컬럼 정밀도(문자 수,intl_char_count)까지 공백으로 실행기가 보는 모양대로 되돌려like_match_value(db_string_like) 와rlike_match_string(cubregex::search) 에 넣습니다. 정밀도는 히스토그램 blob 헤더에서, 코드셋은 이미 결정된 콜레이션에서 읽습니다(아래 항목). 질의문의 컬럼 노드는 보지 않습니다.data_type에서 읽었습니다. 그 노드가data_type을 달고 오지 않으면 정밀도가 0 이 되어 되채우기가 조용히 꺼지고 이 PR 이 고친 추정이 그 경로에서 되살아납니다. 헤더 v2 의 오프셋 28 은 빌더가 상수 0 만 쓰고 읽는 곳이 없던 예약 4바이트였으므로, 이를HV2_PRECISION으로 삼아 컬럼 폭(문자 수)을 적고HistogramReader::value_precision ()으로 꺼냅니다. 샘플러는 이미 읽고 있는 데이터에서 폭을 얻습니다 — 힙 CHAR 값은 도메인을 거쳐 만들어지므로(mr_readval_char_internal ()이domain->precision으로 DB_VALUE 를 초기화) 널이 아닌 첫 행이 컬럼 폭을 들고 옵니다. 병렬 경로의 최종 컬렉터는 행을 직접 먹지 않고 병합만 하므로merge_peers ()가 워커가 본 폭을 가져옵니다. 코드셋은 두 추정 경로가 이미 맞춰 둔 콜레이션이 결정하므로 거기서 읽습니다(LANG_GET_COLLATION은 release 에서 경계 검사 없는 배열 접근이라 LIKE 쪽도 REGEXP 쪽처럼lang_get_collation ()+ NULL 검사를 씁니다).update statistics를 다시 돌린 뒤부터 동작합니다(등가·범위 추정은 재수집 없이 그대로입니다).Remarks
검증(goto release 빌드
a82e6d6ae, demodb 1,600행:'abcd'×200 ·'abcd '×100 ·'abcd '×40 ·'xyz'×60 · 필러 1,200,update statistics ... with fullscan, 300 buckets,set optimization level 513의(sel N)과count(*)비교,ignore_trailing_space=no):t_vc.c = 'abcd '(varchar(32))t_vc.c = 'abcd 't_vc.c = 'abcd't_ch.c = 'abcd'/= 'abcd '(char(8))t_ch.c LIKE 'abcd %'(히스토그램 LIKE 추정)t_ch.c LIKE 'abcd 't_ch.c REGEXP '^abcd +$'t_ch.c REGEXP 'abcd$'t_vc.c LIKE 'abcd %'(범위 재작성, VARCHAR 는 되채우기 대상 아님)ignore_trailing_space=yes에서도t_vc.c = 'abcd'등가 추정 0.2125(340행)는 그대로입니다.재현 SQL 은 JIRA 에 첨부했습니다(
cbrd27251_repro.sql). 조합 TC 는 통계 구현이 바뀔 때마다sel값이 흔들려 별도 발번하지 않았습니다(2026-09-17 결정).범위 밖으로 남긴 것:
ignore_trailing_space=yes에서의 문자열 범위 비교(버킷 경계는 바이트 순서), CHAR↔VARCHAR 조인의 MCV 매칭. 별건으로 기록했습니다.upstream/develop
051d5f61d위로 리베이스했습니다. develop 신규 커밋은 히스토그램 파일을 건드리지 않아 충돌이 없었고, 위 검증 결과는 리베이스 전 트리(a82e6d6ae)에서 얻은 것입니다.리뷰 대응:
db_make_varchar()가 버퍼를 복사한다고 적은 주석을 고쳤습니다(067b319c9). 실제로는db_make_db_char()가medium.buf에 포인터만 넣고need_clear를 false 로 두므로 빌려 쓰는 것이고, 안전 조건은db_string_like()가 같은 호출 안에서 값을 소비한다는 점입니다.rlike_match_string()은std::string으로 실제 복사하므로 기존 주석을 유지했습니다.자기 리뷰(harness-review)에서 걸러낸 것: 되채우기 후
value의 출처(blob 또는pad_buf)를 잘못 안내하던like_match_value·rlike_match_string의 주석을 고쳤습니다. 수치 표는 이 Remarks 에 옮겼습니다.2차 자기 리뷰(harness-review, HEAD
9cd1c1274)에서 걸러낸 것: 되채우기가 필요로 하는 컬럼 정밀도·코드셋을 질의문에서 읽고 있던 것을 blob·콜레이션으로 옮겼습니다(위 Implementation 두 항목). 그 과정에서 병렬 수집의 최종 컬렉터가 폭을 못 받는 경로도 함께 막았습니다.CAST(...)상수는 접히지 않아 히스토그램 경로(PC_CONST)를 타지 않습니다 — 이 PR 과 무관합니다.