Skip to content

branch-3.1: [bugfix](catalog) replace Math.abs with bitwise AND to ensure non-negative ID generation #55183 #55689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
morrySnow merged 1 commit into from
Sep 5, 2025
Merged

branch-3.1: [bugfix](catalog) replace Math.abs with bitwise AND to ensure non-negative ID generation #55183 #55689

morrySnow merged 1 commit into from
Sep 5, 2025

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 4, 2025

Cherry-picked from #55183

@felixwluo
[bugfix](catalog) replace Math.abs with bitwise AND to ensure non-neg…
e088237 
…ative ID generation (#55183)

### What problem does this PR solve?

Problem Summary:
`Replace Math.abs() with bitwise AND operation in genIdByName() method`

problem:
- Math.abs(Long.MIN_VALUE) returns Long.MIN_VALUE (still negative) due
to overflow
- This violates the requirement that external db/table IDs must be >= 0
- Could cause issues in DescriptorTable.toThrift() as mentioned in
comments

fix:
- Use bitwise AND with Long.MAX_VALUE to clear the sign bit
- Guarantees non-negative result in range [0, Long.MAX_VALUE]
- Better performance than Math.abs() conditional check

### Release note

None

### Check List (For Author)

- Test <!-- At least one of them must be included. -->
    - [ ] 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 <!-- Add your reason?  -->

- Behavior changed:
    - [ ] No.
    - [ ] Yes. <!-- Explain the behavior change -->

- Does this need documentation?
    - [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->

### 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 -->
@github-actions github-actions bot requested a review from morrySnow as a code owner September 4, 2025 13:26
@hello-stephen
Copy link
Contributor

hello-stephen commented Sep 4, 2025

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?

@dataroaring dataroaring closed this Sep 4, 2025
@dataroaring dataroaring reopened this Sep 4, 2025
@hello-stephen
Copy link
Contributor

hello-stephen commented Sep 4, 2025

run buildall

@doris-robot
Copy link

doris-robot commented Sep 4, 2025

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

------ Round 1 ----------------------------------
q1	17582	5530	5364	5364
q2	2009	397	276	276
q3	12338	1261	762	762
q4	10563	880	452	452
q5	9631	2403	2147	2147
q6	183	163	134	134
q7	889	758	625	625
q8	9348	1451	1187	1187
q9	5317	4978	4949	4949
q10	6764	2249	1815	1815
q11	475	278	268	268
q12	337	354	212	212
q13	17799	3599	3018	3018
q14	228	224	213	213
q15	533	460	468	460
q16	430	432	385	385
q17	607	863	376	376
q18	7103	6569	6306	6306
q19	1204	951	546	546
q20	331	340	210	210
q21	3038	2242	2038	2038
q22	1076	1018	981	981
Total cold run time: 107785 ms
Total hot run time: 32724 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5557	5465	5573	5465
q2	235	330	231	231
q3	2260	2613	2301	2301
q4	1385	1836	1446	1446
q5	4400	5096	5031	5031
q6	162	159	128	128
q7	2125	1987	1792	1792
q8	2676	2859	2703	2703
q9	7377	7285	7210	7210
q10	3040	3207	2773	2773
q11	613	514	488	488
q12	688	756	594	594
q13	3394	3753	3172	3172
q14	278	301	271	271
q15	523	474	470	470
q16	445	495	454	454
q17	1218	1759	1303	1303
q18	7734	7420	7349	7349
q19	847	1091	1136	1091
q20	2000	2044	1931	1931
q21	5333	5009	4469	4469
q22	1089	1107	1042	1042
Total cold run time: 53379 ms
Total hot run time: 51714 ms

@doris-robot
Copy link

doris-robot commented Sep 4, 2025

TPC-DS: Total hot run time: 192734 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 e0882374450635aa23a537912871c64fdff8c578, data reload: false

query1	952	397	395	395
query2	6213	1916	1873	1873
query3	8710	214	202	202
query4	33226	23496	23536	23496
query5	3729	613	483	483
query6	289	204	183	183
query7	4212	486	315	315
query8	305	239	229	229
query9	9261	2609	2605	2605
query10	476	327	263	263
query11	18312	15369	15152	15152
query12	163	107	110	107
query13	1556	546	417	417
query14	8854	6661	6734	6661
query15	208	202	180	180
query16	7853	651	499	499
query17	1603	761	602	602
query18	2058	429	333	333
query19	212	192	178	178
query20	129	122	116	116
query21	211	136	112	112
query22	4536	4761	4467	4467
query23	34972	34771	33956	33956
query24	8524	2741	2765	2741
query25	564	496	434	434
query26	1214	286	179	179
query27	3127	495	370	370
query28	5535	2251	2220	2220
query29	816	592	476	476
query30	254	190	170	170
query31	985	918	877	877
query32	98	62	62	62
query33	520	381	338	338
query34	734	866	543	543
query35	791	867	730	730
query36	1013	1072	950	950
query37	103	87	71	71
query38	4096	4030	3988	3988
query39	1530	1469	1473	1469
query40	205	117	98	98
query41	49	50	45	45
query42	125	114	113	113
query43	524	527	492	492
query44	1399	840	852	840
query45	185	180	176	176
query46	916	1058	688	688
query47	1997	2037	1971	1971
query48	425	432	346	346
query49	766	482	427	427
query50	666	689	474	474
query51	7494	7418	7280	7280
query52	99	105	93	93
query53	227	261	196	196
query54	565	563	495	495
query55	82	78	81	78
query56	268	283	265	265
query57	1269	1319	1212	1212
query58	237	221	235	221
query59	3076	3122	3129	3122
query60	294	292	275	275
query61	126	112	131	112
query62	825	773	693	693
query63	237	206	210	206
query64	4446	1027	647	647
query65	3399	3321	3320	3320
query66	1009	429	324	324
query67	16664	15763	15702	15702
query68	7851	841	559	559
query69	482	306	269	269
query70	1205	1125	1129	1125
query71	418	284	270	270
query72	5135	3615	4020	3615
query73	640	749	355	355
query74	10487	9394	9129	9129
query75	3684	3116	2653	2653
query76	3394	1180	780	780
query77	768	399	276	276
query78	10380	10394	9671	9671
query79	5741	843	596	596
query80	676	511	438	438
query81	486	253	225	225
query82	336	121	90	90
query83	174	165	151	151
query84	291	103	83	83
query85	759	359	319	319
query86	354	317	291	291
query87	4336	4370	4288	4288
query88	3950	2447	2419	2419
query89	436	333	297	297
query90	2042	193	197	193
query91	140	141	116	116
query92	67	57	54	54
query93	3560	888	558	558
query94	658	424	304	304
query95	360	281	273	273
query96	499	607	287	287
query97	3197	3258	3156	3156
query98	223	217	215	215
query99	1529	1391	1326	1326
Total cold run time: 299776 ms
Total hot run time: 192734 ms

@doris-robot
Copy link

doris-robot commented Sep 4, 2025

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.24	0.06	0.07
query4	1.62	0.11	0.10
query5	0.53	0.51	0.53
query6	1.14	0.73	0.73
query7	0.02	0.02	0.02
query8	0.04	0.03	0.03
query9	0.60	0.49	0.51
query10	0.56	0.55	0.55
query11	0.15	0.11	0.10
query12	0.14	0.10	0.10
query13	0.62	0.59	0.60
query14	0.79	0.79	0.79
query15	0.85	0.82	0.83
query16	0.38	0.39	0.38
query17	1.04	1.07	1.06
query18	0.23	0.22	0.21
query19	1.91	1.81	1.90
query20	0.02	0.01	0.02
query21	15.43	0.93	0.58
query22	0.74	0.89	0.60
query23	15.09	1.37	0.56
query24	3.44	0.67	1.45
query25	0.21	0.23	0.05
query26	0.24	0.15	0.14
query27	0.04	0.05	0.04
query28	13.60	0.99	0.43
query29	12.60	3.93	3.25
query30	0.24	0.09	0.06
query31	2.82	0.58	0.37
query32	3.23	0.54	0.46
query33	2.99	3.01	3.02
query34	16.82	5.25	4.50
query35	4.55	4.56	4.61
query36	0.65	0.52	0.48
query37	0.09	0.05	0.06
query38	0.05	0.03	0.04
query39	0.04	0.02	0.03
query40	0.17	0.13	0.12
query41	0.08	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 104.18 s
Total hot run time: 28.47 s

@morrySnow morrySnow merged commit aba9d6a into branch-3.1 Sep 5, 2025
23 checks passed
@morrySnow morrySnow deleted the auto-pick-55183-branch-3.1 branch September 5, 2025 16:39
@morrySnow morrySnow mentioned this pull request Sep 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morrySnow morrySnow morrySnow approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@hello-stephen @doris-robot @morrySnow @dataroaring @felixwluo