Skip to content
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

refactor(codegen): null safe for struct ir builder #3547

Merged
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
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 @@
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 @@

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 @@
}
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 @@
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
Loading