Skip to content

Commit

Permalink
Refactor: Error logging (#1256)
Browse files Browse the repository at this point in the history 
### What problem does this PR solve?

Lots of unrecoverable error won't be print as critical level log. This
PR is going to fix it.

### Type of change

- [x] Refactoring

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
  • Loading branch information
@JinHai-CN
JinHai-CN authored May 29, 2024
1 parent 35e08f9 commit 6671d33
Show file tree
Hide file tree
Showing 319 changed files with 5,497 additions and 1,663 deletions.
5 changes: 2 additions & 3 deletions python/hello_infinity.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

import infinity
import infinity.index as index
from infinity.common import REMOTE_HOST
from infinity.common import ConflictType
from infinity.common import REMOTE_HOST, NetworkAddress, ConflictType
import pandas as pds


Expand Down Expand Up @@ -138,4 +137,4 @@ def test_chinese():

if __name__ == '__main__':
test_english()
test_chinese()
# test_chinese()
121 changes: 90 additions & 31 deletions src/executor/explain_physical_plan.cpp
View file

Large diffs are not rendered by default.

20 changes: 15 additions & 5 deletions src/executor/expression/expression_evaluator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ void ExpressionEvaluator::Execute(const SharedPtr<BaseExpression> &expr, SharedP
case ExpressionType::kIn:
return Execute(std::static_pointer_cast<InExpression>(expr), state, output_column);
default: {
UnrecoverableError(fmt::format("Unknown expression type: {}", expr->Name()));
String error_message = fmt::format("Unknown expression type: {}", expr->Name());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
}
}
Expand Down Expand Up @@ -134,11 +136,15 @@ void ExpressionEvaluator::Execute(const SharedPtr<CastExpression> &expr,
}

void ExpressionEvaluator::Execute(const SharedPtr<CaseExpression> &, SharedPtr<ExpressionState> &, SharedPtr<ColumnVector> &) {
UnrecoverableError("Case execution");
String error_message = "Case execution";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}

void ExpressionEvaluator::Execute(const SharedPtr<ColumnExpression> &, SharedPtr<ExpressionState> &, SharedPtr<ColumnVector> &) {
UnrecoverableError("Column expression");
String error_message = "Column expression";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}

void ExpressionEvaluator::Execute(const SharedPtr<FunctionExpression> &expr,
Expand Down Expand Up @@ -178,10 +184,14 @@ void ExpressionEvaluator::Execute(const SharedPtr<ReferenceExpression> &expr,
SizeT column_index = expr->column_index();

if (input_data_block_ == nullptr) {
UnrecoverableError("Input data block is NULL");
String error_message = "Input data block is NULL";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
if (column_index >= input_data_block_->column_count()) {
UnrecoverableError("Invalid column index");
String error_message = "Invalid column index";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}

output_column_vector = input_data_block_->column_vectors[column_index];
Expand Down
13 changes: 10 additions & 3 deletions src/executor/expression/expression_selector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import expression_evaluator;
import internal_types;
import third_party;
import data_type;
import logger;

import infinity_exception;

Expand Down Expand Up @@ -63,10 +64,14 @@ void ExpressionSelector::Select(const SharedPtr<BaseExpression> &expr,
return; // All data are false;
}
if (output_true_select.get() == nullptr && output_false_select.get() == nullptr) {
UnrecoverableError("No output select column vector is given");
String error_message = "No output select column vector is given";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
if (expr->Type().type() != LogicalType::kBoolean) {
UnrecoverableError("Attempting to select non-boolean expression");
String error_message = "Attempting to select non-boolean expression";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
Select(expr, state, count, output_true_select);
}
Expand All @@ -87,7 +92,9 @@ void ExpressionSelector::Select(const SharedPtr<BaseExpression> &expr,

void ExpressionSelector::Select(const SharedPtr<ColumnVector> &bool_column, SizeT count, SharedPtr<Selection> &output_true_select, bool nullable) {
if (bool_column->vector_type() != ColumnVectorType::kCompactBit || bool_column->data_type()->type() != LogicalType::kBoolean) {
UnrecoverableError("Attempting to select non-boolean expression");
String error_message = "Attempting to select non-boolean expression";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
const auto &boolean_buffer = *(bool_column->buffer_);
const auto &null_mask = bool_column->nulls_ptr_;
Expand Down
8 changes: 6 additions & 2 deletions src/executor/expression/expression_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,14 @@ SharedPtr<ExpressionState> ExpressionState::CreateState(const SharedPtr<BaseExpr
case ExpressionType::kIn:
return CreateState(static_pointer_cast<InExpression>(expression));
case ExpressionType::kKnn: {
UnrecoverableError("Unexpected expression type: KNN");
String error_message = "Unexpected expression type: KNN";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
default: {
UnrecoverableError(fmt::format("Unknown expression type: {}", expression->Name()));
String error_message = fmt::format("Unknown expression type: {}", expression->Name());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
}
return nullptr;
Expand Down
92 changes: 69 additions & 23 deletions src/executor/fragment_builder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,19 @@ void FragmentBuilder::BuildExplain(PhysicalOperator *phys_op, PlanFragment *curr
break;
}
case ExplainType::kInvalid: {
UnrecoverableError("Invalid explain type");
String error_message = "Invalid explain type";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
}
}

void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *current_fragment_ptr) {
switch (phys_op->operator_type()) {
case PhysicalOperatorType::kInvalid: {
UnrecoverableError("Invalid physical operator type");
String error_message = "Invalid physical operator type";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
case PhysicalOperatorType::kExplain: {
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand Down Expand Up @@ -130,7 +134,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kMatch: {
current_fragment_ptr->AddOperator(phys_op);
if (phys_op->left() != nullptr or phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("{} shouldn't have child.", phys_op->GetName()));
String error_message = fmt::format("{} shouldn't have child.", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
current_fragment_ptr->SetSourceNode(query_context_ptr_, SourceType::kEmpty, phys_op->GetOutputNames(), phys_op->GetOutputTypes());
Expand All @@ -139,7 +145,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kAggregate: {
current_fragment_ptr->AddOperator(phys_op);
if (phys_op->left() == nullptr) {
UnrecoverableError("No input node of aggregate operator");
String error_message = "No input node of aggregate operator";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
} else {
BuildFragments(phys_op->left(), current_fragment_ptr);
current_fragment_ptr->SetFragmentType(FragmentType::kParallelMaterialize);
Expand All @@ -151,7 +159,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kHash:
case PhysicalOperatorType::kLimit: {
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("No input node of {}", phys_op->GetName()));
String error_message = fmt::format("No input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->SetFragmentType(FragmentType::kParallelMaterialize);
current_fragment_ptr->AddOperator(phys_op);
Expand All @@ -160,7 +170,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kTop: {
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("No input node of {}", phys_op->GetName()));
String error_message = fmt::format("No input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
BuildFragments(phys_op->left(), current_fragment_ptr);
Expand All @@ -171,7 +183,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kDelete:
case PhysicalOperatorType::kSort: {
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("No input node of {}", phys_op->GetName()));
String error_message = fmt::format("No input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
BuildFragments(phys_op->left(), current_fragment_ptr);
Expand All @@ -180,11 +194,15 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kFusion: {
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("No input node of {}", phys_op->GetName()));
String error_message = fmt::format("No input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
if (phys_op->left()->operator_type() == PhysicalOperatorType::kFusion) {
if (phys_op->right() != nullptr) {
UnrecoverableError("Fusion operator with fusion operator child shouldn't have right child.");
String error_message = "Fusion operator with fusion operator child shouldn't have right child.";
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
// call next Fusion operator
Expand All @@ -203,7 +221,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetSourceNode(query_context_ptr_, SourceType::kLocalQueue, phys_op->GetOutputNames(), phys_op->GetOutputTypes());
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("No input node of {}", phys_op->GetName()));
String error_message = fmt::format("No input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);

Expand Down Expand Up @@ -234,12 +254,16 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kJoinMerge:
case PhysicalOperatorType::kJoinIndex:
case PhysicalOperatorType::kCrossProduct: {
UnrecoverableError(fmt::format("Not support {}.", phys_op->GetName()));
String error_message = fmt::format("Not support {}.", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
case PhysicalOperatorType::kMatchTensorScan:
case PhysicalOperatorType::kKnnScan: {
if (phys_op->left() != nullptr or phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("{} shouldn't have child.", phys_op->GetName()));
String error_message = fmt::format("{} shouldn't have child.", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
if (phys_op->TaskletCount() == 1) {
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand All @@ -253,7 +277,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
case PhysicalOperatorType::kTableScan:
case PhysicalOperatorType::kIndexScan: {
if (phys_op->left() != nullptr or phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("{} shouldn't have child.", phys_op->GetName()));
String error_message = fmt::format("{} shouldn't have child.", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->SetFragmentType(FragmentType::kParallelStream);
current_fragment_ptr->AddOperator(phys_op);
Expand All @@ -276,7 +302,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCreateIndexPrepare: {
if (phys_op->left() != nullptr || phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand All @@ -285,7 +313,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCreateIndexDo: {
if (phys_op->left() == nullptr || phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kParallelMaterialize);
Expand All @@ -303,7 +333,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCreateIndexFinish: {
if (phys_op->left() == nullptr || phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand All @@ -321,7 +353,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCompact: {
if (phys_op->left() != nullptr || phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kParallelMaterialize);
Expand All @@ -330,7 +364,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCompactIndexPrepare: {
if (phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand All @@ -345,13 +381,17 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
current_fragment_ptr->AddChild(std::move(next_plan_fragment));
}
if (phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
return;
}
case PhysicalOperatorType::kCompactIndexDo: {
if (phys_op->left() == nullptr || phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kParallelMaterialize);
Expand All @@ -369,7 +409,9 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
}
case PhysicalOperatorType::kCompactFinish: {
if (phys_op->left() == nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
current_fragment_ptr->AddOperator(phys_op);
current_fragment_ptr->SetFragmentType(FragmentType::kSerialMaterialize);
Expand All @@ -392,12 +434,16 @@ void FragmentBuilder::BuildFragments(PhysicalOperator *phys_op, PlanFragment *cu
current_fragment_ptr->AddChild(std::move(next_plan_fragment2));
}
} else if (phys_op->right() != nullptr) {
UnrecoverableError(fmt::format("Invalid input node of {}", phys_op->GetName()));
String error_message = fmt::format("Invalid input node of {}", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
return;
}
default: {
UnrecoverableError(fmt::format("Invalid operator type: {} in Fragment Builder", phys_op->GetName()));
String error_message = fmt::format("Invalid operator type: {} in Fragment Builder", phys_op->GetName());
LOG_CRITICAL(error_message);
UnrecoverableError(error_message);
}
}
}
Expand Down
Loading

0 comments on commit 6671d33

Please sign in to comment.