Repository navigation
[fix](function) Reject a non-constant seed in array_shuffle - #68438
Merged
Merged
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
`array_shuffle(arr, seed)` only read the seed of the first row in a block
and built one `std::mt19937` from it. Every later row kept drawing from the
same random stream, so its own seed was ignored and its result depended on
how many rows came before it in the block.
Reproduce (all rows in one block):
CREATE TABLE t (k INT, a ARRAY<INT>, s BIGINT) DUPLICATE KEY(k)
DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES ("replication_num" = "1");
INSERT INTO t VALUES (1,[1,2,3,4,5],1),(2,[1,2,3,4,5],2),(3,[1,2,3,4,5],1);
SELECT k, array_shuffle(a, s) FROM t ORDER BY k;
Before: k=2 gave [1,3,2,5,4] while array_shuffle([1,2,3,4,5], 2) gives
[4,1,3,5,2], and k=3 (seed 1) differed from k=1 (seed 1). Even a constant
seed gave a different result on every row.
Fix: when a seed is given, re-seed the generator with the row's own seed
before shuffling that row, for both column and constant seeds. The result
of a row now depends only on its array and seed, and matches the constant
call. Without a seed the function still uses one time-based stream per
block, as before.
The `array_shuffle` cases in test_array_functions were commented out
because "the result depends on the row position in the column"; they are
stable now and enabled again.
### Release note
Fix `array_shuffle(arr, seed)` ignoring the per-row seed. With a seed, a
row's result now depends only on its array and seed, so rows with the same
array and seed (including a constant seed) get the same permutation.
### Check List (For Author)
- Test: Regression test / Unit Test
- BE UT function_array_shuffle_test (fails before the fix, passes after)
- Regression test_array_shuffle_seed (new), test_array_functions
(re-enabled shuffle cases), test_size_function
- Behavior changed: Yes. With a constant seed, all rows with the same array
now get the same permutation instead of different ones per row.
- Does this need documentation: No
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 27825 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes: two issues are documented inline.
Checkpoint conclusions:
- Goal and data correctness: the per-row reseed design fixes the original block-order dependence for ordinary vector and constant seeds, but nullable execution can still fail rows whose propagated result must be NULL (inline P1).
- Scope and compatibility: the change is focused; FE/BE signatures and the
shufflealias are unchanged, and the one-argument time-seeded stream remains unchanged. There is no storage-format, protocol, configuration, or rolling-upgrade surface. - Concurrency, lifecycle, and memory: generators and callbacks are stack-local per execution, with no shared mutable state, lock ordering, static-initialization, or persistent ownership concern. New includes are confined to the
.cpp. - Null/error handling and parallel paths: default nullable wrapping is the blocking correctness gap; both function spellings share the same implementation and tests.
- Tests and expected results: the new BE test is discovered by the existing recursive test target and supplies a direct one-block oracle. Added/re-enabled regression results are ordered or order-invariant and internally consistent across aliases, constants, vector seeds, NULLs, empty arrays, lengths, and element types. They do not cover the combined-null failure or the no-op reseed cost identified inline.
- Performance: empty and singleton seeded arrays initialize the full
mt19937state per row despite requiring no shuffle (inline P2). - Persistence, transactions, writes, and observability: not applicable to this scalar-function change; no additional logging or metrics are needed.
- User focus: no additional focus was provided.
Validation was static only, as required by the review runner; I did not run builds or tests and do not treat author/CI claims as independent execution evidence.
Contributor
TPC-H: Total hot run time: 27234 ms |
Contributor
TPC-DS: Total hot run time: 153556 ms |
Contributor
ClickBench: Total hot run time: 24.01 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#44901 Problem Summary: With the per-row seed fix, every row's seed goes through `cast_set<uint32_t>`, which throws INTERNAL_ERROR for a negative seed or one above 4294967295: SELECT array_shuffle([1, 2, 3, 4, 5], -1); -- [INTERNAL_ERROR]value -1 cast to type j out of range [0,4294967295] Default NULL handling runs the function on every physical row, so this also fails rows whose result is NULL: a NULL array next to seed -1 in the same block, or `NULLIF(s, -1)`, which keeps -1 under the NULL. The range check came from apache#44901 (a macOS build fix in 4.0); 2.1/3.0/3.1 accepted these seeds. Rows with 0 or 1 element also re-seeded the generator, which fills the 624-word mt19937 state, although shuffling them draws no random numbers. Fix: - Like ClickHouse's arrayShuffle, use the whole BIGINT as a 64-bit seed with std::mt19937_64 (the engine uniform() already seeds per row). Any BIGINT is a valid seed, seeds with the same low 32 bits (such as -1 and 4294967295) no longer give the same result, and no row can fail on its seed. The query above now returns [3, 1, 4, 2, 5]. - Skip arrays with fewer than 2 elements before seeding the generator. - The no-seed path uses the same engine; it is still one time-based stream per block. - The UT helper now stops when execute fails, instead of reading the missing result column and aborting the whole test binary. ### Release note array_shuffle(arr, seed) accepts any BIGINT seed again, including negative values. The permutation for a given seed differs from earlier versions. ### Check List (For Author) - Test: Unit Test / Regression test - BE UT function_array_shuffle_test: 4/4 pass; the new any_bigint_seed case fails on the previous code with the INTERNAL_ERROR above - Regression test_array_shuffle_seed (new cases: negative seeds, the BIGINT limits, a NULL array with seed -1, NULLIF(s, -1)), test_array_functions, test_size_function, test_array_function_doc (.out regenerated, only seeded shuffle results changed), test_array_functions_by_literal, test_array_zip_array_enumerate_uniq - Behavior changed: Yes. Negative seeds and seeds above 4294967295 work instead of failing, and the permutation for a given seed differs from earlier versions. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review result for exact head 61608979f6e81905b0fce7f173241fa0ea2eca71: requesting changes for one compatibility issue, detailed inline.
Critical checkpoints:
- Goal and correctness: the per-row seed bug is otherwise fixed. Seeded rows are isolated from preceding rows and block boundaries; constant and column seeds, nullable inputs, both aliases, negative seeds, and BIGINT extremes follow the intended local call chain.
- Scope and compatibility: replacing
mt19937withmt19937_64changes established outputs even for a first/single row and also changes the unrelated unseeded stream. Doris documents a supplied seed as producing reproducible results, but this PR provides no old implementation selected during rolling upgrades. This is the blocking point. - Concurrency and lifecycle: generators are stack-local to each execution call; no shared mutable state, lock ordering, static initialization, or special lifetime issue is introduced.
- Error, NULL, and parallel paths: default constant/NULL wrappers keep physical row and seed indexes aligned. The prior NULL-payload and empty/singleton concerns were treated as duplicate fences; every BIGINT is now accepted and the short-array path skips generator setup.
array_shuffleandshuffleshare the same BE implementation and FE BIGINT coercion. - Tests and results: the new BE test is discovered by the recursive UT glob and uses the public function wrapper. Regression coverage includes per-row and constant seeds, aliases, nullable combinations,
NULLIF, short arrays, block-versus-single-row behavior, and BIGINT limits; changed golden blocks map to their queries. No build or test was run in this review environment, per the review contract, so this is static evidence rather than independent execution. - Configuration, persistence, data writes, FE/BE protocol, observability: not applicable; the PR adds no config, persisted or serialized state, transaction/write path, transmitted variable, or operational signal.
- Performance: per-row state initialization is inherent in the chosen fix for nontrivial seeded rows, and the new
< 2guard removes the avoidable trivial-row cost. No additional distinct performance issue survived review. - User focus: no additional user-provided focus was supplied.
Review completeness: the authoritative eight-file diff received a main scan, two normal full-review passes per round, a separate risk-focused pass per round, candidate adjudication, duplicate fencing, and a final changed-file sweep. Round 2 converged with NO_NEW_VALUABLE_FINDINGS; M-001 below is the only accepted new issue.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 27989 ms |
Contributor
TPC-DS: Total hot run time: 152903 ms |
Contributor
ClickBench: Total hot run time: 23.96 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: None Related PR: apache#44901 Problem Summary: array_shuffle(arr[, seed]) builds one random generator per block, and every row of the block draws from it. The earlier commits on this branch re-seeded the generator for every row, which gave each row its own seed but costs about 1 us per row, because seeding std::mt19937_64 fills 312 state words. Other engines with a seeded array shuffle (ClickHouse arrayShuffle, Spark shuffle, Velox, the DataFusion Spark-compatible shuffle) do not take a per-row seed: the seed must be a constant, and each block draws from one random sequence. This commit switches to that model and fixes the related problems: 1. A non-constant seed was accepted but silently ignored, because the generator was seeded from the seed of the first row of the block. The FE now rejects it in ArrayShuffle.checkLegalityBeforeTypeCoercion ("The seed of array_shuffle must be a constant"), and the BE rejects it in open(), so a BE that gets such a plan from an older FE during a rolling upgrade fails instead of ignoring the seeds. 2. Since apache#44901, a negative seed or a seed larger than 4294967295 failed with an out-of-range INTERNAL_ERROR from cast_set<uint32_t>. The seed now keeps its low 32 bits with static_cast<uint32_t>, which is the 3.x behavior. Results for seeds in [0, 4294967295] do not change. 3. A constant array got the same order on every row, because the BE computed the call once for all-constant arguments, and with enable_fold_constant_by_be = true the FE folded the call into one literal. The BE now returns false from use_default_implementation_for_constants(), and ArrayShuffle extends UniqueFunction like random() and uuid(), so it is never folded into a constant or merged with another call. Before: 100 rows of a constant array gave 1 order; after: 100 orders. 4. Without a seed, each block was seeded with time(nullptr), so blocks processed in the same second repeated the same orders: 100000 rows of a 20-element array gave only 26560 different orders. Each block is now seeded from std::random_device, like rand(), and the same query gives 100000 orders. A single call with a constant seed, such as array_shuffle([1, 2, 3, 4], 0), still gives the same result as before. ### Release note array_shuffle / shuffle: a non-constant seed now reports an error instead of being silently ignored; a negative seed or a seed larger than 4294967295 is accepted again, and only its low 32 bits are used; a constant array is shuffled on each row instead of getting the same order on every row; without a seed, rows processed in the same second no longer repeat the same orders. ### Check List (For Author) - Test: Unit Test / Regression test - BE unit test: function_array_shuffle_test, 6 cases passed - FE unit test: UniqueFunctionTest, 18 cases passed - Regression test: test_array_shuffle_seed passed; test_array_functions, test_size_function and test_array_function_doc passed and their .out files are the same as on master - Behavior changed: Yes. A non-constant seed reports an error, a negative or large seed no longer reports an error, and a constant array gets a different order on each row. - Does this need documentation: Yes, an apache/doris-website PR will follow Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
csun5285
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
September 28, 2026 05:57
### What problem does this PR solve? Issue Number: None Related PR: apache#44901 Problem Summary: array_shuffle kept only the low 32 bits of its BIGINT seed for std::mt19937, so seeds with the same low 32 bits, such as -1 and 4294967295, gave the same order. ClickHouse arrayShuffle gives the full 64-bit seed to pcg64_fast. This commit switches the generator to std::mt19937_64 and uses all 64 bits of the seed, so -1 and 4294967295 now give different orders. Without a seed, each block is seeded with 64 bits from std::random_device. Because the generator changed, the order for a given seed is different from before, so the seeded results of test_size_function, test_array_function_doc and test_array_shuffle_seed are regenerated. Unit tests that expect two orders to differ now use a 20-element array, because a 5-element array has only 120 orders and two seeds can give the same one by chance. This commit also fixes the comments that said all rows share one random sequence. Each block starts its own sequence from the seed, so a per-row seed would be ignored, which is why a non-constant seed is rejected. ### Release note array_shuffle / shuffle uses all 64 bits of the seed, so the order for a given seed differs from earlier versions, and seeds with the same low 32 bits no longer give the same order. ### Check List (For Author) - Test: Unit Test / Regression test - BE unit test: function_array_shuffle_test, 6 cases passed - Regression test: test_array_shuffle_seed, test_array_functions, test_size_function and test_array_function_doc passed; test_array_functions keeps its .out file - Behavior changed: Yes. The order for a given seed changes, and seeds with the same low 32 bits give different orders. - Does this need documentation: Yes, an apache/doris-website PR will follow Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
### What problem does this PR solve? Issue Number: None Related PR: apache#44901 Problem Summary: An earlier commit on this branch read the constant seed with unpack_if_const, which takes the one-value column inside the ColumnConst instead of expanding it to a full column. Both give the same seed, and expanding the constant only copies one BIGINT per row of the block, which is small next to shuffling the arrays. Go back to convert_to_full_column_if_const, as the original code does, so the change to this function stays small, and drop the two includes that were only added for unpack_if_const. ### Release note None ### Check List (For Author) - Test: Unit Test - BE unit test: function_array_shuffle_test, 6 cases passed - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
csun5285
changed the title
2 of 10 tasks
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: {"type":"error","status":400,"error":{"type":"invalid_request_error","message":"The 'gpt-6-sol' model is not supported when using Codex with a ChatGPT account."}} Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Request changes: one P1 data-correctness issue is inline. The new per-row behavior for a constant-array shuffle can make external file scans run the same volatile predicate twice and discard rows. The existing threads already cover rolling-upgrade output, NULL seeds, and short arrays; this review does not repeat them. The user supplied no additional focus points.
Critical checkpoints (static review):
- Goal and proof: FE and BE now reject nonconstant seeds, use the full BIGINT seed, and vary constant-array and unseeded results by row/block as intended. The added BE, FE, and SQL tests cover these direct behaviors. They do not cover a shuffle predicate on an external file scan, where the accepted issue occurs.
- Scope and parallel paths: The implementation is focused on the function and its FE identity.
shuffleshares the BE registration and FE binding. BE's separate determinism check used by file-scan localization was not updated for either name. - Concurrency and lifecycle: The generator is local to execution; no new shared state or lock order appears. The
FunctionContextconstant check runs at open, and the generator has block lifetime. No new long-lived ownership or static initialization issue was found. - Configuration and compatibility: No configuration or FE/BE protocol variable was added. Seeded output deliberately changes; the rolling-upgrade concern is already in an inline thread.
- Conditions and tests: FE constantness and BE constant-column validation align for the reviewed literal, expression, and slot cases. The nullable and zero-row wrappers avoid a separate established failure. Expected-error tests use the suite pattern; changed goldens correspond to the source queries by static inspection. Exact permutations and test execution were not independently verified.
- Observability, persistence, and writes: No new persistent state, transaction, data-write path, or distributed operation is involved. Existing error status is adequate for the seed rejection; no new log or metric requirement was established.
- Performance and remaining issues: The new random-device call is per block; no separate material regression was substantiated. The short-array cost is already covered by an existing thread. The file-scan double-evaluation issue is the sole new blocking finding.
This was a static review only. The review environment prohibits builds and tests, so no runtime validation is claimed.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 27796 ms |
Contributor
TPC-DS: Total hot run time: 152414 ms |
Contributor
ClickBench: Total hot run time: 23.92 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Oct 9, 2026
yiguolei
pushed a commit
to apache/doris-website
that referenced
this pull request
Oct 9, 2026
Update ARRAY_SHUFFLE docs, for apache/doris#68438: - seed must be a constant; a column or another non-constant seed reports an error - all 64 bits of the seed are used, so any BIGINT works, a negative one too - each row is shuffled on its own, a constant array too, and each batch starts a random sequence from the seed ## Versions - [x] dev - [x] 4.x - [ ] 3.x - [ ] 2.1 or older (not covered by version/language sync gate) ## Languages - [ ] Chinese - [ ] English ## Docs Checklist - [ ] Checked by AI - [ ] Test Cases Built - [ ] Updated required version and language counterparts, or explained why not - [ ] If only one language changed, confirmed whether source/translation counterparts need sync Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
924060929
approved these changes
Oct 9, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: #44901 Problem Summary: 1. `array_shuffle` now rejects a non-constant seed, which was silently ignored for every row but the first one of each block. 2. The seed now uses all 64 bits (`std::mt19937_64`), so any BIGINT seed works, including a negative one, which failed with an out-of-range error since #44901. 3. Every row is now shuffled on its own, so a constant array no longer gets the same order on every row, and blocks without a seed no longer repeat each other. doc: apache/doris-website#4176 ### Release note `array_shuffle` / `shuffle` rejects a non-constant seed, uses all 64 bits of the seed (so the order for a given seed changes), and shuffles every row on its own. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [x] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [x] Yes. See the three points above. - Does this need documentation? - [ ] No. - [x] Yes. <!-- apache/doris-website PR link to be added --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
4 of 9 tasks
yiguolei
pushed a commit
that referenced
this pull request
Oct 10, 2026
#68438 (#68832) Cherry-picked from #68438 Co-authored-by: Chenyang Sun <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
yiguolei
pushed a commit
that referenced
this pull request
Oct 10, 2026
#68438 (#68832) Cherry-picked from #68438 Co-authored-by: Chenyang Sun <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
yiguolei
pushed a commit
that referenced
this pull request
Oct 10, 2026
…#68672) ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Condition cache can incorrectly reuse false ranges for expressions whose results vary across executions, and for virtual-column predicates whose definitions are omitted from the digest. For example, running `abs(k + 1) + abs(k + 1) = 0` before `abs(k - 9999) + abs(k - 9999) = 0` can incorrectly omit `k = 9999`. Share expression eligibility with query cache, inspect scan predicates, virtual columns and runtime-filter probes (including implicit casts), and pass scan eligibility to BE before computing the cache digest. Recognize scalar and ARRAY/MAP/STRUCT TIME-to-date conversions while preserving supported NULL folding. VirtualSlotRef returns the zero-digest sentinel to disable reuse when its definition is absent from the inherited digest. Add per-query ConditionCacheLookup counters for hits and misses. Regression coverage identifies individual execution profiles, checks that unsafe scans perform zero lookups, and retains positive checks for deterministic cache hits. Rebased onto master `a53b182f810be2b3731c7dfdf6f40ff5a0ad63bd`. Preserve master's #68438 shuffle identity and constant-seed validation while allowing NULL-only folding, and retain master's grouped non-blocking runtime-filter test alongside cache-eligibility coverage. All five original commits are retained. ### Release note Prevent incorrect cached results for volatile predicates, query-date-dependent TIME casts, and virtual-column predicates. Preserve supported NULL folding for TIME casts and shuffle calls. Expose ConditionCacheLookup in OLAP scan profiles. Rolling upgrade: upgrade BE before FE. The optional plan flag defaults false, so a new BE disables condition cache for old FE plans; an old BE cannot enforce the new eligibility flag. ### Check List (For Author) - Test: - [x] Unit Test: 39 targeted FE tests passed after rebase using `FE_UT_PARALLEL=48 bash run-fe-ut.sh --run 'org.apache.doris.nereids.util.CacheExpressionTest,org.apache.doris.planner.ConditionCacheEligibilityTest,org.apache.doris.nereids.glue.translator.RuntimeFilterTranslatorBucketPruneTest,org.apache.doris.nereids.trees.expressions.functions.scalar.UniqueFunctionTest'` (8 + 6 + 7 + 18). - [x] Manual test: FE Checkstyle passed with zero violations; repository check-format.sh with clang-format 16.0.6 and BE build hygiene passed. Original patch comparison confirms identical added/removed lines for 23 files; ArrayShuffle adapts to the equivalent functionality now in master. - [ ] Regression test: BE tests and regression suites were not rerun locally for this rebase; existing coverage is retained and run buildall will verify integration. - Behavior changed: - [x] Yes. Unsafe scans bypass condition cache; deterministic scans retain eligibility. No additional behavior change introduced by rebase. - Does this need documentation? - [x] No. Correctness fix using existing cache switches; upgrade considerations are stated above. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
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
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.
What problem does this PR solve?
Issue Number: None
Related PR: #44901
Problem Summary:
array_shufflenow rejects a non-constant seed, which was silently ignored for every row but the first one of each block.std::mt19937_64), so any BIGINT seed works, including a negative one, which failed with an out-of-range error since [fix](compile) Fix MacOS compile failed #44901.doc: apache/doris-website#4176
Release note
array_shuffle/shufflerejects a non-constant seed, uses all 64 bits of the seed (so the order for a given seed changes), and shuffles every row on its own.Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)