-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Add timestamp type support for kv-ir splits. #30
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.
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
Changes from 38 commits
5af7ec3
fe8a130
cc82dd7
65d3bd1
d45baf0
7f40b7e
c0cac3d
c7bd83b
27ba897
19bad61
d3fafde
f53c6c7
e466f68
8be8eb7
d6ba0da
221bec6
679d338
6e1462d
f8f75c3
1c091c7
8ad9b9c
7f25210
28e9a6f
595e74b
1b12402
5db1cf9
e71a957
cc82c62
3ca7361
184f9de
2102275
f9e1409
4ef6e96
e576db7
fd2b308
82778cc
b257938
10e222a
1928748
1982c3b
947fded
03d251d
da66e99
ae97766
a6a9e49
4c3e4e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,10 +59,10 @@ VectorPtr ClpIrCursor::createVector( | |||||||||||||||||||
| const TypePtr& vectorType, | ||||||||||||||||||||
| size_t vectorSize) { | ||||||||||||||||||||
| VELOX_CHECK_EQ( | ||||||||||||||||||||
| projectedColumnIdxNodeIdMap_.size(), | ||||||||||||||||||||
| projectedColumnIdxNodeIdsMap_.size(), | ||||||||||||||||||||
| outputColumns_.size(), | ||||||||||||||||||||
| "Projected columns size {} does not match fields size {}", | ||||||||||||||||||||
| projectedColumnIdxNodeIdMap_.size(), | ||||||||||||||||||||
| projectedColumnIdxNodeIdsMap_.size(), | ||||||||||||||||||||
| outputColumns_.size()); | ||||||||||||||||||||
anlowee marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
| return createVectorHelper(pool, vectorType, vectorSize); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
@@ -135,7 +135,8 @@ ClpIrCursor::splitFieldsToNamesAndTypes() const { | |||||||||||||||||||
| case ColumnType::Timestamp: | ||||||||||||||||||||
| // TODO: IR timestamp support pending; constrain to Unknown to avoid | ||||||||||||||||||||
| // mismatched projections. | ||||||||||||||||||||
|
||||||||||||||||||||
| literalType = search::ast::LiteralType::EpochDateT; | ||||||||||||||||||||
| literalType = search::ast::LiteralType::FloatT | | ||||||||||||||||||||
| search::ast::LiteralType::IntegerT; | ||||||||||||||||||||
| break; | ||||||||||||||||||||
anlowee marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||
| default: | ||||||||||||||||||||
| literalType = search::ast::LiteralType::UnknownT; | ||||||||||||||||||||
|
|
@@ -189,22 +190,23 @@ VectorPtr ClpIrCursor::createVectorHelper( | |||||||||||||||||||
| readerIndex_, outputColumns_.size(), "Reader index out of bounds"); | ||||||||||||||||||||
| auto projectedColumn = outputColumns_[readerIndex_]; | ||||||||||||||||||||
| auto projectedColumnType = projectedColumn.type; | ||||||||||||||||||||
| auto it = projectedColumnIdxNodeIdMap_.find(readerIndex_); | ||||||||||||||||||||
| bool isResolved = it != projectedColumnIdxNodeIdMap_.end(); | ||||||||||||||||||||
| ::clp::ffi::SchemaTree::Node::id_t projectedColumnNodeId; | ||||||||||||||||||||
| auto it = projectedColumnIdxNodeIdsMap_.find(readerIndex_); | ||||||||||||||||||||
|
Comment on lines
190
to
+191
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick Avoid a copy of the projected column. Bind by const reference; Apply: - auto projectedColumn = outputColumns_[readerIndex_];
+ const auto& projectedColumn = outputColumns_[readerIndex_];📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| bool isResolved = it != projectedColumnIdxNodeIdsMap_.end(); | ||||||||||||||||||||
| std::vector<::clp::ffi::SchemaTree::Node::id_t> projectedColumnNodeIds; | ||||||||||||||||||||
| if (isResolved) { | ||||||||||||||||||||
| projectedColumnNodeId = it->second; | ||||||||||||||||||||
| projectedColumnNodeIds = it->second; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| readerIndex_++; | ||||||||||||||||||||
| return std::make_shared<LazyVector>( | ||||||||||||||||||||
| pool, | ||||||||||||||||||||
| vectorType, | ||||||||||||||||||||
| vectorSize, | ||||||||||||||||||||
| std::make_unique<ClpIrVectorLoader>( | ||||||||||||||||||||
| irDeserializer_->get_ir_unit_handler().getFilteredLogEvents(), | ||||||||||||||||||||
| isResolved, | ||||||||||||||||||||
| projectedColumnType, | ||||||||||||||||||||
| projectedColumnNodeId, | ||||||||||||||||||||
| irDeserializer_->get_ir_unit_handler().getFilteredLogEvents()), | ||||||||||||||||||||
| projectedColumnNodeIds, | ||||||||||||||||||||
| projectedColumn.name, | ||||||||||||||||||||
| projectedColumnType), | ||||||||||||||||||||
| std::move(vector)); | ||||||||||||||||||||
anlowee marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,8 +14,10 @@ | |||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/ir/ClpIrVectorLoader.h" | ||||||||||||||||||||||||||||||
| #include <chrono> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/BaseClpCursor.h" | ||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/ir/ClpIrVectorLoader.h" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
anlowee marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
| namespace facebook::velox::connector::clp::search_lib { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -33,12 +35,21 @@ void ClpIrVectorLoader::loadInternal( | |||||||||||||||||||||||||||||
| auto& logEvent = filteredLogEvents_->at(vectorIndex); | ||||||||||||||||||||||||||||||
| // TODO: also need to support auto-generated keys | ||||||||||||||||||||||||||||||
| auto userGenNodeIdValueMap = logEvent->get_user_gen_node_id_value_pairs(); | ||||||||||||||||||||||||||||||
| auto const value_it{userGenNodeIdValueMap.find(nodeId_)}; | ||||||||||||||||||||||||||||||
| if (userGenNodeIdValueMap.end() == value_it || | ||||||||||||||||||||||||||||||
| false == value_it->second.has_value()) { | ||||||||||||||||||||||||||||||
| std::unordered_map<id_t, std::optional<::clp::ffi::Value>>::iterator | ||||||||||||||||||||||||||||||
| valueIt; | ||||||||||||||||||||||||||||||
| ::clp::ffi::SchemaTree::Node::id_t nodeId; | ||||||||||||||||||||||||||||||
| for (size_t i{0}; i < nodeIds_.size(); ++i) { | ||||||||||||||||||||||||||||||
| valueIt = userGenNodeIdValueMap.find(nodeIds_[i]); | ||||||||||||||||||||||||||||||
| if (valueIt != userGenNodeIdValueMap.end()) { | ||||||||||||||||||||||||||||||
| nodeId = nodeIds_[i]; | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| for (size_t i{0}; i < nodeIds_.size(); ++i) { | |
| valueIt = userGenNodeIdValueMap.find(nodeIds_[i]); | |
| if (valueIt != userGenNodeIdValueMap.end()) { | |
| nodeId = nodeIds_[i]; | |
| break; | |
| } | |
| } | |
| for (auto const candidateNodeId : nodeIds_) { | |
| valueIt = userGenNodeIdValueMap.find(candidateNodeId); | |
| if (valueIt != userGenNodeIdValueMap.end()) { | |
| nodeId = candidateNodeId; | |
| break; | |
| } | |
| } |
Sort of up to taste, but I think this is a bit more readable.
Uh oh!
There was an error while loading. Please reload this page.