diff options
author | Jan de Visser <jan@de-visser.net> | 2021-10-23 10:46:33 -0400 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-10-25 12:59:42 +0200 |
commit | 9022cf99ffdff94f8a20cb1ed65f970449179410 (patch) | |
tree | d9886997725243540a85894399308ad2720a872c /Userland | |
parent | 0cfb5eec327f14be49e29b18e820d28ee10874fa (diff) | |
download | serenity-9022cf99ffdff94f8a20cb1ed65f970449179410.zip |
LibSQL: Add better error handling to `evaluate` and `execute` methods
There was a lot of `VERIFY_NOT_REACHED` error handling going on. Fixed
most of those.
A bit of a caveat is that after every `evaluate` call for expressions
that are part of a statement the error status of the `SQLResult` return
value must be called.
Diffstat (limited to 'Userland')
-rw-r--r-- | Userland/Libraries/LibSQL/AST/AST.h | 84 | ||||
-rw-r--r-- | Userland/Libraries/LibSQL/AST/Expression.cpp | 47 | ||||
-rw-r--r-- | Userland/Libraries/LibSQL/AST/Insert.cpp | 4 | ||||
-rw-r--r-- | Userland/Libraries/LibSQL/AST/Select.cpp | 4 | ||||
-rw-r--r-- | Userland/Libraries/LibSQL/SQLResult.h | 41 |
5 files changed, 125 insertions, 55 deletions
diff --git a/Userland/Libraries/LibSQL/AST/AST.h b/Userland/Libraries/LibSQL/AST/AST.h index 06f2f2fdbc..ca2c50946b 100644 --- a/Userland/Libraries/LibSQL/AST/AST.h +++ b/Userland/Libraries/LibSQL/AST/AST.h @@ -437,13 +437,32 @@ private: String m_column_name; }; +#define __enum_UnaryOperator(S) \ + S(Minus, "-") \ + S(Plus, "+") \ + S(BitwiseNot, "~") \ + S(Not, "NOT") + enum class UnaryOperator { - Minus, - Plus, - BitwiseNot, - Not, +#undef __UnaryOperator +#define __UnaryOperator(code, name) code, + __enum_UnaryOperator(__UnaryOperator) +#undef __UnaryOperator }; +constexpr char const* UnaryOperator_name(UnaryOperator op) +{ + switch (op) { +#undef __UnaryOperator +#define __UnaryOperator(code, name) \ + case UnaryOperator::code: \ + return name; + __enum_UnaryOperator(__UnaryOperator) +#undef __UnaryOperator + default : VERIFY_NOT_REACHED(); + } +} + class UnaryOperatorExpression : public NestedExpression { public: UnaryOperatorExpression(UnaryOperator type, NonnullRefPtr<Expression> expression) @@ -459,28 +478,47 @@ private: UnaryOperator m_type; }; +// Note: These are in order of highest-to-lowest operator precedence. +#define __enum_BinaryOperator(S) \ + S(Concatenate, "||") \ + S(Multiplication, "*") \ + S(Division, "/") \ + S(Modulo, "%") \ + S(Plus, "+") \ + S(Minus, "-") \ + S(ShiftLeft, "<<") \ + S(ShiftRight, ">>") \ + S(BitwiseAnd, "&") \ + S(BitwiseOr, "|") \ + S(LessThan, "<") \ + S(LessThanEquals, "<=") \ + S(GreaterThan, ">") \ + S(GreaterThanEquals, ">=") \ + S(Equals, "=") \ + S(NotEquals, "!=") \ + S(And, "and") \ + S(Or, "or") + enum class BinaryOperator { - // Note: These are in order of highest-to-lowest operator precedence. - Concatenate, - Multiplication, - Division, - Modulo, - Plus, - Minus, - ShiftLeft, - ShiftRight, - BitwiseAnd, - BitwiseOr, - LessThan, - LessThanEquals, - GreaterThan, - GreaterThanEquals, - Equals, - NotEquals, - And, - Or, +#undef __BinaryOperator +#define __BinaryOperator(code, name) code, + __enum_BinaryOperator(__BinaryOperator) +#undef __BinaryOperator }; +constexpr char const* BinaryOperator_name(BinaryOperator op) +{ + switch (op) { +#undef __BinaryOperator +#define __BinaryOperator(code, name) \ + case BinaryOperator::code: \ + return name; + __enum_BinaryOperator(__BinaryOperator) +#undef __BinaryOperator + default : VERIFY_NOT_REACHED(); + } +} + class BinaryOperatorExpression : public NestedDoubleExpression { public: BinaryOperatorExpression(BinaryOperator type, NonnullRefPtr<Expression> lhs, NonnullRefPtr<Expression> rhs) diff --git a/Userland/Libraries/LibSQL/AST/Expression.cpp b/Userland/Libraries/LibSQL/AST/Expression.cpp index cd53fa66f0..d3e1337be2 100644 --- a/Userland/Libraries/LibSQL/AST/Expression.cpp +++ b/Userland/Libraries/LibSQL/AST/Expression.cpp @@ -14,15 +14,19 @@ Value Expression::evaluate(ExecutionContext&) const return Value::null(); } -Value NumericLiteral::evaluate(ExecutionContext&) const +Value NumericLiteral::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); Value ret(SQLType::Float); ret = value(); return ret; } -Value StringLiteral::evaluate(ExecutionContext&) const +Value StringLiteral::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); Value ret(SQLType::Text); ret = value(); return ret; @@ -35,11 +39,15 @@ Value NullLiteral::evaluate(ExecutionContext&) const Value NestedExpression::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); return expression()->evaluate(context); } Value ChainedExpression::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); Value ret(SQLType::Tuple); Vector<Value> values; for (auto& expression : expressions()) { @@ -51,6 +59,8 @@ Value ChainedExpression::evaluate(ExecutionContext& context) const Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); Value lhs_value = lhs()->evaluate(context); Value rhs_value = rhs()->evaluate(context); switch (type()) { @@ -97,8 +107,8 @@ Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const auto lhs_bool_maybe = lhs_value.to_bool(); auto rhs_bool_maybe = rhs_value.to_bool(); if (!lhs_bool_maybe.has_value() || !rhs_bool_maybe.has_value()) { - // TODO Error handling - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, BinaryOperator_name(type())); + return Value::null(); } return Value(lhs_bool_maybe.value() && rhs_bool_maybe.value()); } @@ -106,24 +116,27 @@ Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const auto lhs_bool_maybe = lhs_value.to_bool(); auto rhs_bool_maybe = rhs_value.to_bool(); if (!lhs_bool_maybe.has_value() || !rhs_bool_maybe.has_value()) { - // TODO Error handling - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, BinaryOperator_name(type())); + return Value::null(); } return Value(lhs_bool_maybe.value() || rhs_bool_maybe.value()); } + default: + VERIFY_NOT_REACHED(); } - VERIFY_NOT_REACHED(); } Value UnaryOperatorExpression::evaluate(ExecutionContext& context) const { + if (context.result->has_error()) + return Value::null(); Value expression_value = NestedExpression::evaluate(context); switch (type()) { case UnaryOperator::Plus: if (expression_value.type() == SQLType::Integer || expression_value.type() == SQLType::Float) return expression_value; - // TODO: Error handling. - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::NumericOperatorTypeMismatch, UnaryOperator_name(type())); + return Value::null(); case UnaryOperator::Minus: if (expression_value.type() == SQLType::Integer) { expression_value = -int(expression_value); @@ -133,22 +146,22 @@ Value UnaryOperatorExpression::evaluate(ExecutionContext& context) const expression_value = -double(expression_value); return expression_value; } - // TODO: Error handling. - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::NumericOperatorTypeMismatch, UnaryOperator_name(type())); + return Value::null(); case UnaryOperator::Not: if (expression_value.type() == SQLType::Boolean) { expression_value = !bool(expression_value); return expression_value; } - // TODO: Error handling. - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, UnaryOperator_name(type())); + return Value::null(); case UnaryOperator::BitwiseNot: if (expression_value.type() == SQLType::Integer) { expression_value = ~u32(expression_value); return expression_value; } - // TODO: Error handling. - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::IntegerOperatorTypeMismatch, UnaryOperator_name(type())); + return Value::null(); } VERIFY_NOT_REACHED(); } @@ -162,8 +175,8 @@ Value ColumnNameExpression::evaluate(ExecutionContext& context) const if (column_descriptor.name == column_name()) return { (*context.current_row)[ix] }; } - // TODO: Error handling. - VERIFY_NOT_REACHED(); + context.result->set_error(SQLErrorCode::ColumnDoesNotExist, column_name()); + return Value::null(); } } diff --git a/Userland/Libraries/LibSQL/AST/Insert.cpp b/Userland/Libraries/LibSQL/AST/Insert.cpp index 2b1fe3d72a..5e1c39a91b 100644 --- a/Userland/Libraries/LibSQL/AST/Insert.cpp +++ b/Userland/Libraries/LibSQL/AST/Insert.cpp @@ -44,6 +44,7 @@ RefPtr<SQLResult> Insert::execute(ExecutionContext& context) const Vector<Row> inserted_rows; inserted_rows.ensure_capacity(m_chained_expressions.size()); + context.result = SQLResult::construct(); for (auto& row_expr : m_chained_expressions) { for (auto& column_def : table_def->columns()) { if (!m_column_names.contains_slow(column_def.name())) { @@ -51,6 +52,8 @@ RefPtr<SQLResult> Insert::execute(ExecutionContext& context) const } } auto row_value = row_expr.evaluate(context); + if (context.result->has_error()) + return context.result; VERIFY(row_value.type() == SQLType::Tuple); auto values = row_value.to_vector().value(); @@ -76,6 +79,7 @@ RefPtr<SQLResult> Insert::execute(ExecutionContext& context) const for (auto& inserted_row : inserted_rows) { context.database->insert(inserted_row); + // FIXME Error handling } return SQLResult::construct(SQLCommand::Insert, 0, m_chained_expressions.size(), 0); diff --git a/Userland/Libraries/LibSQL/AST/Select.cpp b/Userland/Libraries/LibSQL/AST/Select.cpp index 56d7d0be26..6214e67015 100644 --- a/Userland/Libraries/LibSQL/AST/Select.cpp +++ b/Userland/Libraries/LibSQL/AST/Select.cpp @@ -39,12 +39,16 @@ RefPtr<SQLResult> Select::execute(ExecutionContext& context) const context.current_row = &row; if (where_clause()) { auto where_result = where_clause()->evaluate(context); + if (context.result->has_error()) + return context.result; if (!where_result) continue; } tuple.clear(); for (auto& col : columns) { auto value = col.expression()->evaluate(context); + if (context.result->has_error()) + return context.result; tuple.append(value); } context.result->append(tuple); diff --git a/Userland/Libraries/LibSQL/SQLResult.h b/Userland/Libraries/LibSQL/SQLResult.h index 9702bc1eb9..33eab92c2a 100644 --- a/Userland/Libraries/LibSQL/SQLResult.h +++ b/Userland/Libraries/LibSQL/SQLResult.h @@ -41,21 +41,25 @@ constexpr char const* command_tag(SQLCommand command) } } -#define ENUMERATE_SQL_ERRORS(S) \ - S(NoError, "No error") \ - S(DatabaseUnavailable, "Database Unavailable") \ - S(StatementUnavailable, "Statement with id '{}' Unavailable") \ - S(SyntaxError, "Syntax Error") \ - S(DatabaseDoesNotExist, "Database '{}' does not exist") \ - S(SchemaDoesNotExist, "Schema '{}' does not exist") \ - S(SchemaExists, "Schema '{}' already exist") \ - S(TableDoesNotExist, "Table '{}' does not exist") \ - S(ColumnDoesNotExist, "Column '{}' does not exist") \ - S(TableExists, "Table '{}' already exist") \ - S(InvalidType, "Invalid type '{}'") \ - S(InvalidDatabaseName, "Invalid database name '{}'") \ - S(InvalidValueType, "Invalid type for attribute '{}'") \ - S(InvalidNumberOfValues, "Number of values does not match number of columns") +#define ENUMERATE_SQL_ERRORS(S) \ + S(NoError, "No error") \ + S(DatabaseUnavailable, "Database Unavailable") \ + S(StatementUnavailable, "Statement with id '{}' Unavailable") \ + S(SyntaxError, "Syntax Error") \ + S(DatabaseDoesNotExist, "Database '{}' does not exist") \ + S(SchemaDoesNotExist, "Schema '{}' does not exist") \ + S(SchemaExists, "Schema '{}' already exist") \ + S(TableDoesNotExist, "Table '{}' does not exist") \ + S(ColumnDoesNotExist, "Column '{}' does not exist") \ + S(TableExists, "Table '{}' already exist") \ + S(InvalidType, "Invalid type '{}'") \ + S(InvalidDatabaseName, "Invalid database name '{}'") \ + S(InvalidValueType, "Invalid type for attribute '{}'") \ + S(InvalidNumberOfValues, "Number of values does not match number of columns") \ + S(BooleanOperatorTypeMismatch, "Cannot apply '{}' operator to non-boolean operands") \ + S(NumericOperatorTypeMismatch, "Cannot apply '{}' operator to non-numeric operands") \ + S(IntegerOperatorTypeMismatch, "Cannot apply '{}' operator to non-numeric operands") \ + S(InvalidOperator, "Invalid operator '{}'") enum class SQLErrorCode { #undef __ENUMERATE_SQL_ERROR @@ -113,6 +117,13 @@ public: int updated() const { return m_update_count; } int inserted() const { return m_insert_count; } int deleted() const { return m_delete_count; } + void set_error(SQLErrorCode code, String argument = {}) + { + m_error.code = code; + m_error.error_argument = argument; + } + + bool has_error() const { return m_error.code != SQLErrorCode::NoError; } SQLError const& error() const { return m_error; } bool has_results() const { return m_has_results; } Vector<Tuple> const& results() const { return m_result_set; } |