Skip to content

[fix](compile) Fix MacOS compile failed - #44901

Merged
zy-kkk merged 1 commit into
apache:masterfrom
xinyiZzz:20241203_fix_compile
Dec 4, 2024
Merged

zy-kkk merged 1 commit into
apache:masterfrom
xinyiZzz:20241203_fix_compile

Conversation

@xinyiZzz

@xinyiZzz xinyiZzz commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

img_v3_02h7_cf83e8be-0905-4f68-83e1-ba77afd342ag

Check List (For Author)

  • Test

    • Regression test
    • 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
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@doris-robot

Copy link
Copy Markdown

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@xinyiZzz

xinyiZzz commented Dec 3, 2024

Copy link
Copy Markdown
Contributor Author

run buildall

@xinyiZzz
xinyiZzz force-pushed the 20241203_fix_compile branch from b5d6193 to 66d8bfc Compare December 3, 2024 07:26
@xinyiZzz

xinyiZzz commented Dec 3, 2024

Copy link
Copy Markdown
Contributor Author

run buildall

@xinyiZzz xinyiZzz changed the title [fix](compile) Fix MacOS compile failed column_decimal.h:262 [fix](compile) Fix MacOS compile failed Dec 3, 2024
@doris-robot

Copy link
Copy Markdown
TPC-H: Total hot run time: 40653 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 66d8bfcc89490198c1f53e950a2d45b6eafb0fbc, data reload: false

------ Round 1 ----------------------------------
q1	17598	7548	7377	7377
q2	2049	186	172	172
q3	10541	1156	1166	1156
q4	10227	755	800	755
q5	7623	2791	2738	2738
q6	232	149	150	149
q7	1009	638	607	607
q8	9266	1836	1954	1836
q9	6716	6534	6532	6532
q10	6980	2372	2358	2358
q11	468	251	257	251
q12	421	218	216	216
q13	17779	3063	3079	3063
q14	249	232	216	216
q15	565	542	535	535
q16	641	608	597	597
q17	1011	556	540	540
q18	7417	6779	6849	6779
q19	1338	972	986	972
q20	465	182	178	178
q21	4044	3320	3390	3320
q22	376	306	309	306
Total cold run time: 107015 ms
Total hot run time: 40653 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7353	7215	7203	7203
q2	328	226	226	226
q3	2934	2785	2782	2782
q4	1975	1720	1735	1720
q5	5383	5428	5399	5399
q6	220	139	138	138
q7	2160	1741	1742	1741
q8	3265	3418	3399	3399
q9	8756	8749	8701	8701
q10	3499	3471	3460	3460
q11	581	496	497	496
q12	790	593	568	568
q13	11011	3051	3066	3051
q14	299	282	262	262
q15	568	524	502	502
q16	663	631	643	631
q17	1817	1557	1595	1557
q18	7894	7367	7521	7367
q19	1648	1603	1619	1603
q20	2096	1843	1780	1780
q21	5638	5367	5360	5360
q22	646	572	577	572
Total cold run time: 69524 ms
Total hot run time: 58518 ms

@doris-robot

Copy link
Copy Markdown
TPC-DS: Total hot run time: 190896 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 66d8bfcc89490198c1f53e950a2d45b6eafb0fbc, data reload: false

query1	964	382	369	369
query2	6513	2126	2037	2037
query3	6742	214	214	214
query4	34058	23734	23544	23544
query5	4300	477	460	460
query6	284	190	177	177
query7	4620	306	309	306
query8	295	238	239	238
query9	9758	2699	2722	2699
query10	482	254	256	254
query11	18101	15299	15396	15299
query12	160	107	101	101
query13	1652	405	387	387
query14	10268	6810	6599	6599
query15	258	176	179	176
query16	8081	475	469	469
query17	1632	563	537	537
query18	2129	287	284	284
query19	359	149	139	139
query20	114	110	112	110
query21	206	109	136	109
query22	4447	4279	4238	4238
query23	34940	34039	34191	34039
query24	10147	2585	2437	2437
query25	587	373	376	373
query26	713	163	151	151
query27	2088	280	280	280
query28	6739	2442	2427	2427
query29	753	418	402	402
query30	297	153	153	153
query31	1025	770	827	770
query32	99	55	55	55
query33	663	292	315	292
query34	910	533	514	514
query35	900	761	762	761
query36	1117	962	966	962
query37	124	79	73	73
query38	4465	4265	4240	4240
query39	1460	1460	1437	1437
query40	207	104	105	104
query41	49	48	47	47
query42	120	103	104	103
query43	536	481	502	481
query44	1171	807	800	800
query45	196	168	167	167
query46	1142	721	709	709
query47	1970	1865	1902	1865
query48	426	320	321	320
query49	1122	409	420	409
query50	803	384	389	384
query51	7394	7226	7130	7130
query52	101	90	94	90
query53	255	178	182	178
query54	1150	397	409	397
query55	77	76	79	76
query56	254	238	250	238
query57	1259	1141	1132	1132
query58	225	199	213	199
query59	3243	3031	2925	2925
query60	270	244	292	244
query61	109	108	109	108
query62	860	658	688	658
query63	216	186	184	184
query64	3775	665	633	633
query65	3270	3216	3288	3216
query66	890	311	307	307
query67	16220	15758	15583	15583
query68	4545	559	563	559
query69	455	249	249	249
query70	1238	1155	1156	1155
query71	344	246	246	246
query72	6442	4174	4104	4104
query73	766	356	369	356
query74	10380	9009	9041	9009
query75	3436	2690	2660	2660
query76	2986	1070	1094	1070
query77	438	300	306	300
query78	10334	9563	9377	9377
query79	1487	596	601	596
query80	1216	425	446	425
query81	624	243	238	238
query82	948	125	115	115
query83	244	153	147	147
query84	237	68	81	68
query85	1106	298	292	292
query86	397	270	284	270
query87	4698	4519	4676	4519
query88	3580	2220	2182	2182
query89	411	305	294	294
query90	1886	187	182	182
query91	137	105	103	103
query92	61	51	52	51
query93	1163	532	538	532
query94	859	305	280	280
query95	347	243	255	243
query96	615	285	284	284
query97	2878	2696	2660	2660
query98	234	194	199	194
query99	1567	1377	1337	1337
Total cold run time: 295819 ms
Total hot run time: 190896 ms

@doris-robot

Copy link
Copy Markdown
ClickBench: Total hot run time: 32.82 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 66d8bfcc89490198c1f53e950a2d45b6eafb0fbc, data reload: false

query1	0.04	0.03	0.03
query2	0.06	0.06	0.04
query3	0.23	0.07	0.07
query4	1.62	0.10	0.10
query5	0.42	0.40	0.42
query6	1.14	0.66	0.66
query7	0.02	0.01	0.02
query8	0.04	0.03	0.03
query9	0.56	0.51	0.50
query10	0.57	0.56	0.56
query11	0.13	0.11	0.11
query12	0.14	0.11	0.12
query13	0.63	0.61	0.61
query14	2.83	2.86	2.74
query15	0.92	0.83	0.82
query16	0.38	0.39	0.38
query17	1.07	1.03	1.04
query18	0.23	0.21	0.22
query19	1.91	1.87	2.01
query20	0.01	0.02	0.01
query21	15.38	0.62	0.58
query22	3.93	2.33	1.24
query23	16.89	1.18	0.87
query24	3.33	1.47	1.61
query25	0.32	0.23	0.08
query26	0.42	0.14	0.14
query27	0.04	0.04	0.04
query28	9.53	1.11	1.07
query29	12.58	3.23	3.25
query30	0.25	0.06	0.07
query31	2.88	0.40	0.38
query32	3.26	0.49	0.48
query33	3.05	3.05	3.11
query34	16.99	4.52	4.58
query35	4.56	4.54	4.54
query36	0.68	0.51	0.49
query37	0.09	0.06	0.07
query38	0.04	0.04	0.03
query39	0.03	0.02	0.03
query40	0.16	0.12	0.13
query41	0.07	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 107.51 s
Total hot run time: 32.82 s

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 38.51% (10008/25988)
Line Coverage: 29.50% (83794/284031)
Region Coverage: 28.63% (43102/150552)
Branch Coverage: 25.23% (21906/86814)
Coverage Report: http://coverage.selectdb-in.cc/coverage/66d8bfcc89490198c1f53e950a2d45b6eafb0fbc_66d8bfcc89490198c1f53e950a2d45b6eafb0fbc/report/index.html

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Dec 4, 2024
@github-actions

github-actions Bot commented Dec 4, 2024

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

github-actions Bot commented Dec 4, 2024

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@zy-kkk
zy-kkk merged commit 864ebb8 into apache:master Dec 4, 2024
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Apr 24, 2026
csun5285 added a commit to csun5285/doris that referenced this pull request Sep 24, 2026
### 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]>
csun5285 added a commit to csun5285/doris that referenced this pull request Sep 28, 2026
### 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 added a commit to csun5285/doris that referenced this pull request Sep 28, 2026
### 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]>
csun5285 added a commit to csun5285/doris that referenced this pull request Sep 28, 2026
### 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 added 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]>
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants