Skip to content

Commit

Permalink
Capture read only db message in query result (#2508)
Browse files Browse the repository at this point in the history
* Capture read only db message in query result

* Fix rust test for new read-only query message

* fix ASAN

---------

Co-authored-by: Benjamin Winger <bmw@disroot.org>
  • Loading branch information
andyfengHKU and benjaminwinger committed Nov 29, 2023
1 parent 4b5656a commit 2504cd4
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 19 deletions.
2 changes: 0 additions & 2 deletions src/include/main/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ class Connection {

KUZU_API void addScalarFunction(std::string name, function::function_set definitions);

void checkPreparedStatementAccessMode(PreparedStatement* preparedStatement);

private:
Database* database;
std::unique_ptr<ClientContext> clientContext;
Expand Down
10 changes: 3 additions & 7 deletions src/main/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ std::unique_ptr<PreparedStatement> Connection::prepareNoLock(
// parsing
auto statement = Parser::parseQuery(query);
preparedStatement->readOnly = parser::StatementReadWriteAnalyzer().isReadOnly(*statement);
if (database->systemConfig.readOnly && !preparedStatement->isReadOnly()) {
throw ConnectionException("Cannot execute write operations in a read-only database!");
}
// binding
auto binder = Binder(*database->catalog, database->memoryManager.get(),
database->storageManager.get(), clientContext.get());
Expand Down Expand Up @@ -197,7 +200,6 @@ void Connection::bindParametersNoLock(PreparedStatement* preparedStatement,

std::unique_ptr<QueryResult> Connection::executeAndAutoCommitIfNecessaryNoLock(
PreparedStatement* preparedStatement, uint32_t planIdx) {
checkPreparedStatementAccessMode(preparedStatement);
clientContext->resetActiveQuery();
clientContext->startTimingIfEnabled();
auto mapper = PlanMapper(
Expand Down Expand Up @@ -259,11 +261,5 @@ void Connection::addScalarFunction(std::string name, function::function_set defi
database->catalog->addFunction(std::move(name), std::move(definitions));
}

void Connection::checkPreparedStatementAccessMode(PreparedStatement* preparedStatement) {
if (database->systemConfig.readOnly && !preparedStatement->isReadOnly()) {
throw ConnectionException("Cannot execute write operations in a read-only database!");
}
}

} // namespace main
} // namespace kuzu
3 changes: 2 additions & 1 deletion test/c_api/database_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ TEST_F(CApiDatabaseTest, CreationReadOnly) {
ASSERT_NE(connection, nullptr);
auto queryResult = kuzu_connection_query(connection,
"CREATE NODE TABLE User(name STRING, age INT64, reg_date DATE, PRIMARY KEY (name))");
ASSERT_EQ(queryResult, nullptr);
ASSERT_FALSE(kuzu_query_result_is_success(queryResult));
kuzu_query_result_destroy(queryResult);
kuzu_connection_destroy(connection);
kuzu_database_destroy(database);
}
Expand Down
3 changes: 2 additions & 1 deletion test/main/access_mode_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ using namespace kuzu::main;
class AccessModeTest : public ApiTest {};

void assertQuery(QueryResult& result) {
auto a = result.toString();
ASSERT_TRUE(result.isSuccess()) << result.toString();
}

Expand All @@ -23,6 +24,6 @@ TEST_F(AccessModeTest, testAccessMode) {
std::unique_ptr<Connection> con2;
EXPECT_NO_THROW(db2 = std::make_unique<Database>(databasePath, *systemConfig));
EXPECT_NO_THROW(con2 = std::make_unique<Connection>(db2.get()));
EXPECT_ANY_THROW(con2->query("DROP TABLE Person"));
ASSERT_FALSE(con2->query("DROP TABLE Person")->isSuccess());
EXPECT_NO_THROW(con2->query("MATCH (:Person) RETURN COUNT(*)"));
}
13 changes: 7 additions & 6 deletions test/main/db_locking_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,12 @@ TEST_F(DBLockingTest, testReadOnly) {
// we can query the database
ASSERT_TRUE(conn->query("MATCH (:Person) RETURN COUNT(*)")->isSuccess());
// however, we can't perform DDL statements
EXPECT_ANY_THROW(conn->query("CREATE NODE TABLE university(ID INT64, PRIMARY KEY(ID))"));
EXPECT_ANY_THROW(conn->query("ALTER TABLE Peron DROP name"));
EXPECT_ANY_THROW(conn->query("DROP TABLE Peron"));
ASSERT_FALSE(
conn->query("CREATE NODE TABLE university(ID INT64, PRIMARY KEY(ID))")->isSuccess());
ASSERT_FALSE(conn->query("ALTER TABLE Peron DROP name")->isSuccess());
ASSERT_FALSE(conn->query("DROP TABLE Peron")->isSuccess());
// neither can we insert/update/delete data
EXPECT_ANY_THROW(conn->query("CREATE (:Person {name: 'Bob', age: 25});"));
EXPECT_ANY_THROW(conn->query("MATCH (p:Person) WHERE p.name='Alice' SET p.age=26;"));
EXPECT_ANY_THROW(conn->query("MATCH (p:Person) WHERE name='Alice' DELETE p;"));
ASSERT_FALSE(conn->query("CREATE (:Person {name: 'Bob', age: 25});")->isSuccess());
ASSERT_FALSE(conn->query("MATCH (p:Person) WHERE p.name='Alice' SET p.age=26;")->isSuccess());
ASSERT_FALSE(conn->query("MATCH (p:Person) WHERE name='Alice' DELETE p;")->isSuccess());
}
4 changes: 2 additions & 2 deletions tools/rust_api/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ mod tests {
let conn = Connection::new(&db)?;
let result: Error = conn
.query("CREATE NODE TABLE Person(name STRING, age INT64, PRIMARY KEY(name));")
.expect_err("Invalid syntax in query should produce an error")
.expect_err("Write query should produce an error on a read-only DB")
.into();

assert_eq!(
result.to_string(),
"Cannot execute write operations in a read-only database!"
"Query execution failed: Cannot execute write operations in a read-only database!"
);
Ok(())
}
Expand Down

0 comments on commit 2504cd4

Please sign in to comment.