Skip to content

refactor(clp): Prepare Grep code for deduplication with clp-s: #1143

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

Merged
merged 22 commits into from
Aug 5, 2025
Merged

refactor(clp): Prepare Grep code for deduplication with clp-s: #1143

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
93d6dae
Update Query.{cpp,hpp} to work on dictionary ids instead of pointers …
gibber9809 Jul 29, 2025
3109833
Revert "Remove dependence on changes do Query.{cpp,hpp} in order to s…
gibber9809 Jul 30, 2025
3e34814
Split core parts of Grep into GrepCore; Use templates to decouple fro…
gibber9809 Jul 29, 2025
0b91784
Update Grep tests
gibber9809 Jul 29, 2025
4df6ac0
Fix format after cherry-pick
gibber9809 Jul 30, 2025
e77b51a
Fix compilation error after cherry-pick
gibber9809 Jul 30, 2025
3f09334
Remove concept usage to help break up changes.
gibber9809 Jul 30, 2025
d3d55a5
Add new files to cmake scripts
gibber9809 Jul 30, 2025
c8bc92b
Add new files that consitute old code moved around to lint exceptions.
gibber9809 Jul 30, 2025
255fad3
Fix lint.yaml
gibber9809 Jul 30, 2025
463543b
Address review comments
gibber9809 Jul 31, 2025
7d635df
Merge remote-tracking branch 'upstream/main' into refactor-grep
gibber9809 Jul 31, 2025
e0fcba5
Address review comments
gibber9809 Jul 31, 2025
5983199
Merge remote-tracking branch 'upstream/main' into refactor-grep
gibber9809 Jul 31, 2025
56a5655
Fix issue after merge
gibber9809 Jul 31, 2025
f996556
Rename template parameter to be consistent with previous PR
gibber9809 Jul 31, 2025
7f73b23
Rename test-Grep.cpp to test-GrepCore.cpp
gibber9809 Jul 31, 2025
178ca04
Update components/core/src/clp/Query.hpp
gibber9809 Aug 4, 2025
5a04728
Update components/core/src/clp/GrepCore.hpp
gibber9809 Aug 4, 2025
72bfe06
Address review comments.
gibber9809 Aug 4, 2025
0685c2a
Merge remote-tracking branch 'upstream/main' into refactor-grep
gibber9809 Aug 4, 2025
0808664
Lint fix.
gibber9809 Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions components/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,8 @@ set(SOURCE_FILES_unitTest
src/clp/GlobalSQLiteMetadataDB.hpp
src/clp/Grep.cpp
src/clp/Grep.hpp
src/clp/GrepCore.cpp
src/clp/GrepCore.hpp
src/clp/hash_utils.cpp
src/clp/hash_utils.hpp
Comment on lines +582 to 585
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 30, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Unit-test build lists growing unmanageably

The manual source lists have grown past 700 entries. Every new pair of files must be added in three places (clg, clo, and unit tests). Consider moving to:

target_sources(unitTest PRIVATE
    $<TARGET_OBJECTS:clp_core_objects>)

or a file(GLOB ...) guarded behind an opt-in cache flag. This keeps explicit listing for production targets while sparing tests from constant churn.

Also applies to: 632-633

🤖 Prompt for AI Agents 
In components/core/CMakeLists.txt around lines 582 to 585, the unit-test source
file lists are manually maintained and have grown excessively large, requiring
updates in multiple places. To fix this, replace the explicit file listings for
the unit tests with a target_sources call that references the object files from
the clp_core_objects target, or alternatively use a file(GLOB ...) command
controlled by an opt-in cache flag. This change will reduce maintenance overhead
by avoiding repeated manual updates while keeping production target listings
explicit. Apply the same approach to lines 632-633 as well.

src/clp/ir/constants.hpp
Expand Down Expand Up @@ -627,6 +629,8 @@ set(SOURCE_FILES_unitTest
src/clp/Profiler.hpp
src/clp/Query.cpp
src/clp/Query.hpp
src/clp/QueryToken.cpp
src/clp/QueryToken.hpp
src/clp/ReaderInterface.cpp
src/clp/ReaderInterface.hpp
src/clp/ReadOnlyMemoryMappedFile.cpp
Expand Down
21 changes: 9 additions & 12 deletions components/core/src/clp/EncodedVariableInterpreter.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ class EncodedVariableInterpreter {
* Encodes a string-form variable, and if it is dictionary variable, searches for its ID in the
* given variable dictionary.
* @tparam VariableDictionaryReaderType
* @tparam VariableDictionaryEntryType
* @param var_str
* @param var_dict
* @param ignore_case
Expand All @@ -194,9 +193,7 @@ class EncodedVariableInterpreter {
* dictionary
* @return false otherwise
*/
template <
typename VariableDictionaryReaderType,
typename VariableDictionaryEntryType = typename VariableDictionaryReaderType::entry_t>
template <typename VariableDictionaryReaderType>
static bool encode_and_search_dictionary(
std::string_view var_str,
VariableDictionaryReaderType const& var_dict,
Expand Down Expand Up @@ -437,7 +434,7 @@ bool EncodedVariableInterpreter::decode_variables_into_message(
return true;
}

template <typename VariableDictionaryReaderType, typename VariableDictionaryEntryType>
template <typename VariableDictionaryReaderType>
bool EncodedVariableInterpreter::encode_and_search_dictionary(
std::string_view var_str,
VariableDictionaryReaderType const& var_dict,
Expand Down Expand Up @@ -468,20 +465,18 @@ bool EncodedVariableInterpreter::encode_and_search_dictionary(

if (entries.size() == 1) {
auto const* entry = entries.at(0);
sub_query.add_dict_var(encode_var_dict_id(entry->get_id()), entry);
sub_query.add_dict_var(encode_var_dict_id(entry->get_id()), entry->get_id());
return true;
}

std::unordered_set<VariableDictionaryEntryType const*> const entries_set{
entries.cbegin(),
entries.cend()
};
std::unordered_set<encoded_variable_t> encoded_vars;
std::unordered_set<variable_dictionary_id_t> var_dict_ids;
encoded_vars.reserve(entries.size());
for (auto const* entry : entries) {
encoded_vars.emplace(encode_var_dict_id(entry->get_id()));
var_dict_ids.emplace(entry->get_id());
}
sub_query.add_imprecise_dict_var(encoded_vars, entries_set);
sub_query.add_imprecise_dict_var(encoded_vars, var_dict_ids);
}

return true;
Expand All @@ -504,11 +499,13 @@ bool EncodedVariableInterpreter::wildcard_search_dictionary_and_get_encoded_matc

// Encode matches
std::unordered_set<encoded_variable_t> encoded_vars;
std::unordered_set<variable_dictionary_id_t> var_dict_ids;
for (auto entry : var_dict_entries) {
encoded_vars.emplace(encode_var_dict_id(entry->get_id()));
var_dict_ids.emplace(entry->get_id());
}

sub_query.add_imprecise_dict_var(encoded_vars, var_dict_entries);
sub_query.add_imprecise_dict_var(encoded_vars, var_dict_ids);

return true;
}
Expand Down
Loading
Loading