Skip to content

[CBRD-26956] Partially revert CBRD-26663: drop variable header + length, keep only CHAR fixed-to-variable + compression (feature → develop)#7326

Open
jongmin-won wants to merge 6 commits into
developfrom
feature/revert-CBRD-26663-char-vlen
Open

[CBRD-26956] Partially revert CBRD-26663: drop variable header + length, keep only CHAR fixed-to-variable + compression (feature → develop)#7326
jongmin-won wants to merge 6 commits into
developfrom
feature/revert-CBRD-26663-char-vlen

Conversation

@jongmin-won

@jongmin-won jongmin-won commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

http://jira.cubrid.org/browse/CBRD-26956

Purpose

CBRD-26663(CHAR/VARCHAR 통합, #7164)에서 도입한 가변 헤더(Tiny/Small/Medium/LARGE) + length(글자 수) 헤더 저장이 VARCHAR 저장 용량을 늘려 성능 저하(YCSB)를 유발했다.
이를 통합 전으로 되돌리되, Sysbench에서 이득이 확인된 CHAR 고정→가변 저장 및 압축만 유지한다.

Implementation

본 변경은 두 단계로 구성된다.

1) CBRD-26663 전체 revert (#7164)

  • 스쿼시 머지 83b29b02 를 전체 revert → 통합 전 문자열 저장 구조로 복원.
  • VARCHAR: byte_size 기반 헤더로 복귀, 가변 헤더 4종 분기 및 length(char_count) 필드 제거.
  • 대상: object_representation.*, object_primitive.*, string_opfunc.*, query_executor.c, query_aggregate.cpp, object_domain.c, loaddb/*, parse_evaluate.c, network_interface_sr.cpp 등 18개 파일.

2) CHAR 고정→가변 재이식

  • 통합 전 VARCHAR(tp_String)의 가변 저장 경로(mr_*_string)를 본떠 CHAR(tp_Char) 경로를 작성. 헤더는 VARCHAR와 동일한 byte_size only(length·가변 tier 없음).
  • CHAR 전용 추가: 저장/cast 시 trailing-space pad(precision까지 0x20) 유지, 비교 시 trailing-space 무시 semantic 유지.
  • tp_Char.variable_p = 1, pr_midxkey_element_disk_size CHAR element 허용.
  • 압축: 통합 전 VARCHAR 압축 경로를 공유하여 CHAR도 byte_size 임계 이상에서 압축.

Remarks

  • 외부 SQL 동작 변화 없음 — 비교/정렬/문자열 함수 결과 동일. 글자 수 기반 함수(LENGTH/INSTR/CONCAT 등)는 결과는 같으나 글자 수를 매번 계산(통합 전과 동일 비용).
  • TC는 별도 PR — 동반 testcases [CBRD-23914] Deprecated usage of ftime #2751 중 VARCHAR 관련 답안은 되돌리고, CHAR storage 답안(disk_size 등)은 CHAR 가변화에 맞춰 재생성.

…th, keep only CHAR fixed-to-variable + compression
@jongmin-won jongmin-won self-assigned this Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

❌ TC Merge Gate — Merge Blocked

One or more TC PRs are still open. Please merge or close them before merging this PR.

TC Repositories & Branches:

  • cubrid-testcases: TC PR tc/pr-7326 is open — must be merged or closed first
  • cubrid-testcases-private-ex: TC PR tc/pr-7326 is open — must be merged or closed first

Steps to unblock:

  1. Merge or close all TC PRs listed above.
  2. Re-run this check: Actions tab → TC Merge Gate → Re-run failed jobs

@github-actions

Copy link
Copy Markdown

🧪 TC Test Environment Ready

CircleCI Testing:

  • CircleCI will automatically test using the branches below.

TC Repositories & Branches:

Next Steps:

  1. Wait for CircleCI tests to complete
  2. If CircleCI tests failed, please check the test results and fix the issues.
  3. When ready to merge this PR, please merge the TC PR first, then merge this PR.

jongmin-won and others added 2 commits July 1, 2026 18:35
…th, keep only CHAR fixed-to-variable + compression (→ feature/revert-CBRD-26663-char-vlen) (#7327)

http://jira.cubrid.org/browse/CBRD-26956

Fully revert CBRD-26663 (unified variable-length storage for CHAR/VARCHAR), which caused the
benchmark (YCSB/sysbench) performance regression, then re-port only the CHAR storage improvement
that does not introduce the regression.

1. Full revert of CBRD-26663 (squash merge 83b29b0)
   - Restore the pre-unification string storage layout: VARCHAR uses the byte_size-based header and
     CHAR uses fixed-length storage.
   - Remove the variable headers (Tiny/Small/Medium/Large) and the length (character-count) field
     that increased VARCHAR storage size and caused the performance regression.

2. Re-port CHAR to variable-length storage with LZ4 compression
   - Re-port only the CHAR-related part of CBRD-26663 so CHAR is stored using variable-length
     storage with LZ4 compression, like VARCHAR, while preserving CHAR's precision-padding
     semantics.
   - Add the pr_pad_char_to_precision() helper so precision padding is applied uniformly in the
     storage layer.
   - Extend the string compression/decompression path and replace VARCHAR-only checks with
     TP_IS_CHAR_TYPE so CHAR follows the same path.

3. Fix loaddb regression + uninitialized partition-expression serialization
   - xasl_to_stream.c: The unwritten tail of the serialization stream was left uninitialized,
     making the character-count computation for CHAR values stored in partition expressions
     non-deterministic and causing intermittent ER_TF_SIZE_MISMATCH errors.
   - load_sa_loader.cpp: Clean up the initialization and cleanup of the hand-built DB_VALUE to
     remove uninitialized references and padding-buffer leaks in the variable-length CHAR storage
     path.
   - object_primitive.c: Cache char_count where padding is guaranteed to avoid a multi-byte
     re-walk.

4. Review feedback
   - xasl_to_stream.c: Zero only the unwritten tail instead of the entire buffer.
   - object_representation_sr.c: Relax the attribute-length assertion since empty strings (e.g.
     COMMENT '') can be stored legitimately (corruption cases are still detected).
   - string_opfunc.c: Tidy the dest_prec assignment (behavior unchanged).
…BRD-26663-char-vlen

Resolve conflict in src/storage/page_buffer.c: adopt develop's THREAD_ENTRY-based
pgbuf fix/unfix monitor counters (CBRD-26941) over the feature branch's thread_counters
shard array; both replace the same base global counters, so the canonical develop
implementation is kept.
@jongmin-won
jongmin-won marked this pull request as ready for review July 2, 2026 04:44
@jongmin-won
jongmin-won removed the request for review from shparkcubrid July 2, 2026 04:44
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'upstream/d..." | Re-trigger Greptile

Comment thread src/object/object_primitive.c
Comment thread src/object/object_domain.c
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'upstream/d..." | Re-trigger Greptile

@jongmin-won

Copy link
Copy Markdown
Contributor Author

/run all

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'upstream/d..." | Re-trigger Greptile

@jongmin-won

Copy link
Copy Markdown
Contributor Author

/run all

@jongmin-won

Copy link
Copy Markdown
Contributor Author

/run all

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'upstream/d..." | Re-trigger Greptile

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