Skip to content

Commit 20708a2

Browse files
committed
fix: address CodeRabbit review findings in query_executor_tests
- execute_sql: Guard against null parse result to avoid dereferencing - Remove unused create_test_table helper function - SelectNonExistentColumn: Assert actual behavior (success with empty result) - DivisionByZero: Assert success and row_count to verify query execution
1 parent b6298bd commit 20708a2

1 file changed

Lines changed: 11 additions & 14 deletions

File tree

tests/query_executor_tests.cpp

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,14 @@ struct TestEnvironment {
6666
QueryResult execute_sql(QueryExecutor& exec, const char* sql) {
6767
auto lexer = std::make_unique<Lexer>(sql);
6868
auto stmt = Parser(std::move(lexer)).parse_statement();
69+
if (!stmt) {
70+
QueryResult res;
71+
res.set_error("Parse error: invalid SQL");
72+
return res;
73+
}
6974
return exec.execute(*stmt);
7075
}
7176

72-
// Helper to create a simple table
73-
void create_test_table(QueryExecutor& exec, const char* name, const char* schema_sql) {
74-
execute_sql(exec, schema_sql);
75-
}
76-
7777
class QueryExecutorTests : public ::testing::Test {
7878
protected:
7979
void SetUp() override {}
@@ -283,13 +283,10 @@ TEST_F(QueryExecutorTests, SelectNonExistentTable) {
283283
TEST_F(QueryExecutorTests, SelectNonExistentColumn) {
284284
TestEnvironment env;
285285
execute_sql(env.executor, "CREATE TABLE test_table (id INT)");
286-
// Note: Some implementations may succeed even with non-existent column
287-
// This test documents actual behavior
288286
const auto res = execute_sql(env.executor, "SELECT nonexistent FROM test_table");
289-
// Either success with empty/error is acceptable
290-
if (res.success()) {
291-
EXPECT_EQ(res.row_count(), 0U);
292-
}
287+
// Implementation returns success with empty result for non-existent column
288+
EXPECT_TRUE(res.success());
289+
EXPECT_EQ(res.row_count(), 0U);
293290
}
294291

295292
// ============= UPDATE Tests =============
@@ -457,10 +454,10 @@ TEST_F(QueryExecutorTests, DivisionByZero) {
457454
TestEnvironment env;
458455
execute_sql(env.executor, "CREATE TABLE test_table (id INT)");
459456
execute_sql(env.executor, "INSERT INTO test_table VALUES (1)");
460-
// Depending on implementation, division by zero may return error or special value
461457
const auto res = execute_sql(env.executor, "SELECT 10 / 0 FROM test_table");
462-
// Just verify the query executed (behavior depends on implementation)
463-
SUCCEED();
458+
// Division by zero succeeds with result (implementation-dependent behavior)
459+
EXPECT_TRUE(res.success());
460+
EXPECT_EQ(res.row_count(), 1U);
464461
}
465462

466463
} // namespace

0 commit comments

Comments
 (0)