Added support for local distributed tables in join queries#3577
Conversation
There was a problem hiding this comment.
Pull Request Overview
Added support for performing joins over local partitions of distributed tables by propagating the original “parent” table name through queue creation, join sorter, and search handler logic.
- Extended queue creator and multi-queue interfaces with a
szParentparameter to carry the parent index name. - Updated join sorter and filter-checking logic to use the passed parent name for distributed-local joins.
- Modified search handler to build and track
JoinedIndexes_twith parent metadata. - Added an SQL test in
test/test_278/test.xmlfor inner joins over a local distributed table.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/test_278/test.xml | Added inner-join test case for a local-distributed table d. |
| src/queuecreator.h/.cpp | Added const char* szParent to queue-creation APIs and constructors. |
| src/joinsorter.h/.cpp | Propagated parent table names into join sorter constructors and filters. |
| src/daemon/search_handler.h/.cpp | Introduced JoinedIndexes_t, updated joined index population, and search flow. |
Comments suppressed due to low confidence (3)
src/queuecreator.h:53
- Adding the
szParentparameter changes the public API signature without a default argument or overload, breaking existing callers. Please add a default value (= nullptr) or provide an overload to maintain backward compatibility.
void sphCreateMultiQueue ( const SphQueueSettings_t & tQueue, const VecTraits_T<CSphQuery> & dQueries, VecTraits_T<ISphMatchSorter *> & dSorters, VecTraits_T<CSphString> & dErrors, SphQueueRes_t & tRes, StrVec_t * pExtra, QueryProfile_c * pProfile, const char * szParent );
test/test_278/test.xml:665
- [nitpick] The new test covers only an inner join on the distributed-local table. Consider adding tests for left joins and failure scenarios (e.g., joining on a distributed table with remote agents) to ensure full coverage of the new feature.
<!-- join over distributed left table -->
src/joinsorter.cpp:722
- [nitpick]
GetJoinedIndexName()allocates a newCSphStringon every call, which may be expensive in hot paths. Consider returning aconst CSphString&or caching the result to reduce heap allocations.
CSphString GetJoinedIndexName() const { return m_sRightIndexSchemaName.IsEmpty() ? m_pJoinedIndex->GetName() : m_sRightIndexSchemaName; }
| struct JoinedIndexes_t | ||
| { | ||
| CSphVector<const CSphIndex *> m_dIndexes; | ||
| const char * m_szParent = nullptr; |
There was a problem hiding this comment.
Storing raw const char* pointers to external CSphString data can lead to dangling pointers when the original string is modified or destroyed. Consider using a CSphString member for m_szParent to own the string and ensure safety.
| const char * m_szParent = nullptr; | |
| CSphString m_sParent; |
| { | ||
| if ( pServedDistIndex->m_dAgents.GetLength() ) | ||
| { | ||
| m_sError << "join not allowed on distributed table containing remote agents '" << tQuery.m_sJoinIdx << "'"; |
There was a problem hiding this comment.
The error message literal opens with a single quote but does not clearly close it, leading to mismatched quotes. Update the literal or add a trailing apostrophe in the string to ensure the output is consistent.
Windows test results 3 files 3 suites 24m 46s ⏱️ Results for commit ab59480. ♻️ This comment has been updated with latest results. |
Added support for local distributed tables in join queries