Skip to content

Commit

Permalink
fix: avoid holding dangling pointer of scalar expressions.
Browse files Browse the repository at this point in the history
  • Loading branch information
ashigeru committed Jul 8, 2024
1 parent 8a6a746 commit bd6b9bc
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 19 deletions.
25 changes: 14 additions & 11 deletions src/mizugaki/analyzer/details/analyze_query_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ class engine {
variable.region() = result.value().region();
auto direction = convert(spec.direction());
if (!direction) {
context_.clear_expression_resolution(result.value());
return {};
}
project.columns().emplace_back(variable, result.release());
Expand Down Expand Up @@ -275,7 +276,7 @@ class engine {
if (!nrows_expr) {
return {};
}
auto nrows = extract_size(*nrows_expr);
auto nrows = extract_size(nrows_expr.release());
if (!nrows) {
return {};
}
Expand Down Expand Up @@ -337,9 +338,10 @@ class engine {
return { *output, std::move(info) };
}

static optional_ptr<tdescriptor::variable> extract_variable(tscalar::expression& expr) {
if (expr.kind() == tscalar::variable_reference::tag) {
return unsafe_downcast<tscalar::variable_reference>(expr).variable();
[[nodiscard]] std::optional<tdescriptor::variable> extract_variable(std::unique_ptr<tscalar::expression> expr) {
context_.clear_expression_resolution(*expr);
if (expr->kind() == tscalar::variable_reference::tag) {
return std::move(unsafe_downcast<tscalar::variable_reference>(*expr).variable());
}
return {};
}
Expand Down Expand Up @@ -725,7 +727,7 @@ class engine {
elem.region());
return false;
}
auto variable = extract_variable(r.value());
auto variable = extract_variable(r.release());
if (!variable) {
context_.report(
sql_analyzer_code::unsupported_feature,
Expand Down Expand Up @@ -937,17 +939,18 @@ class engine {
return true;
}

[[nodiscard]] std::optional<std::size_t> extract_size(tscalar::expression const& expr) {
if (expr.kind() == tscalar::immediate::tag) {
auto&& immediate = unsafe_downcast<tscalar::immediate>(expr);
[[nodiscard]] std::optional<std::size_t> extract_size(std::unique_ptr<tscalar::expression> expr) {
context_.clear_expression_resolution(*expr);
if (expr->kind() == tscalar::immediate::tag) {
auto&& immediate = unsafe_downcast<tscalar::immediate>(*expr);
auto value = extract_int_value(immediate.value());
if (!value || value < 0) {
context_.report(
sql_analyzer_code::invalid_unsigned_integer,
string_builder {}
<< "not a valid unsigned integer: " << immediate.value()
<< string_builder::to_string,
expr.region());
expr->region());
return {};
}
return static_cast<std::size_t>(*value);
Expand All @@ -956,9 +959,9 @@ class engine {
sql_analyzer_code::invalid_unsigned_integer,
string_builder {}
<< "must be a unsigned integer: "
<< expr.kind()
<< expr->kind()
<< string_builder::to_string,
expr.region());
expr->region());
return {};
}

Expand Down
16 changes: 9 additions & 7 deletions src/mizugaki/analyzer/details/analyze_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,8 +875,7 @@ class engine {
if (!result) {
return {}; // error
}
auto expr = result.release();
auto value = extract_column_value(*expr);
auto value = extract_column_value(result.release());
if (!value) {
context_.report(
sql_analyzer_code::unsupported_feature,
Expand All @@ -886,19 +885,21 @@ class engine {
return value;
}

[[nodiscard]] std::optional<::yugawara::storage::column_value> extract_column_value(tscalar::expression const& expr) {
[[nodiscard]] std::optional<::yugawara::storage::column_value> extract_column_value(
std::unique_ptr<tscalar::expression> expr) {
context_.clear_expression_resolution(*expr);
using kind = tscalar::expression_kind;
switch (expr.kind()) {
switch (expr->kind()) {
case kind::immediate:
{
auto&& value = unsafe_downcast<tscalar::immediate>(expr);
auto&& value = unsafe_downcast<tscalar::immediate>(*expr);
return ::yugawara::storage::column_value { context_.values().get(value.value()) };
}
case kind::cast:
{
auto&& e = unsafe_downcast<tscalar::cast>(expr);
auto&& e = unsafe_downcast<tscalar::cast>(*expr);
// FIXME: impl casting
return extract_column_value(e.operand());
return extract_column_value(e.release_operand());
}
default:
// FIXME: more types for column default values
Expand Down Expand Up @@ -968,6 +969,7 @@ class engine {
return {};
}
auto expr = result.release();
context_.clear_expression_resolution(*expr);
if (expr->kind() != tscalar::expression_kind::variable_reference) {
context_.report(
sql_analyzer_code::unsupported_feature,
Expand Down
10 changes: 9 additions & 1 deletion src/mizugaki/analyzer/details/analyzer_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void analyzer_context::finalize() {
}

std::shared_ptr<::takatori::type::data const>
analyzer_context::resolve(takatori::scalar::expression const& expression, bool validate) {
analyzer_context::resolve(::takatori::scalar::expression const& expression, bool validate) {
auto result = expression_analyzer_.resolve(expression, validate, types_);
if (expression_analyzer_.has_diagnostics()) {
for (auto&& d : expression_analyzer_.diagnostics()) {
Expand Down Expand Up @@ -99,6 +99,14 @@ bool analyzer_context::resolve(::takatori::relation::expression const& expressio
return true;
}

void analyzer_context::clear_expression_resolution() {
expression_analyzer_.expressions().clear();
}

void analyzer_context::clear_expression_resolution(::takatori::scalar::expression const& expression) {
expression_analyzer_.expressions().unbind(expression);
}

void analyzer_context::resolve_as(
::takatori::descriptor::variable const& variable,
::yugawara::analyzer::variable_resolution resolution) {
Expand Down
4 changes: 4 additions & 0 deletions src/mizugaki/analyzer/details/analyzer_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ class analyzer_context {
::takatori::descriptor::variable const& variable,
::yugawara::analyzer::variable_resolution resolution);

void clear_expression_resolution();

void clear_expression_resolution(::takatori::scalar::expression const& expression);

[[nodiscard]] ::takatori::document::region convert(ast::node_region region) const;

template<class T>
Expand Down
1 change: 1 addition & 0 deletions src/mizugaki/analyzer/details/set_function_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ optional_ptr<output_port_type> set_function_processor::install(output_port_type&
void set_function_processor::consume(ownership_reference<tscalar::expression> expression) {
activate();
auto&& invocation = downcast<::yugawara::extension::scalar::aggregate_function_call>(expression.get());
context_.clear_expression_resolution(invocation);

std::vector<tdescriptor::variable> arguments {};
arguments.reserve(invocation.arguments().size());
Expand Down

0 comments on commit bd6b9bc

Please sign in to comment.