Skip to content
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

[fix](variant) fix index in variant #43375

Merged
merged 3 commits into from
Nov 11, 2024
Merged

[fix](variant) fix index in variant #43375

merged 3 commits into from
Nov 11, 2024

Conversation

csun5285
Copy link
Contributor

@csun5285 csun5285 commented Nov 6, 2024 •
edited by airborne12
Loading

What problem does this PR solve?

Problem Summary:

  1. Fixing error checks, InvertedIndexColumnWriter::check_support_inverted_index, It is not appropriate to determine support for other indexes by checking the inverted index.
  2. Fix TableSchema::update_index()

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 colde files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

  • Release note

    None

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

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

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@csun5285
Copy link
Contributor Author

csun5285 commented Nov 6, 2024

run buildall

github-actions[bot]
github-actions bot reviewed Nov 6, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17583	7483	7313	7313
q2	2060	173	189	173
q3	10510	1112	1193	1112
q4	10517	873	811	811
q5	7781	3080	3074	3074
q6	245	149	150	149
q7	1007	619	611	611
q8	9377	2006	2053	2006
q9	6607	6452	6439	6439
q10	7081	2445	2405	2405
q11	472	258	260	258
q12	402	218	218	218
q13	17781	3005	3008	3005
q14	246	213	216	213
q15	568	519	510	510
q16	651	595	579	579
q17	983	531	551	531
q18	7379	6790	6707	6707
q19	1328	1002	994	994
q20	448	181	179	179
q21	3992	3041	3094	3041
q22	1112	1013	1011	1011
Total cold run time: 108130 ms
Total hot run time: 41339 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7297	7281	7303	7281
q2	359	256	251	251
q3	3019	2974	2950	2950
q4	2051	1919	1798	1798
q5	5793	5753	5781	5753
q6	230	149	150	149
q7	2221	1875	1790	1790
q8	3450	3600	3449	3449
q9	9034	8990	8869	8869
q10	3592	3572	3600	3572
q11	614	491	496	491
q12	830	624	650	624
q13	9869	3170	3194	3170
q14	323	265	291	265
q15	601	550	555	550
q16	702	634	644	634
q17	1890	1646	1599	1599
q18	8418	7937	7641	7641
q19	1708	1613	1573	1573
q20	2134	1873	1858	1858
q21	5714	5389	5407	5389
q22	1175	1037	1036	1036
Total cold run time: 71024 ms
Total hot run time: 60692 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.97% (9871/26000)
Line Coverage: 29.08% (82021/282018)
Region Coverage: 28.31% (42244/149245)
Branch Coverage: 24.90% (21447/86144)
Coverage Report: http://coverage.selectdb-in.cc/coverage/9ea6b19c4e8897af8be1111cd78ebc12c5442720_9ea6b19c4e8897af8be1111cd78ebc12c5442720/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 194742 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 9ea6b19c4e8897af8be1111cd78ebc12c5442720, data reload: false

query1	2352	2109	2089	2089
query2	6239	2135	2030	2030
query3	15183	11439	234	234
query4	33059	23615	23763	23615
query5	3496	455	428	428
query6	276	190	180	180
query7	3980	282	300	282
query8	290	230	239	230
query9	9457	2712	2678	2678
query10	461	258	271	258
query11	17887	15285	15286	15285
query12	158	102	101	101
query13	1601	413	418	413
query14	8911	7607	7145	7145
query15	252	182	186	182
query16	8121	489	434	434
query17	1632	592	571	571
query18	2522	631	647	631
query19	393	197	181	181
query20	121	113	108	108
query21	209	106	114	106
query22	4967	4217	4325	4217
query23	36033	33995	34097	33995
query24	11475	3342	3338	3338
query25	546	408	399	399
query26	821	173	174	173
query27	2576	280	282	280
query28	7839	2430	2415	2415
query29	662	427	426	426
query30	396	311	314	311
query31	1027	815	814	814
query32	89	53	57	53
query33	764	280	277	277
query34	931	497	503	497
query35	879	721	736	721
query36	1099	955	944	944
query37	116	70	74	70
query38	4393	4233	4301	4233
query39	1492	1471	1454	1454
query40	258	98	100	98
query41	47	47	45	45
query42	107	97	95	95
query43	544	513	491	491
query44	1236	819	811	811
query45	185	164	164	164
query46	1123	697	693	693
query47	1978	1875	1887	1875
query48	409	317	330	317
query49	939	394	376	376
query50	804	401	396	396
query51	7185	7155	7156	7155
query52	99	95	88	88
query53	262	182	175	175
query54	1012	424	386	386
query55	75	78	77	77
query56	248	241	222	222
query57	1272	1160	1136	1136
query58	231	196	205	196
query59	3339	3154	3116	3116
query60	261	239	251	239
query61	115	111	107	107
query62	866	671	645	645
query63	211	180	182	180
query64	4557	640	612	612
query65	3268	3211	3236	3211
query66	1040	296	310	296
query67	16026	15652	15722	15652
query68	4875	582	583	582
query69	435	252	245	245
query70	1154	1150	1145	1145
query71	341	246	266	246
query72	6025	4057	3973	3973
query73	767	355	355	355
query74	10408	9123	9027	9027
query75	3421	2698	2668	2668
query76	2673	1142	1118	1118
query77	386	275	270	270
query78	10279	9410	9347	9347
query79	1763	587	595	587
query80	1183	413	418	413
query81	561	239	243	239
query82	963	113	118	113
query83	245	159	157	157
query84	244	73	75	73
query85	1452	305	301	301
query86	439	302	305	302
query87	4909	4766	4662	4662
query88	3459	2181	2149	2149
query89	418	286	289	286
query90	2007	186	182	182
query91	135	111	106	106
query92	58	48	49	48
query93	2251	539	537	537
query94	959	298	282	282
query95	348	248	247	247
query96	619	294	287	287
query97	2922	2673	2715	2673
query98	214	206	190	190
query99	1526	1298	1280	1280
Total cold run time: 307841 ms
Total hot run time: 194742 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.06	0.03	0.03
query3	0.23	0.06	0.06
query4	1.69	0.10	0.10
query5	0.41	0.42	0.39
query6	1.14	0.66	0.67
query7	0.02	0.02	0.01
query8	0.03	0.03	0.03
query9	0.58	0.49	0.51
query10	0.55	0.56	0.56
query11	0.14	0.11	0.10
query12	0.13	0.11	0.10
query13	0.60	0.59	0.60
query14	2.82	2.88	2.74
query15	0.90	0.82	0.83
query16	0.40	0.39	0.39
query17	1.04	1.04	1.07
query18	0.20	0.19	0.20
query19	1.94	1.81	1.96
query20	0.01	0.01	0.02
query21	15.36	0.60	0.57
query22	2.94	2.66	1.70
query23	17.06	0.89	0.92
query24	3.44	1.60	1.13
query25	0.20	0.19	0.11
query26	0.44	0.14	0.14
query27	0.04	0.05	0.04
query28	10.05	1.08	1.08
query29	12.55	3.23	3.23
query30	0.24	0.06	0.05
query31	2.88	0.38	0.37
query32	3.29	0.46	0.45
query33	2.95	3.03	3.04
query34	17.05	4.42	4.41
query35	4.51	4.42	4.51
query36	0.66	0.48	0.48
query37	0.09	0.06	0.06
query38	0.04	0.03	0.03
query39	0.03	0.02	0.02
query40	0.16	0.12	0.12
query41	0.08	0.02	0.02
query42	0.03	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 107.04 s
Total hot run time: 32.55 s

@csun5285
Copy link
Contributor Author

csun5285 commented Nov 8, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.00% (9891/26027)
Line Coverage: 29.14% (82393/282767)
Region Coverage: 28.29% (42425/149951)
Branch Coverage: 24.84% (21491/86508)
Coverage Report: http://coverage.selectdb-in.cc/coverage/caa329deff31cd696fb807eb2743d76544e731e1_caa329deff31cd696fb807eb2743d76544e731e1/report/index.html

@csun5285
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.00% (9890/26028)
Line Coverage: 29.14% (82393/282777)
Region Coverage: 28.30% (42407/149858)
Branch Coverage: 24.86% (21491/86438)
Coverage Report: http://coverage.selectdb-in.cc/coverage/cd6025ec468f5785fb724ef41053db454f09b5d0_cd6025ec468f5785fb724ef41053db454f09b5d0/report/index.html

eldenmoon
eldenmoon approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@eldenmoon eldenmoon left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

airborne12
airborne12 approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@airborne12 airborne12 left a comment

Choose a reason for hiding this comment

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

LGTM

@airborne12 airborne12 merged commit 1043962 into apache:master Nov 11, 2024
28 of 33 checks passed
csun5285 added a commit to csun5285/doris that referenced this pull request Nov 12, 2024
@csun5285
[fix](variant) fix index in variant (apache#43375)
91fa0a6 
### What problem does this PR solve?
Problem Summary:

1. Fixing error checks,
`InvertedIndexColumnWriter::check_support_inverted_index`, It is not
appropriate to determine support for other indexes by checking the
inverted index.
2. Fix `TableSchema::update_index()`

Co-authored-by: csun5285 <[email protected]>
@csun5285 csun5285 mentioned this pull request Nov 12, 2024
Merged
airborne12 pushed a commit that referenced this pull request Nov 13, 2024
@csun5285
[fix](variant) fix index in variant (#43375) (#43773)
27e0051 
pick from master (#43375)
csun5285 added a commit to csun5285/doris that referenced this pull request Nov 14, 2024
@csun5285
[fix](variant) fix index in variant (apache#43375)
0a5c0de 
Problem Summary:

1. Fixing error checks,
`InvertedIndexColumnWriter::check_support_inverted_index`, It is not
appropriate to determine support for other indexes by checking the
inverted index.
2. Fix `TableSchema::update_index()`

Co-authored-by: csun5285 <[email protected]>
@csun5285 csun5285 mentioned this pull request Nov 14, 2024
Merged
yiguolei pushed a commit that referenced this pull request Nov 16, 2024
@csun5285
[fix](variant) fix index in variant (#43375) (#43971)
10009dc 
Problem Summary:

1. Fixing error checks,
`InvertedIndexColumnWriter::check_support_inverted_index`, It is not
appropriate to determine support for other indexes by checking the
inverted index.
2. Fix `TableSchema::update_index()`

pick from master #43375
@gavinchou gavinchou mentioned this pull request Nov 26, 2024
Open
@gavinchou gavinchou mentioned this pull request Feb 18, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@airborne12 airborne12 airborne12 approved these changes

@eldenmoon eldenmoon eldenmoon approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.8-merged dev/3.0.3-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@csun5285 @doris-robot @airborne12 @eldenmoon @yiguolei