Skip to content

Commit

Permalink
refactor(codegen): null safe for struct ir builder
Browse files Browse the repository at this point in the history
fixes #926
  • Loading branch information
aceforeverd committed Oct 23, 2023
1 parent 6fe2a30 commit 0583dff
Show file tree
Hide file tree
Showing 18 changed files with 197 additions and 173 deletions.
48 changes: 44 additions & 4 deletions cases/query/const_query.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,55 @@ cases:
columns: ['c1 bool', 'c2 int16', 'c3 int', 'c4 double', 'c5 string', 'c6 date', 'c7 timestamp' ]
rows:
- [ true, 3, 13, 10.0, 'a string', '2020-05-22', 1590115420000 ]

# =================================================================================
# Null safe for structure types: String, Date, Timestamp and Array
# creating struct from:
# 1. NULL liternal (const null)
# 2. another supported date type but fails to cast, e.g. timestamp(-1) returns NULL of timestamp
#
# casting to array type un-implemented
# =================================================================================
- id: 10
desc: null safe for date
mode: procedure-unsupport
sql: |
select
datediff(Date(timestamp(-1)), Date("2021-05-01")) as out1,
datediff(Date(timestamp(-2177481600)), Date("2021-05-01")) as out2,
datediff(cast(NULL as date), Date("2021-05-01")) as out3
;
datediff(cast(NULL as date), Date("2021-05-01")) as out3,
date(NULL) as out4,
date("abc") as out5,
date(timestamp("abc")) as out6
expect:
columns: ["out1 int", "out2 int", "out3 int", "out4 date", "out5 date", "out6 date"]
data: |
NULL, NULL, NULL, NULL, NULL, NULL
- id: 11
desc: null safe for timestamp
mode: procedure-unsupport
sql: |
select
month(cast(NULL as timestamp)) as out1,
month(timestamp(NULL)) as out2,
month(timestamp(-1)) as out3,
month(timestamp("abc")) as out4,
month(timestamp(date("abc"))) as out5
expect:
columns: ["out1 int", "out2 int", "out3 int", "out4 int", "out5 int"]
data: |
NULL, NULL, NULL, NULL, NULL
- id: 12
desc: null safe for string
mode: procedure-unsupport
sql: |
select
char_length(cast(NULL as string)) as out1,
char_length(string(int(NULL))) as out2,
char_length(string(bool(null))) as out3,
char_length(string(timestamp(null))) as out4,
char_length(string(date(null))) as out5
expect:
columns: ["out1 int", "out2 int", "out3 int"]
columns: ["out1 int", "out2 int", "out3 int", "out4 int", "out5 int"]
data: |
NULL, NULL, NULL
NULL, NULL, NULL, NULL, NULL
16 changes: 16 additions & 0 deletions hybridse/src/codegen/array_ir_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,21 @@ base::Status ArrayIRBuilder::NewEmptyArray(llvm::BasicBlock* bb, NativeValue* ou
return base::Status::OK();
}

bool ArrayIRBuilder::CreateDefault(::llvm::BasicBlock* block, ::llvm::Value** output) {
llvm::Value* array_alloca = nullptr;
if (!Create(block, &array_alloca)) {
return false;

Check warning on line 120 in hybridse/src/codegen/array_ir_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/array_ir_builder.cc#L117-L120

Added lines #L117 - L120 were not covered by tests
}

llvm::IRBuilder<> builder(block);
::llvm::Value* array_sz = builder.getInt64(0);
if (!Set(block, array_alloca, 2, array_sz)) {
return false;

Check warning on line 126 in hybridse/src/codegen/array_ir_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/array_ir_builder.cc#L123-L126

Added lines #L123 - L126 were not covered by tests
}

*output = array_alloca;
return true;

Check warning on line 130 in hybridse/src/codegen/array_ir_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/array_ir_builder.cc#L129-L130

Added lines #L129 - L130 were not covered by tests
}

} // namespace codegen
} // namespace hybridse
4 changes: 2 additions & 2 deletions hybridse/src/codegen/array_ir_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ class ArrayIRBuilder : public StructTypeIRBuilder {

void InitStructType() override;

bool CreateDefault(::llvm::BasicBlock* block, ::llvm::Value** output) override { return true; }
bool CreateDefault(::llvm::BasicBlock* block, ::llvm::Value** output) override;

bool CopyFrom(::llvm::BasicBlock* block, ::llvm::Value* src, ::llvm::Value* dist) override { return true; }

base::Status CastFrom(::llvm::BasicBlock* block, const NativeValue& src, NativeValue* output) override {
return base::Status::OK();
CHECK_TRUE(false, common::kCodegenError, "casting to array un-implemented");

Check warning on line 57 in hybridse/src/codegen/array_ir_builder.h

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/array_ir_builder.h#L57

Added line #L57 was not covered by tests
};

private:
Expand Down
86 changes: 32 additions & 54 deletions hybridse/src/codegen/cast_expr_ir_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
*/

#include "codegen/cast_expr_ir_builder.h"

#include "codegen/date_ir_builder.h"
#include "codegen/ir_base_builder.h"
#include "codegen/string_ir_builder.h"
#include "codegen/timestamp_ir_builder.h"
#include "codegen/type_ir_builder.h"
#include "glog/logging.h"
#include "node/node_manager.h"
#include "proto/fe_common.pb.h"

using hybridse::common::kCodegenError;

Expand Down Expand Up @@ -72,98 +75,73 @@ Status CastExprIRBuilder::Cast(const NativeValue& value,
}
return Status::OK();
}
Status CastExprIRBuilder::SafeCast(const NativeValue& value, ::llvm::Type* type,
NativeValue* output) {

Status CastExprIRBuilder::SafeCast(const NativeValue& value, ::llvm::Type* dst_type, NativeValue* output) {
::llvm::IRBuilder<> builder(block_);
CHECK_TRUE(IsSafeCast(value.GetType(), type), kCodegenError,
"Safe cast fail: unsafe cast");
CHECK_TRUE(IsSafeCast(value.GetType(), dst_type), kCodegenError, "Safe cast fail: unsafe cast");
Status status;
if (value.IsConstNull()) {
if (TypeIRBuilder::IsStringPtr(type)) {
StringIRBuilder string_ir_builder(block_->getModule());
CHECK_STATUS(string_ir_builder.CreateNull(block_, output));
return base::Status::OK();
} else {
*output = NativeValue::CreateNull(type);
}
} else if (TypeIRBuilder::IsTimestampPtr(type)) {
auto res = CreateSafeNull(block_, dst_type);
CHECK_TRUE(res.ok(), kCodegenError, res.status().ToString());
*output = res.value();

Check warning on line 86 in hybridse/src/codegen/cast_expr_ir_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/cast_expr_ir_builder.cc#L84-L86

Added lines #L84 - L86 were not covered by tests
} else if (TypeIRBuilder::IsTimestampPtr(dst_type)) {
TimestampIRBuilder timestamp_ir_builder(block_->getModule());
CHECK_STATUS(timestamp_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsDatePtr(type)) {
} else if (TypeIRBuilder::IsDatePtr(dst_type)) {
DateIRBuilder date_ir_builder(block_->getModule());
CHECK_STATUS(date_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsStringPtr(type)) {
} else if (TypeIRBuilder::IsStringPtr(dst_type)) {
StringIRBuilder string_ir_builder(block_->getModule());
CHECK_STATUS(string_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsNumber(type)) {
} else if (TypeIRBuilder::IsNumber(dst_type)) {
Status status;
::llvm::Value* output_value = nullptr;
CHECK_TRUE(SafeCastNumber(value.GetValue(&builder), type, &output_value,
status),
kCodegenError);
CHECK_TRUE(SafeCastNumber(value.GetValue(&builder), dst_type, &output_value, status), kCodegenError);
if (value.IsNullable()) {
*output = NativeValue::CreateWithFlag(output_value,
value.GetIsNull(&builder));
*output = NativeValue::CreateWithFlag(output_value, value.GetIsNull(&builder));
} else {
*output = NativeValue::Create(output_value);
}
} else {
return Status(common::kCodegenError,
"Can't cast from " +
TypeIRBuilder::TypeName(value.GetType()) + " to " +
TypeIRBuilder::TypeName(type));
return Status(common::kCodegenError, "Can't cast from " + TypeIRBuilder::TypeName(value.GetType()) + " to " +
TypeIRBuilder::TypeName(dst_type));

Check warning on line 110 in hybridse/src/codegen/cast_expr_ir_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/cast_expr_ir_builder.cc#L109-L110

Added lines #L109 - L110 were not covered by tests
}
return Status::OK();
}
Status CastExprIRBuilder::UnSafeCast(const NativeValue& value,
::llvm::Type* type, NativeValue* output) {
::llvm::IRBuilder<> builder(block_);
if (value.IsConstNull()) {
if (TypeIRBuilder::IsStringPtr(type)) {
StringIRBuilder string_ir_builder(block_->getModule());
CHECK_STATUS(string_ir_builder.CreateNull(block_, output));
return base::Status::OK();

} else if (TypeIRBuilder::IsDatePtr(type)) {
DateIRBuilder date_ir(block_->getModule());
CHECK_STATUS(date_ir.CreateNull(block_, output));
return base::Status::OK();
} else {
*output = NativeValue::CreateNull(type);
}
} else if (TypeIRBuilder::IsTimestampPtr(type)) {
Status CastExprIRBuilder::UnSafeCast(const NativeValue& value, ::llvm::Type* dst_type, NativeValue* output) {
::llvm::IRBuilder<> builder(block_);
if (value.IsConstNull() || (TypeIRBuilder::IsNumber(dst_type) && TypeIRBuilder::IsDatePtr(value.GetType()))) {
// input is const null or (cast date to number)
auto res = CreateSafeNull(block_, dst_type);
CHECK_TRUE(res.ok(), kCodegenError, res.status().ToString());
*output = res.value();
} else if (TypeIRBuilder::IsTimestampPtr(dst_type)) {
TimestampIRBuilder timestamp_ir_builder(block_->getModule());
CHECK_STATUS(timestamp_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsDatePtr(type)) {
} else if (TypeIRBuilder::IsDatePtr(dst_type)) {
DateIRBuilder date_ir_builder(block_->getModule());
CHECK_STATUS(date_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsStringPtr(type)) {
} else if (TypeIRBuilder::IsStringPtr(dst_type)) {
StringIRBuilder string_ir_builder(block_->getModule());
CHECK_STATUS(string_ir_builder.CastFrom(block_, value, output));
return Status::OK();
} else if (TypeIRBuilder::IsNumber(type) &&
TypeIRBuilder::IsStringPtr(value.GetType())) {
} else if (TypeIRBuilder::IsNumber(dst_type) && TypeIRBuilder::IsStringPtr(value.GetType())) {
StringIRBuilder string_ir_builder(block_->getModule());
CHECK_STATUS(
string_ir_builder.CastToNumber(block_, value, type, output));
CHECK_STATUS(string_ir_builder.CastToNumber(block_, value, dst_type, output));
return Status::OK();
} else if (TypeIRBuilder::IsNumber(type) &&
TypeIRBuilder::IsDatePtr(value.GetType())) {
*output = NativeValue::CreateNull(type);
} else {
Status status;
::llvm::Value* output_value = nullptr;
CHECK_TRUE(UnSafeCastNumber(value.GetValue(&builder), type,
&output_value, status),
kCodegenError, status.msg);
CHECK_TRUE(UnSafeCastNumber(value.GetValue(&builder), dst_type, &output_value, status), kCodegenError,
status.msg);
if (value.IsNullable()) {
*output = NativeValue::CreateWithFlag(output_value,
value.GetIsNull(&builder));
*output = NativeValue::CreateWithFlag(output_value, value.GetIsNull(&builder));
} else {
*output = NativeValue::Create(output_value);
}
Expand Down
24 changes: 7 additions & 17 deletions hybridse/src/codegen/cast_expr_ir_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
#define HYBRIDSE_SRC_CODEGEN_CAST_EXPR_IR_BUILDER_H_
#include "base/fe_status.h"
#include "codegen/cond_select_ir_builder.h"
#include "codegen/scope_var.h"
#include "llvm/IR/IRBuilder.h"
#include "proto/fe_type.pb.h"

namespace hybridse {
namespace codegen {
Expand All @@ -32,26 +29,19 @@ class CastExprIRBuilder {
explicit CastExprIRBuilder(::llvm::BasicBlock* block);
~CastExprIRBuilder();

Status Cast(const NativeValue& value, ::llvm::Type* cast_type,
NativeValue* output); // NOLINT
Status SafeCast(const NativeValue& value, ::llvm::Type* type,
NativeValue* output); // NOLINT
Status UnSafeCast(const NativeValue& value, ::llvm::Type* type,
NativeValue* output); // NOLINT
Status Cast(const NativeValue& value, ::llvm::Type* cast_type, NativeValue* output);
Status SafeCast(const NativeValue& value, ::llvm::Type* dst_type, NativeValue* output);
Status UnSafeCast(const NativeValue& value, ::llvm::Type* dst_type, NativeValue* output);
static bool IsSafeCast(::llvm::Type* lhs, ::llvm::Type* rhs);
static Status InferNumberCastTypes(::llvm::Type* left_type,
::llvm::Type* right_type);
static Status InferNumberCastTypes(::llvm::Type* left_type, ::llvm::Type* right_type);
static bool IsIntFloat2PointerCast(::llvm::Type* src, ::llvm::Type* dist);
bool BoolCast(llvm::Value* pValue, llvm::Value** pValue1,
base::Status& status); // NOLINT
bool SafeCastNumber(::llvm::Value* value, ::llvm::Type* type,
::llvm::Value** output,
bool SafeCastNumber(::llvm::Value* value, ::llvm::Type* type, ::llvm::Value** output,
base::Status& status); // NOLINT
bool UnSafeCastNumber(::llvm::Value* value, ::llvm::Type* type,
::llvm::Value** output,
bool UnSafeCastNumber(::llvm::Value* value, ::llvm::Type* type, ::llvm::Value** output,
base::Status& status); // NOLINT
bool UnSafeCastDouble(::llvm::Value* value, ::llvm::Type* type,
::llvm::Value** output,
bool UnSafeCastDouble(::llvm::Value* value, ::llvm::Type* type, ::llvm::Value** output,
base::Status& status); // NOLINT

private:
Expand Down
8 changes: 0 additions & 8 deletions hybridse/src/codegen/date_ir_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ void DateIRBuilder::InitStructType() {
return;
}

base::Status DateIRBuilder::CreateNull(::llvm::BasicBlock* block, NativeValue* output) {
::llvm::Value* value = nullptr;
CHECK_TRUE(CreateDefault(block, &value), common::kCodegenError, "Fail to construct string")
::llvm::IRBuilder<> builder(block);
*output = NativeValue::CreateWithFlag(value, builder.getInt1(true));
return base::Status::OK();
}

bool DateIRBuilder::CreateDefault(::llvm::BasicBlock* block,
::llvm::Value** output) {
return NewDate(block, output);
Expand Down
10 changes: 4 additions & 6 deletions hybridse/src/codegen/date_ir_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ class DateIRBuilder : public StructTypeIRBuilder {
public:
explicit DateIRBuilder(::llvm::Module* m);
~DateIRBuilder();
void InitStructType() override;

base::Status CreateNull(::llvm::BasicBlock* block, NativeValue* output);
void InitStructType() override;
bool CreateDefault(::llvm::BasicBlock* block, ::llvm::Value** output) override;
bool CopyFrom(::llvm::BasicBlock* block, ::llvm::Value* src, ::llvm::Value* dist) override;
base::Status CastFrom(::llvm::BasicBlock* block, const NativeValue& src, NativeValue* output) override;

bool NewDate(::llvm::BasicBlock* block, ::llvm::Value** output);
bool NewDate(::llvm::BasicBlock* block, ::llvm::Value* date,
::llvm::Value** output);
bool CopyFrom(::llvm::BasicBlock* block, ::llvm::Value* src, ::llvm::Value* dist);
base::Status CastFrom(::llvm::BasicBlock* block, const NativeValue& src, NativeValue* output);
bool NewDate(::llvm::BasicBlock* block, ::llvm::Value* date, ::llvm::Value** output);

bool GetDate(::llvm::BasicBlock* block, ::llvm::Value* date,
::llvm::Value** output);
Expand Down
5 changes: 1 addition & 4 deletions hybridse/src/codegen/expr_ir_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@
#include "codegen/cond_select_ir_builder.h"
#include "codegen/context.h"
#include "codegen/date_ir_builder.h"
#include "codegen/fn_ir_builder.h"
#include "codegen/ir_base_builder.h"
#include "codegen/list_ir_builder.h"
#include "codegen/struct_ir_builder.h"
#include "codegen/timestamp_ir_builder.h"
#include "codegen/type_ir_builder.h"
#include "codegen/udf_ir_builder.h"
Expand Down Expand Up @@ -217,8 +215,7 @@ Status ExprIRBuilder::BuildConstExpr(
::llvm::IRBuilder<> builder(ctx_->GetCurrentBlock());
switch (const_node->GetDataType()) {
case ::hybridse::node::kNull: {
*output = NativeValue::CreateNull(
llvm::Type::getTokenTy(builder.getContext()));
*output = NativeValue(nullptr, nullptr, llvm::Type::getTokenTy(builder.getContext()));
break;
}
case ::hybridse::node::kBool: {
Expand Down
15 changes: 9 additions & 6 deletions hybridse/src/codegen/ir_base_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "codegen/ir_base_builder.h"

#include <string>
#include <tuple>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -625,21 +624,25 @@ bool GetBaseType(::llvm::Type* type, ::hybridse::node::DataType* output) {
return false;
}

if (pointee_ty->getStructName().startswith("fe.list_ref_")) {
auto struct_name = pointee_ty->getStructName();
if (struct_name.startswith("fe.list_ref_")) {
*output = hybridse::node::kList;
return true;
} else if (pointee_ty->getStructName().startswith("fe.iterator_ref_")) {
} else if (struct_name.startswith("fe.iterator_ref_")) {
*output = hybridse::node::kIterator;
return true;
} else if (pointee_ty->getStructName().equals("fe.string_ref")) {
} else if (struct_name.equals("fe.string_ref")) {
*output = hybridse::node::kVarchar;
return true;
} else if (pointee_ty->getStructName().equals("fe.timestamp")) {
} else if (struct_name.equals("fe.timestamp")) {
*output = hybridse::node::kTimestamp;
return true;
} else if (pointee_ty->getStructName().equals("fe.date")) {
} else if (struct_name.equals("fe.date")) {
*output = hybridse::node::kDate;
return true;
} else if (struct_name.startswith("fe.array_")) {
*output = hybridse::node::kArray;
return true;

Check warning on line 645 in hybridse/src/codegen/ir_base_builder.cc

View check run for this annotation

Codecov / codecov/patch

hybridse/src/codegen/ir_base_builder.cc#L643-L645

Added lines #L643 - L645 were not covered by tests
}
LOG(WARNING) << "no mapping pointee_ty for llvm pointee_ty "
<< pointee_ty->getStructName().str();
Expand Down
Loading

0 comments on commit 0583dff

Please sign in to comment.