airborne12 opened a new pull request, #70272:
URL: https://github.com/apache/doris/pull/70272
> Stacked PR in the SNII / text analysis / ANN CLucene removal series (index
at the end). It depends on #70263 (T4) and #70271 (T12), whose commits show in
this diff until they merge; review only this PR's own commits: `7bea2215cf6`
[fix](be) Build four sources and file_scanner_v2.cpp without unity batches,
`b6c11766f3a` [refactor](be) Keep CLucene out of the shared index file headers.
It stays a draft until its dependencies merge.
### What problem does this PR solve?
Issue Number: None
Related PR: #70263, #70271
Problem Summary:
**[fix](be) Build four sources and file_scanner_v2.cpp without unity
batches** (`7bea2215cf6`)
the BE does not build with ENABLE_UNITY_BUILD=OFF and
ENABLE_PCH=OFF. Four sources use a declaration they never include, which a
unity batch hides behind a sibling that includes it, and file_scanner_v2.cpp
defines two constants nothing reads, which clang reports (an error under
-Werror) only for the main file of a translation unit, never inside a batch:
- format_v2/table_reader.cpp uses ShardedKVCache: include
format/format_common.h.
- load/group_commit/wal/wal_table.cpp uses WalFileReader: include
wal_file_reader.h.
- service/http/utils.cpp uses DBUG_EXECUTE_IF: include util/debug_points.h.
- storage/segment/rowid_read_ahead.cpp uses StorageReadOptions: include
storage/iterators.h.
- exec/scan/file_scanner_v2.cpp: drop kIcebergPositionDeleteContent and
kIcebergDeletionVectorContent, which no code reads.
The next change has to be built that way once, because only then does a file
that got CLucene through a shared index header show the include it lacks.
**[refactor](be) Keep CLucene out of the shared index file headers**
(`b6c11766f3a`)
SNII and ANN no longer use CLucene, but eight of their eighty
sources (snii_index_reader, snii_bkd_index_reader, snii_index_writer,
snii_bkd_index_writer, ann_index_reader, ann_index_writer,
ann_index_iterator,
ann_topn_runtime) still opened 67 CLucene headers, through the index file
headers they share with V1 and V2: index_file_writer.h and
index_file_reader.h included CLucene.h, the writer also
index_compound_reader.h
and inverted_index_searcher.h, and index_common.h ended with
index_common_impl.h, which includes CLucene. This change cuts those edges.
- index_file_writer.h forward-declares lucene::store::Directory,
DorisCompoundReader and IndexSearcherBuilder, which it names only through
pointers.
- index_file_reader.h forward-declares lucene::store::IndexInput; its
constructor, destructor and get_inverted_file_size move to the .cpp,
because
they destroy or read the stream and the entries, incomplete in the header.
- index_common.h no longer includes index_common_impl.h. The V1 and V2
sources
and the tests that instantiate its templates include it themselves, and
index_storage_format.h forward-declares lucene::store::IndexOutput.
- The -Wconversion pragmas around the removed CLucene includes go with them:
every BE source that still reaches CLucene compiles under the production
flags with CLucene.h included first and nothing suppressed.
- The V1 and V2 sources and tests that got CLucene types through
index_file_writer.h include what they use (index_compound_reader.h,
inverted_index_desc.h).
- snii_index_reader.cpp keys a field's statistics with
StringHelper::to_wstring of the column name, as predicate_collector
computes
the same key, instead of CLucene's StringUtil::string_to_wstring. The two
agree on valid UTF-8, which column names are (checked on ASCII, CJK, 2- and
4-byte characters and U+10FFFF).
- check-header-deps.py: the shared index file headers and
snii_index_reader.cpp leave the CLucene exception tables, which now list
analyzer.cpp alone, for the analysis work. The self-test's two stale-entry
cases probed entries earlier changes had removed, and failed; they now list
a clean file of a root, query_info.h, so they hold however far the tables
empty.
- snii_clucene_guard_test checks compaction/eligibility.cpp too: its
exemption
waited for the analyzer to move behind a factory, which happened earlier.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] 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? -->
Details:
- `7bea2215cf6` [fix](be) Build four sources and file_scanner_v2.cpp
without unity batches:
- ./build.sh --be --index-tool and the BE UT build pass with
ENABLE_PCH=OFF ENABLE_UNITY_BUILD=OFF; each changed file compiles
with
its own command from that build under -Werror.
- `b6c11766f3a` [refactor](be) Keep CLucene out of the shared index file
headers:
- BE UT under ASAN, unity build: the frozen goldens 19 of 20 with
EveryCodePoint skipped (it runs in FULL mode only), the index
suites
2204 of 2204, the analysis suites 3368 of 3373 with 5 skipped, the
same
as before this change; snii_clucene_guard_test passes with
eligibility.cpp checked.
- Built with ENABLE_PCH=OFF ENABLE_UNITY_BUILD=OFF, the only way the
missing includes show: ./build.sh --be --index-tool and the BE UT
build
pass.
- Every SNII and ANN source, compiled with -H -fsyntax-only and its
own
command from the non-unity build: 80 of 80 open no CLucene header
(70
SNII, 10 ANN); 8 of them did before. Over the other CLucene-free
roots
(analysis, query, common) only analyzer.cpp does, the one entry
the gate
still lists.
- check-build-hygiene.sh passes, and so do its self-tests
(build-support/tests/run.sh): with the gate's stale-entry check
disabled,
both rewritten cases fail. clang-format 16 leaves the changed
files as
they are; run-clang-tidy.sh reports nothing on the changed lines.
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
