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

Capture read only db message in query result #2508

Merged
merged 3 commits into from
Nov 29, 2023
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
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