Allow multiple indices at the same time. #2476
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit removes the global singleton pointers from IndexImpl and refactors all dependent classes to explicitly store IndexImpl* instead. This improves code clarity, testability, and eliminates global state. Key changes: - Align IndexImpl to 64 bytes for pointer compression opportunities - Remove globalSingletonIndex_ and globalSingletonComparator_ statics - Add CompressedPointer utility to pack pointer + bool flag efficiently - LocalVocab now stores and propagates IndexImpl* to entries - LocalVocabEntry uses compressed pointer (saves 8 bytes per entry) - LiteralOrIri stores IndexImpl* for comparator access - Update test infrastructure to use explicit index references The refactoring maintains functionality while eliminating global state and reducing memory usage through pointer compression in LocalVocabEntry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Per review feedback, this commit makes the IndexImpl* parameter required for LocalVocab and LiteralOrIri, removing the nullptr default. This ensures proper initialization and catches errors earlier. Key changes: - LocalVocab constructor now requires IndexImpl*, asserts non-null - LocalVocab::merge() checks all vocabs have same IndexImpl* - LocalVocabEntry constructors require IndexImpl*, assert non-null - LiteralOrIri factory methods require IndexImpl*, assert non-null - Added Operation::makeLocalVocab() helper for convenient creation - Updated all engine source files to use makeLocalVocab() - Added test infrastructure: getTestIndexImpl() and literalOrIriForTesting() The refactoring properly enforces the invariant that IndexImpl* is never null and that merged LocalVocabs always share the same index. Note: Test files still need updating to use the new required parameters. This will be done in a follow-up commit to keep changes manageable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Updated all TEST functions in LocalVocabTest.cpp to get index from getQec()
- Added index parameter to all LocalVocab{} constructors
- Added index parameter to all lit() and LiteralOrIri factory method calls
- Updated all TEST functions in LiteralOrIriTest.cpp similarly
- All LiteralOrIri factory methods (iriref, literalWithoutQuotes, literalWithQuotes, prefixedIri, fromStringRepresentation) now receive required index parameter
- Updated TestContextWithGivenTTl to initialize LocalVocab with index - Added index parameter to all LiteralOrIri factory calls in helper functions: - checkLiteralContentAndDatatypeFromLiteralOrIri: toLiteralOrIri lambda - checkUnitValueGetterFromLiteralOrIri: litTest lambda and IRI test - checkGeoInfoFromLocalVocab: fromStringRepresentation and LocalVocabEntry - checkGeoInfoFromLiteral: fromStringRepresentation - All helpers now properly propagate IndexImpl* through test infrastructure
- IdTestHelpers.h: Updated LocalVocabId lambda to get index from getTestIndexImpl() - Added forward declaration for getTestIndexImpl() - Added contract check to ensure index is set before use - Passed index to static LocalVocab constructor and literalWithoutQuotes() - SparqlExpressionTestHelpers.h: Updated TestContext struct - Initialize LocalVocab with index from qec - Added index parameter to addLocalLiteral and addLocalIri lambdas - All LiteralOrIri factory calls now receive required index parameter
…structors - TripleComponent.h: Get index from localVocab when creating LiteralOrIri - ScanSpecification.cpp: Initialize LocalVocab with &index in toScanSpecification - ScanSpecification.h: Removed default parameter for LocalVocab (no longer has default constructor) - DeltaTriples.h: Initialize localVocab_ with &index in constructor - AddCombinedRowToTable.h: - Added IndexImpl* parameter to both constructors - Initialize mergedVocab_ with index in initializer lists - Replaced makeLocalVocab() calls with mergedVocab_.clear() - Join.cpp, MultiColumnJoin.cpp, OptionalJoin.cpp: - Pass &getIndex().getImpl() to AddCombinedRowToIdTable constructors
… analysis - Added overloads of litOrIriToUnit that take Iri or Literal directly - Updated GeoExpression.cpp to call these overloads instead of constructing LiteralOrIri - This allows unit extraction during query planning without needing IndexImpl*
- Added util/Exception.h include to LiteralOrIri.h for AD_CONTRACT_CHECK - Changed TripleComponent.h to use localVocab.getIndex() instead of accessing private member
Overview
Conformance check failed ❌Test Status Changes 📊
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes the global singleton pointers from IndexImpl and refactors all dependent classes to explicitly store IndexImpl* instead. This improves code clarity, testability, and eliminates global state.
Key changes:
The refactoring maintains functionality while eliminating global state and reducing memory usage through pointer compression in LocalVocabEntry.
🤖 Generated with Claude Code