Skip to content

[Backport stable-26-3] PR #49216: Reduce test size under sanitizer - #49238

Merged
ydbot merged 1 commit into
stable-26-3from
cherry-pick-stable-26-3-260807-084907
Aug 7, 2026
Merged

[Backport stable-26-3] PR #49216: Reduce test size under sanitizer#49238
ydbot merged 1 commit into
stable-26-3from
cherry-pick-stable-26-3-260807-084907

Conversation

@ydbot

@ydbot ydbot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Changelog entry

Reduce test size under sanitizer

Description for reviewers

Original PR(s)

Metadata

Git Cherry-Pick Log

=== Cherry-picking 8ed5b62 ===
Auto-merging ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp
[cherry-pick-stable-26-3-260807-084907 0dc660c2ccf] Reduce test size under sanitizer (#49216)
 Author: zverevgeny <zverevgeny@ydb.tech>
 Date: Fri Aug 7 11:39:57 2026 +0300
 1 file changed, 17 insertions(+), 5 deletions(-)

PR was created by cherry-pick workflow run

@ydbot
ydbot requested a review from a team as a code owner August 7, 2026 08:49
@ydbot ydbot self-assigned this Aug 7, 2026
@ydbot
ydbot enabled auto-merge August 7, 2026 08:49
@ydbot

ydbot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Run Extra Tests

Run additional tests for this PR. You can customize:

  • Test Size: small, medium, large (default: small, medium)
  • Test Targets: any directory path (default: ydb/)
  • Sanitizers: ASAN, MSAN, TSAN
  • Coredumps: enable for debugging (default: off)
  • Additional args: custom ya make arguments

▶  Run tests

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

2026-08-07 08:50:54 UTC Pre-commit check linux-x86_64-relwithdebinfo for 2753ec4 has started.
2026-08-07 08:52:18 UTC Artifacts will be uploaded here
2026-08-07 08:53:53 UTC ya make is running...
🟢 2026-08-07 08:58:13 UTC Tests successful.

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
606 606 0 0 0 0

🟢 2026-08-07 08:58:16 UTC Build successful.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

2026-08-07 08:50:54 UTC Pre-commit check linux-x86_64-release-asan for 2753ec4 has started.
2026-08-07 08:53:34 UTC Artifacts will be uploaded here
2026-08-07 08:55:04 UTC ya make is running...
🟡 2026-08-07 09:04:18 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
606 605 0 1 0 0

2026-08-07 09:04:36 UTC ya make is running... (failed tests rerun, try 2)
🟢 2026-08-07 09:05:20 UTC Tests successful.

Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
51 (only retried tests) 51 0 0 0 0

🟢 2026-08-07 09:05:23 UTC Build successful.

@robot-vibe-db

robot-vibe-db Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

AI Review Summary

Verdict: No critical issues found

Critical issues

No critical issues found.

Other findings

  • Nit | Low: numColumns, alterColumnsFrom, alterColumnsTo are constant-valued (no PlainOrUnderSanitizer) -- they serve as named constants replacing magic numbers, which is a good readability improvement but not strictly necessary for the sanitizer fix. No action needed. -- ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp:16531-16534

Details

This is a clean backport cherry-pick of PR #49216. The diff is identical to the original (only line numbers differ due to the stable-26-3 branch). The change is correct and follows established patterns in the codebase:

  1. NSan::PlainOrUnderSanitizer usage matches the well-established pattern used in many other test files (btree_cow_ut.cpp, kqp_scripting_ut.cpp, kqp_service_ut.cpp, etc.).

  2. Row count of 100 under sanitizers is reasonable -- many other tests in the same file already use BuildBatch(100) as their standard batch size, so this does not undermine test coverage.

  3. Column count is preserved at 9900 (not reduced under sanitizers), which correctly preserves the core purpose of the test: validating wide-schema behavior with approximately 10,000 columns.

  4. Expected result strings are correctly made dynamic via TStringBuilder() << "[[" << numRows << "u]]", which produces "[[10000u]]" in plain builds and "[[100u]]" under sanitizers. The ui64 type streams correctly as a decimal string.

  5. No magic number mismatches: all four original hardcoded values (9900, 10000, 9900, 9999) are faithfully captured in the named constants.

  6. The comment accurately states "9901-column": 9900 data columns + 1 "id" primary key column = 9901 total columns.

No correctness, crash, data-loss, security, concurrency, or performance issues found.


This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 -- it helps us improve the review bot.

// full column count (the test validates wide-schema behavior) but reduce
// the row count so the BulkUpsert fits within the timeout. See
// NSan::PlainOrUnderSanitizer.
const ui64 numColumns = 9900;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: Nit
Confidence: Low

numColumns, alterColumnsFrom, and alterColumnsTo are constant-valued -- they don't use PlainOrUnderSanitizer and always evaluate to 9900, 9900, and 9999 respectively. They serve purely as named constants replacing magic numbers.

This is a nice readability improvement (makes the relationship between the values explicit and easier to change in the future), but it goes slightly beyond the minimal sanitizer fix. No action needed -- just noting for completeness.

@ydbot
ydbot merged commit 350e7f3 into stable-26-3 Aug 7, 2026
13 checks passed
@ydbot
ydbot deleted the cherry-pick-stable-26-3-260807-084907 branch August 7, 2026 16:46
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.

3 participants