Repository navigation
feat: Implement IR cursor to allow searches over IR streams. #29
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 32 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
36c02c7
ee14262
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -21,3 +21,4 @@ FetchContent_Declare( | |||||
| FetchContent_Populate(ystdlib_cpp) | ||||||
|
|
||||||
| set(CLP_YSTDLIB_SOURCE_DIRECTORY "${ystdlib_cpp_SOURCE_DIR}") | ||||||
| include_directories(${ystdlib_cpp_SOURCE_DIR}/src) | ||||||
|
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 (assertive) Avoid global include_directories; mark third-party headers as SYSTEM. Use SYSTEM (and optionally BEFORE) to reduce warning noise and unintended global leakage. -include_directories(${ystdlib_cpp_SOURCE_DIR}/src)
+include_directories(SYSTEM BEFORE ${ystdlib_cpp_SOURCE_DIR}/src)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||||||||||||||
| # Copyright (c) Facebook, Inc. and its affiliates. | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||||||
| velox_add_library( | ||||||||||||||||||||||||||||||||||||||
| clp-s-ir-search | ||||||||||||||||||||||||||||||||||||||
| STATIC | ||||||||||||||||||||||||||||||||||||||
| ClpIrCursor.cpp | ||||||||||||||||||||||||||||||||||||||
| ClpIrCursor.h | ||||||||||||||||||||||||||||||||||||||
| ClpIrUnitHandler.cpp | ||||||||||||||||||||||||||||||||||||||
| ClpIrUnitHandler.h | ||||||||||||||||||||||||||||||||||||||
| ClpIrVectorLoader.cpp | ||||||||||||||||||||||||||||||||||||||
| ClpIrVectorLoader.h) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| velox_link_libraries( | ||||||||||||||||||||||||||||||||||||||
| clp-s-ir-search | ||||||||||||||||||||||||||||||||||||||
| PUBLIC clp_s::archive_reader | ||||||||||||||||||||||||||||||||||||||
| PRIVATE | ||||||||||||||||||||||||||||||||||||||
| clp_s::clp_dependencies | ||||||||||||||||||||||||||||||||||||||
|
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. We should make a note to export library targets for the IR stream code inside of the CLP repo -- relying on
Author
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. How about I adding a comment "# After IR-stream-related targets are exported, we should replace this with finer granularity dependencies" above
Author
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.
|
||||||||||||||||||||||||||||||||||||||
| clp_s::io | ||||||||||||||||||||||||||||||||||||||
| clp_s::search | ||||||||||||||||||||||||||||||||||||||
| clp_s::search::kql | ||||||||||||||||||||||||||||||||||||||
| velox_vector) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+33
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. 🛠️ Refactor suggestion Link against the IR stream library, not the archive reader. This target implements IR functionality (uses ::clp::ffi::ir_stream symbols elsewhere in this PR). Linking PUBLIC to clp_s::archive_reader looks wrong and may fail at link time or hide missing deps. Please switch to the IR stream target (exact name may vary by clp_s version; e.g., clp_s::ir_stream) and keep search/kql as needed: velox_link_libraries(
clp-s-ir-search
- PUBLIC clp_s::archive_reader
+ PUBLIC clp_s::ir_stream
PRIVATE
clp_s::clp_dependencies
clp_s::io
clp_s::search
clp_s::search::kql
velox_vector)If the clp_s export name differs, adjust accordingly (e.g., clp_s::ir, clp_s::ir_reader). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,208 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Copyright (c) Facebook, Inc. and its affiliates. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "clp_s/ColumnReader.hpp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "clp_s/InputConfig.hpp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "ffi/ir_stream/search/QueryHandler.hpp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/ir/ClpIrCursor.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/ir/ClpIrVectorLoader.h" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using namespace clp_s; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace facebook::velox::connector::clp::search_lib { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint64_t ClpIrCursor::fetchNext(uint64_t numRows) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readerIndex_ = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (ErrorCode::Success != errorCode_) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (false == currentSplitLoaded_) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| errorCode_ = loadSplit(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (ErrorCode::Success != errorCode_) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto deserializeResult = deserialize(numRows); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (deserializeResult.has_error()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto error = deserializeResult.error(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VELOX_FAIL( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "IR file {} might be broken, failed to deserialize. {}: {}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this->splitPath_, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error.category().name(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error.message()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return irDeserializer_->get_num_log_events_deserialized(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
anlowee marked this conversation as resolved.
Resolved
anlowee marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t ClpIrCursor::getNumFilteredRows() const { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return irDeserializer_->get_ir_unit_handler().getFilteredLogEvents()->size(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VectorPtr ClpIrCursor::createVector( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| memory::MemoryPool* pool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const TypePtr& vectorType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t vectorSize) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VELOX_CHECK_EQ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumnNameNodeIdMap_.size(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| outputColumns_.size(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Projected columns size {} does not match fields size {}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumnNameNodeIdMap_.size(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| outputColumns_.size()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return createVectorHelper(pool, vectorType, vectorSize); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ErrorCode ClpIrCursor::loadSplit() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto networkAuthOption = inputSource_ == InputSource::Filesystem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? NetworkAuthOption{.method = AuthMethod::None} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : NetworkAuthOption{.method = AuthMethod::S3PresignedUrlV4}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto irHandler = ClpIrUnitHandler{}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto projections = splitFieldsToNamesAndTypes(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto queryHandlerResult{QueryHandlerType::create( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectionResolutionCallback_, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::move(expr_), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projections, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ignoreCase_)}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!queryHandlerResult) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VLOG(2) << "Failed to create query handler for deserialization."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode::InternalError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto queryHandler = std::move(queryHandlerResult).value(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto irPath = Path{.source = inputSource_, .path = splitPath_}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irReader_ = try_create_reader(irPath, networkAuthOption); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (nullptr == irReader_) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VLOG(2) << "Failed to open kv-ir stream \"" << splitPath_ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| << "\" for reading."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode::InternalError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto deserializerResult = ::clp::ffi::ir_stream::make_deserializer( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *irReader_, std::move(irHandler), std::move(queryHandler)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!deserializerResult) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VLOG(2) << "Failed to create deserializer for deserialization."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode::InternalError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irDeserializer_ = std::make_shared< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ::clp::ffi::ir_stream::Deserializer<ClpIrUnitHandler, QueryHandlerType>>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::move(deserializerResult).value()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentSplitLoaded_ = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode::Success; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::vector<std::pair<std::string, search::ast::literal_type_bitmask_t>> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClpIrCursor::splitFieldsToNamesAndTypes() const { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto result = std::vector< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::pair<std::string, search::ast::literal_type_bitmask_t>>{}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (size_t i{0}; i < outputColumns_.size(); ++i) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto column = outputColumns_[i]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search::ast::literal_type_bitmask_t literalType; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| switch (column.type) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::Array: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::ArrayT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::Boolean: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::BooleanT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::Float: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::FloatT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::Integer: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::IntegerT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::String: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::VarStringT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case ColumnType::Timestamp: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: IR timestamp support pending; constrain to Unknown to avoid | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // mismatched projections. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::EpochDateT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| literalType = search::ast::LiteralType::UnknownT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result.emplace_back(column.name, literalType); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ystdlib::error_handling::Result<void> ClpIrCursor::deserialize( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint64_t numRows) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irDeserializer_->get_ir_unit_handler().clearFilteredLogEvents(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint64_t cnt{0}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while (cnt < numRows) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto deserializeResult = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irDeserializer_->deserialize_next_ir_unit(*irReader_); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (deserializeResult.has_error()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto error = deserializeResult.error(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (std::errc::result_out_of_range == error || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irDeserializer_->is_stream_completed()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+156
to
+162
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. Judging by some comments in the clp code it looks like we might switch from
Author
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. @LinZhihao-723 Do we have such an issue?
Member
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. Yes. We were planning to add our own error code for IR serde. I think we will have it in at some point.
Author
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. Do we have an issue link so that I can put it in the code comment? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cnt++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. I think we're trying to count matching results here right? In that case we need to rely on the fact that for search the I think we should be able to do something like
Author
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. Here actually we are counting the scanned log events. The Do you think we change the 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. I see, in that case what we should do is something like to make sure we're just counting scanned log events instead of counting all IR units. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ystdlib::error_handling::success(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+143
to
+170
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. Compare std::error_code correctly to std::errc. Directly comparing - if (std::errc::result_out_of_range == error ||
+ if (error == std::make_error_code(std::errc::result_out_of_range) ||
irDeserializer_->is_stream_completed()) {
break;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VectorPtr ClpIrCursor::createVectorHelper( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| memory::MemoryPool* pool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const TypePtr& vectorType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t vectorSize) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (vectorType->kind() == TypeKind::ROW) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::vector<VectorPtr> children; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto& rowType = vectorType->as<TypeKind::ROW>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (uint32_t i = 0; i < rowType.size(); ++i) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| children.push_back( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createVectorHelper(pool, rowType.childAt(i), vectorSize)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+177
to
+182
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 (assertive) Pre-reserve children vector capacity. Minor allocation win for ROW vectors. - std::vector<VectorPtr> children;
+ std::vector<VectorPtr> children;
+ children.reserve(rowType.size());
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return std::make_shared<RowVector>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pool, vectorType, nullptr, vectorSize, std::move(children)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto vector = BaseVector::create(vectorType, vectorSize, pool); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vector->setNulls(allocateNulls(vectorSize, pool, bits::kNull)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VELOX_CHECK_LT( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readerIndex_, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumnNameNodeIdMap_.size(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Suggested change
Otherwise we would crash in situations where not all columns have been resolved yet when we should just be marshalling nulls for unresolved fields. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Reader index out of bounds"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto projectedColumn = outputColumns_[readerIndex_]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto projectedColumnType = projectedColumn.type; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto it = projectedColumnNameNodeIdMap_.find(projectedColumn.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Related to my other comment -- we should end up being able to directly look up |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VELOX_CHECK( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it != projectedColumnNameNodeIdMap_.end(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Projected column '{}' not found in node id map", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumn.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Related to my other comment -- instead of asserting here we should be marshalling nulls for fields that haven't resolved to any specific nodes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto projectedColumnNodeId = it->second; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readerIndex_++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return std::make_shared<LazyVector>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vectorType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vectorSize, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::make_unique<ClpIrVectorLoader>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumnType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectedColumnNodeId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| irDeserializer_->get_ir_unit_handler().getFilteredLogEvents()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::move(vector)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } // namespace facebook::velox::connector::clp::search_lib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,89 @@ | ||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||
| * Copyright (c) Facebook, Inc. and its affiliates. | ||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| #pragma once | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| #include "ffi/ir_stream/Deserializer.hpp" | ||||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/BaseClpCursor.h" | ||||||||||||||||||||||||||||||||
| #include "velox/connectors/clp/search_lib/ir/ClpIrUnitHandler.h" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+22
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. Forward-declare clp::ReaderInterface to satisfy pointer usage. You declare a member #include "velox/connectors/clp/search_lib/ir/ClpIrUnitHandler.h"
+namespace clp {
+class ReaderInterface;
+} // namespace clp📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsAdd required standard headers; avoid relying on transitive includes. This header uses std::string, std::string_view, std::vector, std::unordered_map, std::shared_ptr, std::function, and std::pair but doesn’t include their headers. This can break downstream builds. #pragma once
+#include <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+#include <utility>
+#include <vector>
+
#include "ffi/ir_stream/Deserializer.hpp"
#include "velox/connectors/clp/search_lib/BaseClpCursor.h"
#include "velox/connectors/clp/search_lib/ir/ClpIrUnitHandler.h"🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| namespace facebook::velox::connector::clp::search_lib { | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| class ClpIrCursor final : public BaseClpCursor { | ||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||
| explicit ClpIrCursor( | ||||||||||||||||||||||||||||||||
| clp_s::InputSource inputSource, | ||||||||||||||||||||||||||||||||
| std::string_view splitPath, | ||||||||||||||||||||||||||||||||
| bool ignoreCase) | ||||||||||||||||||||||||||||||||
| : BaseClpCursor(inputSource, splitPath), ignoreCase_(ignoreCase) {} | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ClpIrCursor(const ClpIrCursor&) = delete; | ||||||||||||||||||||||||||||||||
| ClpIrCursor& operator=(const ClpIrCursor&) = delete; | ||||||||||||||||||||||||||||||||
| ClpIrCursor(ClpIrCursor&&) = delete; | ||||||||||||||||||||||||||||||||
| ClpIrCursor& operator=(ClpIrCursor&&) = delete; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| uint64_t fetchNext(uint64_t numRows) override; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| size_t getNumFilteredRows() const override; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| VectorPtr createVector( | ||||||||||||||||||||||||||||||||
| memory::MemoryPool* pool, | ||||||||||||||||||||||||||||||||
| const TypePtr& vectorType, | ||||||||||||||||||||||||||||||||
| size_t vectorSize) override; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| protected: | ||||||||||||||||||||||||||||||||
| ErrorCode loadSplit() override; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
anlowee marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||
| private: | ||||||||||||||||||||||||||||||||
| std::function<ystdlib::error_handling::Result<void>( | ||||||||||||||||||||||||||||||||
| bool, | ||||||||||||||||||||||||||||||||
| ::clp::ffi::SchemaTree::Node::id_t, | ||||||||||||||||||||||||||||||||
| std::pair<std::string_view, size_t>)> | ||||||||||||||||||||||||||||||||
| projectionResolutionCallback_ = | ||||||||||||||||||||||||||||||||
| [this]( | ||||||||||||||||||||||||||||||||
| [[maybe_unused]] bool isAutoGenerated, | ||||||||||||||||||||||||||||||||
| [[maybe_unused]] ::clp::ffi::SchemaTree::Node::id_t nodeId, | ||||||||||||||||||||||||||||||||
| [[maybe_unused]] std::pair<std::string_view, size_t> | ||||||||||||||||||||||||||||||||
| projected_key_and_index) | ||||||||||||||||||||||||||||||||
| -> ystdlib::error_handling::Result<void> { | ||||||||||||||||||||||||||||||||
| projectedColumnNameNodeIdMap_.insert( | ||||||||||||||||||||||||||||||||
| {std::string(projected_key_and_index.first), nodeId}); | ||||||||||||||||||||||||||||||||
|
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. Now that this callback also gives the index of the resolved projection within the original list of projections I think we should use that instead of the name -- it will allow this code to work correctly for cases where there are duplicate columns in the projection, and when there are two columns with the same name but different projected type. So basically change |
||||||||||||||||||||||||||||||||
| return ystdlib::error_handling::success(); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| using QueryHandlerType = ::clp::ffi::ir_stream::search::QueryHandler< | ||||||||||||||||||||||||||||||||
| decltype(projectionResolutionCallback_)>; | ||||||||||||||||||||||||||||||||
| bool ignoreCase_; | ||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||||||||||||||||||||||||||||||||
| std::shared_ptr< | ||||||||||||||||||||||||||||||||
| ::clp::ffi::ir_stream::Deserializer<ClpIrUnitHandler, QueryHandlerType>> | ||||||||||||||||||||||||||||||||
| irDeserializer_; | ||||||||||||||||||||||||||||||||
| std::shared_ptr<::clp::ReaderInterface> irReader_{nullptr}; | ||||||||||||||||||||||||||||||||
| std::unordered_map<std::string, ::clp::ffi::SchemaTree::Node::id_t> | ||||||||||||||||||||||||||||||||
| projectedColumnNameNodeIdMap_; | ||||||||||||||||||||||||||||||||
| size_t readerIndex_{0}; | ||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+75
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. 🛠️ Refactor suggestion Clear projection map and reset reader index when loading a new split. Avoid stale mappings across splits. Apply in ClpIrCursor.cpp (loadSplit()): ErrorCode ClpIrCursor::loadSplit() {
+ projectedColumnNameNodeIdMap_.clear();
+ readerIndex_ = 0;
auto networkAuthOption = inputSource_ == InputSource::Filesystem
? NetworkAuthOption{.method = AuthMethod::None}
: NetworkAuthOption{.method = AuthMethod::S3PresignedUrlV4};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| std::vector< | ||||||||||||||||||||||||||||||||
| std::pair<std::string, clp_s::search::ast::literal_type_bitmask_t>> | ||||||||||||||||||||||||||||||||
| splitFieldsToNamesAndTypes() const; | ||||||||||||||||||||||||||||||||
|
Comment on lines
77
to
79
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. 💡 Verification agent 🧩 Analysis chainUnify type qualifier for literal_type_bitmask_t in decl/def and add the direct include. Header uses clp_s::search::ast::literal_type_bitmask_t, but the .cpp shows search::ast::literal_type_bitmask_t. Make them consistent (prefer fully-qualified clp_s::search::ast::...). Proposed .cpp fix: -std::vector<std::pair<std::string, search::ast::literal_type_bitmask_t>>
+std::vector<std::pair<std::string, clp_s::search::ast::literal_type_bitmask_t>>
ClpIrCursor::splitFieldsToNamesAndTypes() const {Verification script: 🏁 Script executed: #!/bin/bash
# Check for inconsistent qualifiers between header and source.
rg -n -C2 'literal_type_bitmask_t' velox/connectors/clp/search_lib/ir/ClpIrCursor.{h,cpp}
# Check if an alias like `using search = clp_s::search` exists.
rg -n -C2 -P 'using\s+search\s*=\s*clp_s::search' velox/connectors/clp/search_lib/ir/ClpIrCursor.cppLength of output: 1422 Unify literal_type_bitmask_t qualifier and add its direct include
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ystdlib::error_handling::Result<void> deserialize(uint64_t numRows); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| VectorPtr createVectorHelper( | ||||||||||||||||||||||||||||||||
| memory::MemoryPool* pool, | ||||||||||||||||||||||||||||||||
| const TypePtr& vectorType, | ||||||||||||||||||||||||||||||||
| size_t vectorSize); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| } // namespace facebook::velox::connector::clp::search_lib | ||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.