Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-227]fix issues from codescan #225

Merged
merged 7 commits into from
Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ public ArrowRecordBatch readNext() throws IOException {
ArrowRecordBatchBuilderImpl recordBatchBuilderImpl =
new ArrowRecordBatchBuilderImpl(recordBatchBuilder);
ArrowRecordBatch batch = recordBatchBuilderImpl.build();
if (batch == null) {
throw new IllegalArgumentException("failed to build record batch");
}
this.lastReadLength = batch.getLength();
return batch;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,10 @@ private static File moveFileFromJarToTemp(String tmpDir, String libraryToLoad) t
try (final InputStream is = JniUtils.class.getClassLoader().getResourceAsStream(libraryToLoad)) {
if (is == null) {
throw new FileNotFoundException(libraryToLoad);
} else {
try {
Files.copy(is, temp.toPath());
} catch (Exception e) {
}
}
try {
Files.copy(is, temp.toPath());
} catch (Exception e) {
}
}
return temp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ArrowComputeCodeGenerator : public CodeGenerator {
#endif
}

~ArrowComputeCodeGenerator() {
virtual ~ArrowComputeCodeGenerator() {
expr_visitor_cache_.clear();
visitor_list_.clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class ProjectKernel::Impl {
auto field_node = std::dynamic_pointer_cast<gandiva::FieldNode>(node);
input_field_list_.push_back(field_node->field());
}
pool_ = nullptr;
}

arrow::Status MakeResultIterator(
Expand Down Expand Up @@ -125,6 +126,7 @@ ProjectKernel::ProjectKernel(arrow::compute::ExecContext* ctx,
const gandiva::NodeVector& project_list) {
impl_.reset(new Impl(ctx, input_field_node_list, project_list));
kernel_name_ = "ProjectKernel";
ctx_ = nullptr;
}

arrow::Status ProjectKernel::MakeResultIterator(
Expand Down Expand Up @@ -153,6 +155,7 @@ class FilterKernel::Impl {
auto field_node = std::dynamic_pointer_cast<gandiva::FieldNode>(node);
input_field_list_.push_back(field_node->field());
}
pool_ = nullptr;
}

arrow::Status MakeResultIterator(
Expand All @@ -178,7 +181,8 @@ class FilterKernel::Impl {
&condition_node_visitor));
codegen_ctx->process_codes += condition_node_visitor->GetPrepare();
for (auto header : condition_node_visitor->GetHeaders()) {
if (std::find(codegen_ctx->header_codes.begin(), codegen_ctx->header_codes.end(),
if (!header.empty() &&
std::find(codegen_ctx->header_codes.begin(), codegen_ctx->header_codes.end(),
header) == codegen_ctx->header_codes.end()) {
codegen_ctx->header_codes.push_back(header);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ gandiva::ExpressionPtr GetHash32Kernel(std::vector<gandiva::NodePtr> key_list,
seed = func_node;
}
func_node_list.push_back(func_node);
return gandiva::TreeExprBuilder::MakeExpression(func_node_list[0],
return gandiva::TreeExprBuilder::MakeExpression(func_node_list.at(0),
arrow::field("hash_key", ret_type));
}

Expand Down Expand Up @@ -605,12 +605,19 @@ arrow::Status CompileCodes(std::string codes, std::string signature) {
std::string exec(const char* cmd) {
std::array<char, 128> buffer;
std::string result;
std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd, "r"), pclose);
FILE* file = popen(cmd, "r");
std::unique_ptr<FILE, decltype(&pclose)> pipe(file, pclose);
if (!pipe) {
pclose(file);
throw std::runtime_error("popen() failed!");
}
while (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
result += buffer.data();
try {
while (fgets(buffer.data(), sizeof buffer, pipe.get()) != nullptr) {
result += buffer.data();
}
} catch (...) {
pclose(file);
throw;
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,8 @@ std::string CodeGenNodeVisitor::GetNaNCheckStr(std::string left, std::string rig
std::string CodeGenNodeVisitor::CombineValidity(std::vector<std::string> validity_list) {
bool first = true;
std::stringstream out;
for (auto validity : validity_list) {
for (int i = 0; i < validity_list.size(); i++) {
auto validity = validity_list[i];
if (first) {
if (validity.compare("true") != 0) {
out << validity;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ class ConditionedMergeJoinKernel::Impl {
THROW_NOT_OK(GetIndexList(result_schema_, left_field_list_, right_field_list_, true,
&exist_index_, &result_schema_index_list_));
}

pool_ = nullptr;
}

arrow::Status MakeResultIterator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ class ConditionedProbeKernel::Impl {
THROW_NOT_OK(GetIndexList(result_schema_, left_field_list_, right_field_list_, true,
&exist_index_, &result_schema_index_list_));
}

pool_ = nullptr;
}

arrow::Status MakeResultIterator(
Expand Down Expand Up @@ -412,6 +414,9 @@ class ConditionedProbeKernel::Impl {
auto iter = dependent_iter_list[0];
auto typed_dependent =
std::dynamic_pointer_cast<ResultIterator<HashRelation>>(iter);
if (typed_dependent == nullptr) {
throw std::runtime_error("casting on hash relation iterator failed");
}
RETURN_NOT_OK(typed_dependent->Next(&hash_relation_));

// chendi: previous result_schema_index_list design is little tricky, it
Expand Down Expand Up @@ -768,40 +773,44 @@ class ConditionedProbeKernel::Impl {
case TypeTraits<InType>::type_id: { \
using ArrayType_ = precompile::TypeTraits<InType>::ArrayType; \
auto typed_first_key_arr = std::make_shared<ArrayType_>(key_payloads[0]); \
if (typed_first_key_arr->null_count() == 0) { \
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) { \
return hash_relation_->Get(typed_key_array->GetView(i), \
typed_first_key_arr->GetView(i)); \
}; \
} else { \
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) { \
if (typed_first_key_arr->IsNull(i)) { \
return hash_relation_->GetNull(); \
} else { \
if (typed_first_key_arr) { \
if (typed_first_key_arr->null_count() == 0) { \
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) { \
return hash_relation_->Get(typed_key_array->GetView(i), \
typed_first_key_arr->GetView(i)); \
} \
}; \
}; \
} else { \
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) { \
if (typed_first_key_arr->IsNull(i)) { \
return hash_relation_->GetNull(); \
} else { \
return hash_relation_->Get(typed_key_array->GetView(i), \
typed_first_key_arr->GetView(i)); \
} \
}; \
} \
} \
} break;
PROCESS_SUPPORTED_TYPES(PROCESS)
#undef PROCESS
case TypeTraits<arrow::StringType>::type_id: {
auto typed_first_key_arr = std::make_shared<StringArray>(key_payloads[0]);
if (typed_first_key_arr->null_count() == 0) {
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) {
return hash_relation_->Get(typed_key_array->GetView(i),
typed_first_key_arr->GetString(i));
};
} else {
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) {
if (typed_first_key_arr->IsNull(i)) {
return hash_relation_->GetNull();
} else {
if (typed_first_key_arr) {
if (typed_first_key_arr->null_count() == 0) {
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) {
return hash_relation_->Get(typed_key_array->GetView(i),
typed_first_key_arr->GetString(i));
}
};
};
} else {
fast_probe = [this, typed_key_array, typed_first_key_arr](int i) {
if (typed_first_key_arr->IsNull(i)) {
return hash_relation_->GetNull();
} else {
return hash_relation_->Get(typed_key_array->GetView(i),
typed_first_key_arr->GetString(i));
}
};
}
}
} break;
default: {
Expand Down Expand Up @@ -1001,6 +1010,7 @@ class ConditionedProbeKernel::Impl {
uint64_t Evaluate(std::shared_ptr<arrow::Array> key_array,
const arrow::ArrayVector& key_payloads) override {
auto typed_key_array = std::dynamic_pointer_cast<arrow::Int32Array>(key_array);
assert(typed_key_array != nullptr);
std::vector<std::shared_ptr<UnsafeArray>> payloads;
int i = 0;
bool do_unsafe_row = true;
Expand Down Expand Up @@ -1071,7 +1081,9 @@ class ConditionedProbeKernel::Impl {
if (!do_unsafe_row) {
index = fast_probe(i);
} else {
unsafe_key_row->reset();
if (unsafe_key_row) {
unsafe_key_row->reset();
}
for (auto payload_arr : payloads) {
payload_arr->Append(i, &unsafe_key_row);
}
Expand Down Expand Up @@ -1853,6 +1865,7 @@ ConditionedProbeKernel::ConditionedProbeKernel(
right_schema_list, condition, join_type, result_schema,
hash_configuration_list, hash_relation_idx));
kernel_name_ = "ConditionedProbeKernel";
ctx_ = nullptr;
}

arrow::Status ConditionedProbeKernel::MakeResultIterator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1026,8 +1026,8 @@ arrow::Status ExpressionCodegenVisitor::Visit(const gandiva::BooleanNode& node)
field_type_ = mixed;
}
for (auto header : child_visitor->GetHeaders()) {
if (std::find(header_list_.begin(), header_list_.end(), header) ==
header_list_.end()) {
if (!header.empty() && std::find(header_list_.begin(), header_list_.end(),
header) == header_list_.end()) {
header_list_.push_back(header);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class HashAggregateKernel::Impl {
if (no_result_project) return;
}
result_expr_list_ = result_expr_node_list;
pool_ = nullptr;
}

virtual arrow::Status MakeResultIterator(
Expand Down Expand Up @@ -1113,6 +1114,7 @@ HashAggregateKernel::HashAggregateKernel(
impl_.reset(new Impl(ctx, input_field_list, action_list, result_field_node_list,
result_expr_node_list));
kernel_name_ = "HashAggregateKernelKernel";
ctx_ = ctx;
}
#undef PROCESS_SUPPORTED_TYPES

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class HashRelationKernel::Impl {
std::shared_ptr<gandiva::Node> root_node,
const std::vector<std::shared_ptr<arrow::Field>>& output_field_list)
: ctx_(ctx), input_field_list_(input_field_list) {
pool_ = ctx_->memory_pool();
std::vector<std::shared_ptr<HashRelationColumn>> hash_relation_list;
for (auto field : input_field_list) {
std::shared_ptr<HashRelationColumn> hash_relation_column;
Expand Down Expand Up @@ -129,7 +130,11 @@ class HashRelationKernel::Impl {
// If single key case, we can put key in KeyArray
auto key_type = std::dynamic_pointer_cast<arrow::FixedWidthType>(
key_hash_field_list[0]->type());
key_size_ = key_type->bit_width() / 8;
if (key_type) {
key_size_ = key_type->bit_width() / 8;
} else {
key_size_ = 0;
}
hash_relation_ =
std::make_shared<HashRelation>(ctx_, hash_relation_list, key_size_);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class HashArrayKernel::Impl {
}
index++;
}
assert(func_node_list.size() > 0);
expr = gandiva::TreeExprBuilder::MakeExpression(func_node_list[0],
arrow::field("res", arrow::int64()));
#ifdef DEBUG
Expand Down Expand Up @@ -307,6 +308,7 @@ ConcatArrayKernel::ConcatArrayKernel(
std::vector<std::shared_ptr<arrow::DataType>> type_list) {
impl_.reset(new Impl(ctx, type_list));
kernel_name_ = "ConcatArrayKernel";
ctx_ = nullptr;
}

arrow::Status ConcatArrayKernel::Evaluate(const ArrayList& in,
Expand All @@ -323,6 +325,7 @@ class CachedRelationKernel::Impl {
result_schema_(result_schema),
key_field_list_(key_field_list),
result_type_(result_type) {
pool_ = ctx_->memory_pool();
for (auto field : key_field_list) {
auto indices = result_schema->GetAllFieldIndices(field->name());
if (indices.size() != 1) {
Expand Down
Loading