Skip to content

Commit ed2fe87

Browse files
authored
fix(rust)!: Let immutable drivers create connections (#2788)
Since the removal of the global driver lock, there is no risk of deadlocks between the locks on the driver and on the database objects. IIUC, this was why member functions on the database struct took a mutable `self` parameter: ```rust /// Initialize the given database using the loaded driver. /// /// This uses `&mut self` to prevent a deadlock. fn database_init( &mut self, mut database: ffi::FFI_AdbcDatabase, ) -> Result<ffi::FFI_AdbcDatabase> { ``` Requiring `&mut self` is overkill and requires another layer of locking at the application side if multiple threads share a database instance (as they should for connection pooling). This doesn't mean we are passing a forged mutable pointer to the driver via the FFI, the locking on the inner database ensures exclusive access to the `FFI_AdbcDatabase`. ```rust /// Initialize the given connection using the loaded driver. fn connection_init( &self, mut connection: ffi::FFI_AdbcConnection, ) -> Result<ffi::FFI_AdbcConnection> { let driver = self.ffi_driver(); let mut database = self.inner.database.lock().unwrap(); // ConnectionInit let mut error = ffi::FFI_AdbcError::with_driver(driver); let method = driver_method!(driver, ConnectionInit); let status = unsafe { method(&mut connection, &mut *database, &mut error) }; check_status(status, error)?; Ok(connection) } ``` **This might be considered a breaking change.** Closes #2790
1 parent e045731 commit ed2fe87

File tree

12 files changed

+50
-61
lines changed

12 files changed

+50
-61
lines changed

rust/core/src/driver_manager.rs

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
//! # fn main() -> Result<(), Box<dyn std::error::Error>> {
5757
//! let opts = [(OptionDatabase::Uri, ":memory:".into())];
5858
//! let mut driver = ManagedDriver::load_dynamic_from_name("adbc_driver_sqlite", None, AdbcVersion::V100)?;
59-
//! let mut database = driver.new_database_with_opts(opts)?;
59+
//! let database = driver.new_database_with_opts(opts)?;
6060
//! let mut connection = database.new_connection()?;
6161
//! let mut statement = connection.new_statement()?;
6262
//!
@@ -251,9 +251,7 @@ impl ManagedDriver {
251251
}
252252

253253
/// Returns a new database using the loaded driver.
254-
///
255-
/// This uses `&mut self` to prevent a deadlock.
256-
fn database_new(&mut self) -> Result<ffi::FFI_AdbcDatabase> {
254+
fn database_new(&self) -> Result<ffi::FFI_AdbcDatabase> {
257255
let driver = self.inner_ffi_driver();
258256
let mut database = ffi::FFI_AdbcDatabase::default();
259257

@@ -267,12 +265,7 @@ impl ManagedDriver {
267265
}
268266

269267
/// Initialize the given database using the loaded driver.
270-
///
271-
/// This uses `&mut self` to prevent a deadlock.
272-
fn database_init(
273-
&mut self,
274-
mut database: ffi::FFI_AdbcDatabase,
275-
) -> Result<ffi::FFI_AdbcDatabase> {
268+
fn database_init(&self, mut database: ffi::FFI_AdbcDatabase) -> Result<ffi::FFI_AdbcDatabase> {
276269
let driver = self.inner_ffi_driver();
277270

278271
// DatabaseInit
@@ -473,9 +466,7 @@ impl ManagedDatabase {
473466
}
474467

475468
/// Returns a new connection using the loaded driver.
476-
///
477-
/// This uses `&mut self` to prevent a deadlock.
478-
fn connection_new(&mut self) -> Result<ffi::FFI_AdbcConnection> {
469+
fn connection_new(&self) -> Result<ffi::FFI_AdbcConnection> {
479470
let driver = self.ffi_driver();
480471
let mut connection = ffi::FFI_AdbcConnection::default();
481472

@@ -489,10 +480,8 @@ impl ManagedDatabase {
489480
}
490481

491482
/// Initialize the given connection using the loaded driver.
492-
///
493-
/// This uses `&mut self` to prevent a deadlock.
494483
fn connection_init(
495-
&mut self,
484+
&self,
496485
mut connection: ffi::FFI_AdbcConnection,
497486
) -> Result<ffi::FFI_AdbcConnection> {
498487
let driver = self.ffi_driver();
@@ -577,7 +566,7 @@ impl Optionable for ManagedDatabase {
577566
impl Database for ManagedDatabase {
578567
type ConnectionType = ManagedConnection;
579568

580-
fn new_connection(&mut self) -> Result<Self::ConnectionType> {
569+
fn new_connection(&self) -> Result<Self::ConnectionType> {
581570
// Construct a new connection.
582571
let connection = self.connection_new()?;
583572
// Initialize the connection.
@@ -592,7 +581,7 @@ impl Database for ManagedDatabase {
592581
}
593582

594583
fn new_connection_with_opts(
595-
&mut self,
584+
&self,
596585
opts: impl IntoIterator<Item = (<Self::ConnectionType as Optionable>::Option, OptionValue)>,
597586
) -> Result<Self::ConnectionType> {
598587
// Construct a new connection.

rust/core/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,11 @@ pub trait Database: Optionable<Option = OptionDatabase> {
121121
type ConnectionType: Connection;
122122

123123
/// Allocate and initialize a new connection without pre-init options.
124-
fn new_connection(&mut self) -> Result<Self::ConnectionType>;
124+
fn new_connection(&self) -> Result<Self::ConnectionType>;
125125

126126
/// Allocate and initialize a new connection with pre-init options.
127127
fn new_connection_with_opts(
128-
&mut self,
128+
&self,
129129
opts: impl IntoIterator<Item = (options::OptionConnection, OptionValue)>,
130130
) -> Result<Self::ConnectionType>;
131131
}

rust/core/tests/common/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub fn test_driver(driver: &mut ManagedDriver, uri: &str) {
9595
assert!(driver.new_database_with_opts(opts).is_err());
9696
}
9797

98-
pub fn test_database(database: &mut ManagedDatabase) {
98+
pub fn test_database(database: &ManagedDatabase) {
9999
assert!(database.new_connection().is_ok());
100100

101101
let opts = [(OptionConnection::AutoCommit, "true".into())];

rust/core/tests/driver_manager_sqlite.rs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ fn test_driver() {
4646
#[test]
4747
fn test_database() {
4848
let mut driver = get_driver();
49-
let mut database = get_database(&mut driver);
50-
common::test_database(&mut database);
49+
let database = get_database(&mut driver);
50+
common::test_database(&database);
5151
}
5252

5353
#[test]
@@ -79,15 +79,15 @@ fn test_database_get_option() {
7979
#[test]
8080
fn test_connection() {
8181
let mut driver = get_driver();
82-
let mut database = get_database(&mut driver);
82+
let database = get_database(&mut driver);
8383
let mut connection = database.new_connection().unwrap();
8484
common::test_connection(&mut connection);
8585
}
8686

8787
#[test]
8888
fn test_connection_get_option() {
8989
let mut driver = get_driver();
90-
let mut database = get_database(&mut driver);
90+
let database = get_database(&mut driver);
9191
let connection = database.new_connection().unwrap();
9292

9393
let error = connection
@@ -114,7 +114,7 @@ fn test_connection_get_option() {
114114
#[test]
115115
fn test_connection_cancel() {
116116
let mut driver = get_driver();
117-
let mut database = get_database(&mut driver);
117+
let database = get_database(&mut driver);
118118
let mut connection = database.new_connection().unwrap();
119119

120120
let error = connection.cancel().unwrap_err();
@@ -124,71 +124,71 @@ fn test_connection_cancel() {
124124
#[test]
125125
fn test_connection_commit_rollback() {
126126
let mut driver = get_driver();
127-
let mut database = get_database(&mut driver);
127+
let database = get_database(&mut driver);
128128
let mut connection = database.new_connection().unwrap();
129129
common::test_connection_commit_rollback(&mut connection);
130130
}
131131

132132
#[test]
133133
fn test_connection_read_partition() {
134134
let mut driver = get_driver();
135-
let mut database = get_database(&mut driver);
135+
let database = get_database(&mut driver);
136136
let connection = database.new_connection().unwrap();
137137
common::test_connection_read_partition(&connection);
138138
}
139139

140140
#[test]
141141
fn test_connection_get_table_types() {
142142
let mut driver = get_driver();
143-
let mut database = get_database(&mut driver);
143+
let database = get_database(&mut driver);
144144
let connection = database.new_connection().unwrap();
145145
common::test_connection_get_table_types(&connection, &["table", "view"]);
146146
}
147147

148148
#[test]
149149
fn test_connection_get_info() {
150150
let mut driver = get_driver();
151-
let mut database = get_database(&mut driver);
151+
let database = get_database(&mut driver);
152152
let connection = database.new_connection().unwrap();
153153
common::test_connection_get_info(&connection, 5);
154154
}
155155

156156
#[test]
157157
fn test_connection_get_objects() {
158158
let mut driver = get_driver();
159-
let mut database = get_database(&mut driver);
159+
let database = get_database(&mut driver);
160160
let connection = database.new_connection().unwrap();
161161
common::test_connection_get_objects(&connection, 1, 1);
162162
}
163163

164164
#[test]
165165
fn test_connection_get_table_schema() {
166166
let mut driver = get_driver();
167-
let mut database = get_database(&mut driver);
167+
let database = get_database(&mut driver);
168168
let mut connection = database.new_connection().unwrap();
169169
common::test_connection_get_table_schema(&mut connection);
170170
}
171171

172172
#[test]
173173
fn test_connection_get_statistic_names() {
174174
let mut driver = get_driver();
175-
let mut database = get_database(&mut driver);
175+
let database = get_database(&mut driver);
176176
let connection = database.new_connection().unwrap();
177177
assert!(connection.get_statistic_names().is_err());
178178
}
179179

180180
#[test]
181181
fn test_connection_get_statistics() {
182182
let mut driver = get_driver();
183-
let mut database = get_database(&mut driver);
183+
let database = get_database(&mut driver);
184184
let connection = database.new_connection().unwrap();
185185
assert!(connection.get_statistics(None, None, None, false).is_err());
186186
}
187187

188188
#[test]
189189
fn test_statement() {
190190
let mut driver = get_driver();
191-
let mut database = get_database(&mut driver);
191+
let database = get_database(&mut driver);
192192
let mut connection = database.new_connection().unwrap();
193193
let mut statement = connection.new_statement().unwrap();
194194
common::test_statement(&mut statement);
@@ -197,7 +197,7 @@ fn test_statement() {
197197
#[test]
198198
fn test_statement_prepare() {
199199
let mut driver = get_driver();
200-
let mut database = get_database(&mut driver);
200+
let database = get_database(&mut driver);
201201
let mut connection = database.new_connection().unwrap();
202202
let mut statement = connection.new_statement().unwrap();
203203
common::test_statement_prepare(&mut statement);
@@ -206,7 +206,7 @@ fn test_statement_prepare() {
206206
#[test]
207207
fn test_statement_set_substrait_plan() {
208208
let mut driver = get_driver();
209-
let mut database = get_database(&mut driver);
209+
let database = get_database(&mut driver);
210210
let mut connection = database.new_connection().unwrap();
211211
let mut statement = connection.new_statement().unwrap();
212212
common::test_statement_set_substrait_plan(&mut statement);
@@ -215,7 +215,7 @@ fn test_statement_set_substrait_plan() {
215215
#[test]
216216
fn test_statement_get_parameter_schema() {
217217
let mut driver = get_driver();
218-
let mut database = get_database(&mut driver);
218+
let database = get_database(&mut driver);
219219
let mut connection = database.new_connection().unwrap();
220220
let mut statement = connection.new_statement().unwrap();
221221

@@ -233,7 +233,7 @@ fn test_statement_get_parameter_schema() {
233233
#[test]
234234
fn test_statement_execute() {
235235
let mut driver = get_driver();
236-
let mut database = get_database(&mut driver);
236+
let database = get_database(&mut driver);
237237
let mut connection = database.new_connection().unwrap();
238238
let mut statement = connection.new_statement().unwrap();
239239
common::test_statement_execute(&mut statement);
@@ -242,15 +242,15 @@ fn test_statement_execute() {
242242
#[test]
243243
fn test_statement_execute_update() {
244244
let mut driver = get_driver();
245-
let mut database = get_database(&mut driver);
245+
let database = get_database(&mut driver);
246246
let mut connection = database.new_connection().unwrap();
247247
common::test_statement_execute_update(&mut connection);
248248
}
249249

250250
#[test]
251251
fn test_statement_execute_schema() {
252252
let mut driver = get_driver();
253-
let mut database = get_database(&mut driver);
253+
let database = get_database(&mut driver);
254254
let mut connection = database.new_connection().unwrap();
255255
let mut statement = connection.new_statement().unwrap();
256256

@@ -261,7 +261,7 @@ fn test_statement_execute_schema() {
261261
#[test]
262262
fn test_statement_execute_partitions() {
263263
let mut driver = get_driver();
264-
let mut database = get_database(&mut driver);
264+
let database = get_database(&mut driver);
265265
let mut connection = database.new_connection().unwrap();
266266
let mut statement = connection.new_statement().unwrap();
267267
common::test_statement_execute_partitions(&mut statement);
@@ -270,7 +270,7 @@ fn test_statement_execute_partitions() {
270270
#[test]
271271
fn test_statement_cancel() {
272272
let mut driver = get_driver();
273-
let mut database = get_database(&mut driver);
273+
let database = get_database(&mut driver);
274274
let mut connection = database.new_connection().unwrap();
275275
let mut statement = connection.new_statement().unwrap();
276276

@@ -281,7 +281,7 @@ fn test_statement_cancel() {
281281
#[test]
282282
fn test_statement_bind() {
283283
let mut driver = get_driver();
284-
let mut database = get_database(&mut driver);
284+
let database = get_database(&mut driver);
285285
let mut connection = database.new_connection().unwrap();
286286
let mut statement = connection.new_statement().unwrap();
287287
common::test_statement_bind(&mut statement);
@@ -290,7 +290,7 @@ fn test_statement_bind() {
290290
#[test]
291291
fn test_statement_bind_stream() {
292292
let mut driver = get_driver();
293-
let mut database = get_database(&mut driver);
293+
let database = get_database(&mut driver);
294294
let mut connection = database.new_connection().unwrap();
295295
let mut statement = connection.new_statement().unwrap();
296296
common::test_statement_bind_stream(&mut statement);
@@ -299,7 +299,7 @@ fn test_statement_bind_stream() {
299299
#[test]
300300
fn test_ingestion_roundtrip() {
301301
let mut driver = get_driver();
302-
let mut database = get_database(&mut driver);
302+
let database = get_database(&mut driver);
303303
let mut connection = database.new_connection().unwrap();
304304
common::test_ingestion_roundtrip(&mut connection);
305305
}

rust/driver/datafusion/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ fn main() {
3636
)
3737
.unwrap();
3838
39-
let mut database = driver.new_database().unwrap();
39+
let database = driver.new_database().unwrap();
4040
4141
let mut connection = database.new_connection().unwrap();
4242

rust/driver/datafusion/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl Optionable for DataFusionDatabase {
183183
impl Database for DataFusionDatabase {
184184
type ConnectionType = DataFusionConnection;
185185

186-
fn new_connection(&mut self) -> Result<Self::ConnectionType> {
186+
fn new_connection(&self) -> Result<Self::ConnectionType> {
187187
let ctx = SessionContext::new();
188188

189189
let runtime = tokio::runtime::Builder::new_multi_thread()
@@ -198,7 +198,7 @@ impl Database for DataFusionDatabase {
198198
}
199199

200200
fn new_connection_with_opts(
201-
&mut self,
201+
&self,
202202
opts: impl IntoIterator<
203203
Item = (
204204
adbc_core::options::OptionConnection,

rust/driver/datafusion/tests/test_datafusion.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn get_connection() -> ManagedConnection {
3434
)
3535
.unwrap();
3636

37-
let mut database = driver.new_database().unwrap();
37+
let database = driver.new_database().unwrap();
3838

3939
database.new_connection().unwrap()
4040
}

rust/driver/dummy/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,12 @@ impl Optionable for DummyDatabase {
230230
impl Database for DummyDatabase {
231231
type ConnectionType = DummyConnection;
232232

233-
fn new_connection(&mut self) -> Result<Self::ConnectionType> {
233+
fn new_connection(&self) -> Result<Self::ConnectionType> {
234234
Ok(Self::ConnectionType::default())
235235
}
236236

237237
fn new_connection_with_opts(
238-
&mut self,
238+
&self,
239239
opts: impl IntoIterator<Item = (<Self::ConnectionType as Optionable>::Option, OptionValue)>,
240240
) -> Result<Self::ConnectionType> {
241241
let mut connection = Self::ConnectionType::default();

rust/driver/dummy/tests/driver_exporter_dummy.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ fn get_exported() -> (
7373
AdbcVersion::V110,
7474
)
7575
.unwrap();
76-
let mut database = driver.new_database().unwrap();
76+
let database = driver.new_database().unwrap();
7777
let mut connection = database.new_connection().unwrap();
7878
let statement = connection.new_statement().unwrap();
7979
(driver, database, connection, statement)
8080
}
8181

8282
fn get_native() -> (DummyDriver, DummyDatabase, DummyConnection, DummyStatement) {
8383
let mut driver = DummyDriver {};
84-
let mut database = driver.new_database().unwrap();
84+
let database = driver.new_database().unwrap();
8585
let mut connection = database.new_connection().unwrap();
8686
let statement = connection.new_statement().unwrap();
8787
(driver, database, connection, statement)
@@ -197,7 +197,7 @@ fn test_database_options() {
197197

198198
#[test]
199199
fn test_connection_options() {
200-
let (_, mut database, _, _) = get_exported();
200+
let (_, database, _, _) = get_exported();
201201

202202
// Pre-init options
203203
let options = [

0 commit comments

Comments
 (0)