Skip to content

[Feature][external catalog/lakesoul] support lakesoul catalog - #32164

Merged
morningman merged 37 commits into
apache:masterfrom
Ceng23333:external_catalog/lakesoul_catalog
May 27, 2024
Merged

[Feature][external catalog/lakesoul] support lakesoul catalog#32164
morningman merged 37 commits into
apache:masterfrom
Ceng23333:external_catalog/lakesoul_catalog

Conversation

@Ceng23333

Copy link
Copy Markdown
Contributor

Proposed changes

Issue Number: close#32163

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@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

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
-	-DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
-	-DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
-	-DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
-	-DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
-	-DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@github-actions

Copy link
Copy Markdown
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors
'shellcheck ' found no issues.
shfmt errors

'shfmt ' returned error 1 finding the following formatting issues:
----------
--- thirdparty/build-thirdparty.sh.orig
+++ thirdparty/build-thirdparty.sh
@@ -821,7 +821,7 @@
"${CMAKE_CMD}" -G "${GENERATOR}" -DBUILD_SHARED_LIBS=ON -DWITH_GLOG=ON -DCMAKE_INSTALL_PREFIX="${TP_INSTALL_DIR}" \
-DCMAKE_LIBRARY_PATH="${TP_INSTALL_DIR}/lib64" -DCMAKE_INCLUDE_PATH="${TP_INSTALL_DIR}/include" \
-DBUILD_BRPC_TOOLS=OFF \
- -DWITH_SNAPPY=ON \
+ -DWITH_SNAPPY=ON \
-DPROTOBUF_PROTOC_EXECUTABLE="${TP_INSTALL_DIR}/bin/protoc" ..
"${BUILD_SYSTEM}" -j "${PARALLEL}"
----------
You can reformat the above files to meet shfmt's requirements by typing:
shfmt -w filename

@Ceng23333
Ceng23333force-pushed the external_catalog/lakesoul_catalog branch from cee3d1c to 87609f1CompareMarch 18, 2024 07:18

@github-actionsgithub-actionsBot left a comment

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.

clang-tidy made some suggestions

return Status::OK();
}

Status LakeSoulJniReader::get_columns(std::unordered_map<std::string, TypeDescriptor>* name_to_type,

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.

warning: method 'get_columns' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status LakeSoulJniReader::get_columns(std::unordered_map<std::string, TypeDescriptor>* name_to_type,
staticStatus LakeSoulJniReader::get_columns(std::unordered_map<std::string, TypeDescriptor>* name_to_type,

return Status::OK();
}

Status LakeSoulJniReader::init_reader(

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.

warning: method 'init_reader' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status LakeSoulJniReader::init_reader(
staticStatus LakeSoulJniReader::init_reader(


#pragma once

#include <stddef.h>

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.

warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead [modernize-deprecated-headers]

Suggested change
#include<stddef.h>
#include<cstddef>

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Ceng23333
Ceng23333force-pushed the external_catalog/lakesoul_catalog branch from 15fcd00 to e6adb8fCompareMarch 18, 2024 07:46
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningmanmorningman self-assigned this Mar 18, 2024
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actionsgithub-actionsBot left a comment

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.

clang-tidy made some suggestions

return Status::OK();
}

Status LakeSoulJniReader::get_columns(

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.

warning: method 'get_columns' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status LakeSoulJniReader::get_columns(
staticStatus LakeSoulJniReader::get_columns(

@morningman
morningmanforce-pushed the external_catalog/lakesoul_catalog branch from 0d63ca7 to 9920620CompareMay 7, 2024 12:57
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningman

Copy link
Copy Markdown
Contributor

run buildall

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 35.69% (8983/25173)
Line Coverage: 27.35% (74211/271372)
Region Coverage: 26.59% (38362/144248)
Branch Coverage: 23.40% (19567/83604)
Coverage Report: http://coverage.selectdb-in.cc/coverage/992062071f23a9da166d52ced07aee5a14f60190_992062071f23a9da166d52ced07aee5a14f60190/report/index.html

@morningman
morningmanforce-pushed the external_catalog/lakesoul_catalog branch from 9920620 to 025ccf0CompareMay 12, 2024 15:28
@morningman

Copy link
Copy Markdown
Contributor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 35.65% (8986/25206)
Line Coverage: 27.32% (74275/271909)
Region Coverage: 26.55% (38392/144592)
Branch Coverage: 23.37% (19576/83776)
Coverage Report: http://coverage.selectdb-in.cc/coverage/49a3a9082cb6ae5bf5d066cf945ee3ebb0f5f575_49a3a9082cb6ae5bf5d066cf945ee3ebb0f5f575/report/index.html

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

2 similar comments
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningman

Copy link
Copy Markdown
Contributor

run buildall

Ceng23333and others added 9 commits May 27, 2024 15:12
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: zenghua <441651826@qq.com>
Signed-off-by: dmetasoul01 <opensource@dmetasoul.com>
Signed-off-by: dmetasoul01 <opensource@dmetasoul.com>
@xuchen-plus
xuchen-plusforce-pushed the external_catalog/lakesoul_catalog branch from 53917d2 to 375989fCompareMay 27, 2024 07:54
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningman

Copy link
Copy Markdown
Contributor

run buildall

@doris-robot

Copy link
Copy Markdown
TPC-H: Total hot run time: 40056 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 375989fccd3901f04342f54c7f1c4e6a6572fc65, data reload: false
------ Round 1 ----------------------------------
q1	17616	4580	4259	4259
q2	2036	198	196	196
q3	10638	1317	1179	1179
q4	10229	851	717	717
q5	7658	2680	2737	2680
q6	221	134	133	133
q7	996	604	600	600
q8	9308	2132	2163	2132
q9	9285	6657	6627	6627
q10	10022	3689	3757	3689
q11	467	240	237	237
q12	529	226	209	209
q13	17773	2984	2996	2984
q14	277	213	224	213
q15	517	479	475	475
q16	540	399	384	384
q17	991	712	706	706
q18	8189	7434	7596	7434
q19	6542	1570	1510	1510
q20	661	299	300	299
q21	5021	3120	3172	3120
q22	333	276	273	273
Total cold run time: 119849 ms
Total hot run time: 40056 ms
----- Round 2, with runtime_filter_mode=off -----
q1	4354	4218	4230	4218
q2	375	276	267	267
q3	3030	2780	2672	2672
q4	1856	1617	1594	1594
q5	5267	5279	5326	5279
q6	215	124	124	124
q7	2118	1714	1756	1714
q8	3175	3326	3338	3326
q9	8384	8391	8294	8294
q10	3855	3644	3679	3644
q11	574	501	500	500
q12	754	568	598	568
q13	17480	2959	2990	2959
q14	293	265	261	261
q15	519	470	462	462
q16	474	423	423	423
q17	1808	1488	1476	1476
q18	7607	7591	7477	7477
q19	1679	1572	1522	1522
q20	1977	1786	1774	1774
q21	4865	4675	4748	4675
q22	549	472	469	469
Total cold run time: 71208 ms
Total hot run time: 53698 ms

@doris-robot

Copy link
Copy Markdown
TPC-DS: Total hot run time: 169290 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 375989fccd3901f04342f54c7f1c4e6a6572fc65, data reload: false
query1	924	414	373	373
query2	6593	2394	2244	2244
query3	6648	203	201	201
query4	19739	17475	17346	17346
query5	4128	419	418	418
query6	253	160	155	155
query7	4584	317	301	301
query8	240	180	191	180
query9	8477	2384	2346	2346
query10	443	286	257	257
query11	10532	10070	10015	10015
query12	139	88	86	86
query13	1631	364	365	364
query14	10388	7100	6730	6730
query15	229	175	169	169
query16	7830	267	265	265
query17	1830	542	530	530
query18	1958	280	285	280
query19	205	157	161	157
query20	100	88	87	87
query21	199	132	136	132
query22	4182	3984	3899	3899
query23	33705	32981	33091	32981
query24	12135	2884	2817	2817
query25	693	373	382	373
query26	1815	156	157	156
query27	2991	327	326	326
query28	7513	2030	2033	2030
query29	1166	634	621	621
query30	297	151	159	151
query31	1006	761	749	749
query32	91	55	59	55
query33	790	288	276	276
query34	1043	486	488	486
query35	753	619	600	600
query36	1090	930	913	913
query37	278	67	71	67
query38	2968	2802	2800	2800
query39	901	793	803	793
query40	281	128	126	126
query41	49	45	47	45
query42	108	99	99	99
query43	570	574	552	552
query44	1235	745	759	745
query45	176	165	163	163
query46	1083	713	714	713
query47	1864	1761	1778	1761
query48	385	308	300	300
query49	1211	396	399	396
query50	777	399	398	398
query51	6804	6710	6731	6710
query52	109	93	90	90
query53	370	287	289	287
query54	1018	438	428	428
query55	73	76	73	73
query56	274	262	262	262
query57	1126	1041	1066	1041
query58	238	215	235	215
query59	3325	3312	3028	3028
query60	283	257	273	257
query61	114	86	85	85
query62	669	474	442	442
query63	310	292	285	285
query64	9829	2202	1760	1760
query65	3161	3115	3126	3115
query66	1408	329	323	323
query67	15352	14871	15087	14871
query68	9597	537	534	534
query69	557	291	273	273
query70	1341	1134	1148	1134
query71	515	273	295	273
query72	8552	4874	2605	2605
query73	2109	323	324	323
query74	6069	5620	5636	5620
query75	4475	2629	2641	2629
query76	5639	1066	958	958
query77	650	268	270	268
query78	10411	9797	9796	9796
query79	7097	505	544	505
query80	1148	442	428	428
query81	492	222	222	222
query82	215	93	97	93
query83	205	176	169	169
query84	274	85	90	85
query85	981	266	264	264
query86	337	325	301	301
query87	3308	3104	3093	3093
query88	4818	2440	2433	2433
query89	510	383	386	383
query90	2150	186	181	181
query91	124	99	95	95
query92	57	49	49	49
query93	5147	499	486	486
query94	1389	187	186	186
query95	409	312	311	311
query96	606	270	267	267
query97	3143	2964	3003	2964
query98	239	230	215	215
query99	1160	848	860	848
Total cold run time: 296413 ms
Total hot run time: 169290 ms

@doris-robot

Copy link
Copy Markdown
ClickBench: Total hot run time: 31.39 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 375989fccd3901f04342f54c7f1c4e6a6572fc65, data reload: false
query1	0.04	0.03	0.03
query2	0.08	0.04	0.04
query3	0.23	0.05	0.06
query4	1.68	0.07	0.07
query5	0.51	0.48	0.49
query6	1.12	0.73	0.73
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.55	0.50	0.49
query10	0.55	0.55	0.53
query11	0.15	0.12	0.11
query12	0.14	0.12	0.11
query13	0.60	0.58	0.59
query14	0.76	0.77	0.78
query15	0.82	0.81	0.80
query16	0.36	0.34	0.36
query17	0.94	1.02	1.03
query18	0.21	0.27	0.22
query19	1.77	1.67	1.69
query20	0.02	0.01	0.01
query21	15.51	0.68	0.65
query22	4.15	6.09	2.84
query23	18.33	1.41	1.23
query24	1.64	0.31	0.21
query25	0.15	0.08	0.08
query26	0.27	0.17	0.17
query27	0.08	0.08	0.07
query28	13.38	1.02	1.00
query29	12.76	3.34	3.33
query30	0.24	0.06	0.05
query31	2.88	0.38	0.38
query32	3.29	0.47	0.46
query33	2.87	2.88	2.93
query34	16.92	4.43	4.43
query35	4.47	4.47	4.53
query36	0.66	0.45	0.45
query37	0.18	0.15	0.16
query38	0.15	0.14	0.14
query39	0.04	0.04	0.04
query40	0.17	0.14	0.14
query41	0.10	0.05	0.05
query42	0.06	0.04	0.05
query43	0.03	0.04	0.03
Total cold run time: 108.93 s
Total hot run time: 31.39 s

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 35.76% (9009/25192)
Line Coverage: 27.39% (74567/272249)
Region Coverage: 26.60% (38566/144958)
Branch Coverage: 23.47% (19667/83802)
Coverage Report: http://coverage.selectdb-in.cc/coverage/375989fccd3901f04342f54c7f1c4e6a6572fc65_375989fccd3901f04342f54c7f1c4e6a6572fc65/report/index.html

@morningmanmorningman left a comment

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.

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label May 27, 2024
@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@kaka11chenkaka11chen left a comment

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.

LGTM

@morningmanmorningman added the kind/feature Categorizes issue or PR as related to a new feature. label May 27, 2024
@morningman
morningman merged commit 9b5a464 into apache:masterMay 27, 2024
dataroaring pushed a commit that referenced this pull request May 28, 2024
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Apr 24, 2026
morningman added a commit to morningman/doris that referenced this pull request Sep 3, 2026
`.gitignore` has had a bare `.github` entry under its "# other" section since
9b5a464 ("[Feature][external catalog/lakesoul] support lakesoul catalog",
apache#32164), a change that otherwise has nothing to do with CI and touched only
those two .gitignore lines -- it looks accidental.
The existing files under .github survived only because they were already
tracked when the entry was added; .gitignore does not affect tracked files. The
entry therefore has no useful effect today, and two harmful ones:
* Any NEW file under .github -- a workflow, an action, CODEOWNERS, an issue
template -- is silently ignored. `git status` does not list it and `git add`
refuses it without -f, so it is easy to open a PR that is missing it.
* Even for a tracked file, `git add .github/workflows/foo.yml` prints
"The following paths are ignored by one of your .gitignore files" and exits
non-zero, which breaks `git add ... && git commit ...` in scripts.
Removing the entry exposes no untracked files: `git status --untracked-files=all
.github/` is empty afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017omcvWDsxEc9AyU83aBZ2g
hello-stephen pushed a commit that referenced this pull request Sep 3, 2026
…67491)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #66957 (introduced the title-checker regression), #67487 (a
PR currently blocked by it)
Problem Summary:
Two independent bugs in the repository's `.github` tooling, both of
which make it easy to get a PR wrong for reasons unrelated to its
content.
---
#### 1. The PR title checker rejects any hyphen in the type or scope
```
[fix](arrow-flight) ...
[feature](inverted-index) ...
[improvement](github-actions) ...
```
**88 of the last 1500 commits on master use such a title**, including
#66957 itself — the change that introduced the current check. Its own
title, `[improvement](github-actions) Reduce redundant GitHub Actions
runs and checkouts`, would not pass the checker it added.
**Root cause.** #66957 replaced the `deepakputhraya/action-pr-title`
action with an inline `grep -qE` and kept the action's regex verbatim,
with the comment "Same regex as the previously used ... submodule". But
that action is **JavaScript**, where `\-` inside a character class is a
valid escape for a literal hyphen. **POSIX ERE has no such escape** — a
backslash inside a bracket expression is just a backslash. So
```
[a-zA-Z0-9 \-_]
```
does not mean "letters, digits, space, hyphen, underscore". It makes `\`
a member and then reads `-_` as a range endpoint, which leaves the
hyphen itself out of the set. Porting the pattern from JS to `grep`
silently changed its meaning.
The fix puts the literal hyphen last in the bracket expression, which is
how POSIX spells it:
```diff
-if ! grep -qE '\[([a-zA-Z0-9 \-_])+\]\(([a-zA-Z0-9 \-_])+\)(.*)' <<< "${TITLE}"; then
+if ! grep -qE '\[([a-zA-Z0-9 _-])+\]\(([a-zA-Z0-9 _-])+\)(.*)' <<< "${TITLE}"; then
```
A comment now records why the JS form cannot be restored verbatim, so
the pattern is not "fixed back" later.
#### 2. `.gitignore` ignores `.github`
`.gitignore` has had a bare `.github` entry under its `# other` section
since 9b5a464 (`[Feature][external catalog/lakesoul] support
lakesoul catalog`, #32164) — a change that otherwise has nothing to do
with CI and touched only those two `.gitignore` lines, so it looks
accidental.
The existing files under `.github` survived only because they were
already tracked when the entry was added; `.gitignore` does not affect
tracked files. The entry therefore has no useful effect today, and two
harmful ones:
* Any **new** file under `.github` — a workflow, an action,
`CODEOWNERS`, an issue template — is silently ignored. `git status` does
not list it and `git add` refuses it without `-f`, so it is easy to open
a PR that is missing it.
* Even for a **tracked** file, `git add .github/workflows/foo.yml`
prints `The following paths are ignored by one of your .gitignore files`
and exits non-zero, which breaks `git add ... && git commit ...` in
scripts. This happened while preparing this very PR.
```
$ git check-ignore -v --no-index .github/workflows/new-thing.yml
.gitignore:155:.github .github/workflows/new-thing.yml
```
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.dev/3.0.0-mergedkind/featureCategorizes issue or PR as related to a new feature.meta-changereviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] (LakeSoul) Support LakeSoul catalog

7 participants

@Ceng23333@doris-robot@morningman@kaka11chen@dataroaring@moresun@dmetasoul-opensource