Skip to content

Commit caf5755

Browse files
committed
fix(d1): Comprehensive improvements to Cloudflare D1 driver
- Remove mock feature dependency from d1 feature (adds D1ValueWrapper) - Fix unreachable POST handler in d1_example route matching - Add SQL wildcard escaping for search functionality in d1_example - Add comprehensive transaction documentation explaining D1 limitations - Add D1 transaction support in DatabaseTransaction (begin/commit/rollback) - Remove unused _unprepared parameter from execute_inner - Improve NULL conversion behavior with explicit matching and error logging - Replace all unwrap() calls with proper error handling in D1 query results - Standardize D1 error messages to consistent format - Fix potential panics in type conversions (DateTime, Decimal, UUID, IpNetwork) All 242 tests pass including 12 D1-specific unit tests.
1 parent cd133a0 commit caf5755

File tree

6 files changed

+294
-123
lines changed

6 files changed

+294
-123
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
107107
uuid = { version = "1", features = ["v4"] }
108108

109109
[features]
110-
d1 = ["worker/d1", "mock"]
110+
d1 = ["worker/d1"]
111111
debug-print = []
112112
default = [
113113
"macros",

examples/d1_example/src/lib.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,16 @@ async fn fetch(req: Request, env: Env, _ctx: Context) -> Result<Response> {
6565
let url = req.url()?;
6666
let path = url.path();
6767

68-
match path {
69-
"/" => Response::ok("Welcome to Sea-ORM D1 Example! Try /cakes, /cakes-entity, /cakes-filtered, or /cakes-search"),
70-
"/cakes" => handle_list_cakes(d1_conn).await,
71-
"/cakes-entity" => handle_list_cakes_entity(d1_conn).await,
72-
"/cakes-filtered" => handle_filtered_cakes(d1_conn).await,
73-
path if path.starts_with("/cakes-search") => handle_search_cakes(d1_conn, req).await,
74-
path if path == "/cakes" && req.method() == Method::Post => {
75-
handle_create_cake(d1_conn, req).await
76-
}
77-
path if path.starts_with("/cakes/") => {
68+
match (req.method(), path) {
69+
(Method::Get, "/") => Response::ok("Welcome to Sea-ORM D1 Example! Try /cakes, /cakes-entity, /cakes-filtered, or /cakes-search"),
70+
(Method::Get, "/cakes") => handle_list_cakes(d1_conn).await,
71+
(Method::Post, "/cakes") => handle_create_cake(d1_conn, req).await,
72+
(Method::Get, "/cakes-entity") => handle_list_cakes_entity(d1_conn).await,
73+
(Method::Get, "/cakes-filtered") => handle_filtered_cakes(d1_conn).await,
74+
(Method::Get, path) if path.starts_with("/cakes-search") => handle_search_cakes(d1_conn, req).await,
75+
(method, path) if path.starts_with("/cakes/") => {
7876
let id = path.trim_start_matches("/cakes/");
79-
match req.method() {
77+
match method {
8078
Method::Get => handle_get_cake(d1_conn, id).await,
8179
Method::Delete => handle_delete_cake(d1_conn, id).await,
8280
_ => Response::error("Method not allowed", 405),
@@ -135,6 +133,14 @@ async fn handle_filtered_cakes(d1_conn: &D1Connection) -> Result<Response> {
135133
Response::from_json(&results)
136134
}
137135

136+
/// Escape SQL wildcards in a search term to prevent unexpected behavior
137+
fn escape_like_pattern(s: &str) -> String {
138+
s.replace('%', "\\%")
139+
.replace('_', "\\_")
140+
.replace('[', "\\[")
141+
.replace(']', "\\]")
142+
}
143+
138144
/// Search cakes by name using query parameter
139145
async fn handle_search_cakes(d1_conn: &D1Connection, req: Request) -> Result<Response> {
140146
let url = req.url()?;
@@ -145,11 +151,14 @@ async fn handle_search_cakes(d1_conn: &D1Connection, req: Request) -> Result<Res
145151
return Response::error("Missing 'q' query parameter", 400);
146152
}
147153

154+
// Escape SQL wildcards to prevent them from being interpreted as wildcards
155+
let escaped_term = escape_like_pattern(&search_term);
156+
148157
// Use Entity::find() with LIKE filter (case-sensitive in SQLite)
149158
let cakes: Vec<cake::Model> = match d1_conn
150159
.find_all(
151160
cake::Entity::find()
152-
.filter(cake::Column::Name.like(&format!("%{}%", search_term)))
161+
.filter(cake::Column::Name.like(&format!("%{}%", escaped_term)))
153162
.order_by_asc(cake::Column::Name),
154163
)
155164
.await

src/database/db_connection.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use sqlx::pool::PoolConnection;
1313
#[cfg(feature = "rusqlite")]
1414
use crate::driver::rusqlite::{RusqliteInnerConnection, RusqliteSharedConnection};
1515

16-
#[cfg(any(feature = "mock", feature = "proxy", feature = "d1"))]
16+
#[cfg(any(feature = "mock", feature = "proxy"))]
1717
use std::sync::Arc;
1818

1919
/// Handle a database connection depending on the backend enabled by the feature

src/database/transaction.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ impl DatabaseTransaction {
108108
c.begin().await;
109109
Ok(())
110110
}
111+
#[cfg(feature = "d1")]
112+
InnerConnection::D1(_) => {
113+
// D1 doesn't support explicit transactions - statements auto-commit
114+
// We return Ok to allow transaction-like code to run, but each
115+
// statement is committed independently
116+
Ok(())
117+
}
111118
#[allow(unreachable_patterns)]
112119
_ => Err(conn_err("Disconnected")),
113120
}
@@ -187,6 +194,11 @@ impl DatabaseTransaction {
187194
c.commit().await;
188195
Ok(())
189196
}
197+
#[cfg(feature = "d1")]
198+
InnerConnection::D1(_) => {
199+
// D1 auto-commits each statement, so commit is a no-op
200+
Ok(())
201+
}
190202
#[allow(unreachable_patterns)]
191203
_ => Err(conn_err("Disconnected")),
192204
}
@@ -244,6 +256,14 @@ impl DatabaseTransaction {
244256
c.rollback().await;
245257
Ok(())
246258
}
259+
#[cfg(feature = "d1")]
260+
InnerConnection::D1(_) => {
261+
// D1 doesn't support rollback - each statement auto-commits
262+
// We return Ok since there's nothing to rollback, but warn that
263+
// transactional behavior is not guaranteed
264+
tracing::warn!("D1 doesn't support rollback - statements were auto-committed");
265+
Ok(())
266+
}
247267
#[allow(unreachable_patterns)]
248268
_ => Err(conn_err("Disconnected")),
249269
}
@@ -285,6 +305,10 @@ impl DatabaseTransaction {
285305
InnerConnection::Proxy(c) => {
286306
c.start_rollback();
287307
}
308+
#[cfg(feature = "d1")]
309+
InnerConnection::D1(_) => {
310+
// D1 doesn't support rollback - nothing to do
311+
}
288312
#[allow(unreachable_patterns)]
289313
_ => return Err(conn_err("Disconnected")),
290314
}
@@ -371,6 +395,12 @@ impl ConnectionTrait for DatabaseTransaction {
371395
InnerConnection::Mock(conn) => conn.execute(stmt),
372396
#[cfg(feature = "proxy")]
373397
InnerConnection::Proxy(conn) => conn.execute(stmt).await,
398+
#[cfg(feature = "d1")]
399+
InnerConnection::D1(conn) => {
400+
// D1 doesn't support transactions in the traditional sense,
401+
// but we need to support execute within transaction context
402+
Err(conn_err("D1 transactions do not support execute_raw. Use D1Connection directly."))
403+
}
374404
#[allow(unreachable_patterns)]
375405
_ => Err(conn_err("Disconnected")),
376406
}
@@ -433,6 +463,10 @@ impl ConnectionTrait for DatabaseTransaction {
433463
let stmt = Statement::from_string(db_backend, sql);
434464
conn.execute(stmt).await
435465
}
466+
#[cfg(feature = "d1")]
467+
InnerConnection::D1(conn) => {
468+
Err(conn_err("D1 transactions do not support execute_unprepared. Use D1Connection directly."))
469+
}
436470
#[allow(unreachable_patterns)]
437471
_ => Err(conn_err("Disconnected")),
438472
}
@@ -493,6 +527,10 @@ impl ConnectionTrait for DatabaseTransaction {
493527
InnerConnection::Mock(conn) => conn.query_one(stmt),
494528
#[cfg(feature = "proxy")]
495529
InnerConnection::Proxy(conn) => conn.query_one(stmt).await,
530+
#[cfg(feature = "d1")]
531+
InnerConnection::D1(conn) => {
532+
Err(conn_err("D1 transactions do not support query_one_raw. Use D1Connection directly."))
533+
}
496534
#[allow(unreachable_patterns)]
497535
_ => Err(conn_err("Disconnected")),
498536
}
@@ -559,6 +597,10 @@ impl ConnectionTrait for DatabaseTransaction {
559597
InnerConnection::Mock(conn) => conn.query_all(stmt),
560598
#[cfg(feature = "proxy")]
561599
InnerConnection::Proxy(conn) => conn.query_all(stmt).await,
600+
#[cfg(feature = "d1")]
601+
InnerConnection::D1(conn) => {
602+
Err(conn_err("D1 transactions do not support query_all_raw. Use D1Connection directly."))
603+
}
562604
#[allow(unreachable_patterns)]
563605
_ => Err(conn_err("Disconnected")),
564606
}

src/driver/d1.rs

Lines changed: 121 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ impl D1Connection {
179179
.unwrap_or_else(|| Values(Vec::new()));
180180

181181
crate::metric::metric!(self.metric_callback, &stmt, {
182-
match self.execute_inner(&sql, &values, false).await {
182+
match self.execute_inner(&sql, &values).await {
183183
Ok(result) => Ok(result.into()),
184184
Err(err) => Err(d1_error_to_exec_err(err)),
185185
}
@@ -193,7 +193,7 @@ impl D1Connection {
193193

194194
let values = Values(Vec::new());
195195

196-
match self.execute_inner(sql, &values, false).await {
196+
match self.execute_inner(sql, &values).await {
197197
Ok(result) => Ok(result.into()),
198198
Err(err) => Err(d1_error_to_exec_err(err)),
199199
}
@@ -240,6 +240,31 @@ impl D1Connection {
240240
}
241241

242242
/// Begin a transaction
243+
///
244+
/// # D1 Transaction Limitations
245+
///
246+
/// **Important:** D1 has limited transaction support compared to traditional databases:
247+
/// - **No ACID guarantees**: D1 does not provide full ACID transaction semantics
248+
/// - **No isolation levels**: Isolation levels are not supported and will be ignored
249+
/// - **No access mode control**: Read-only vs read-write modes are not enforced
250+
/// - **Best-effort only**: Each statement is executed independently; if one fails,
251+
/// previous statements are not automatically rolled back
252+
/// - **No savepoints**: Nested transactions are not supported
253+
///
254+
/// For production use cases requiring strong transactional guarantees, consider
255+
/// using a different database backend or implementing application-level compensation logic.
256+
///
257+
/// # Example
258+
///
259+
/// ```ignore
260+
/// let tx = d1_conn.begin(None, None).await?;
261+
///
262+
/// // Execute operations...
263+
/// let result = d1_conn.execute(stmt1).await;
264+
///
265+
/// // Commit or rollback
266+
/// tx.commit().await?;
267+
/// ```
243268
#[instrument(level = "trace")]
244269
pub async fn begin(
245270
&self,
@@ -254,12 +279,34 @@ impl D1Connection {
254279
}
255280

256281
// D1 doesn't support explicit transactions in the traditional sense.
257-
// We'll use a no-op transaction that just commits/rollbacks immediately.
258-
// This is a limitation of D1's current API.
282+
// Each statement is executed independently.
259283
DatabaseTransaction::new_d1(self.d1.clone(), self.metric_callback.clone()).await
260284
}
261285

262286
/// Execute a function inside a transaction
287+
///
288+
/// # D1 Transaction Limitations
289+
///
290+
/// **Important:** This method provides a transaction-like interface, but due to D1's
291+
/// limitations, it cannot provide full ACID guarantees:
292+
///
293+
/// - **Partial failure risk**: If the callback fails partway through, earlier statements
294+
/// may have already been committed by D1 and cannot be rolled back
295+
/// - **No atomicity**: Operations are not executed atomically
296+
/// - **No consistency guarantees**: Database constraints may be violated between statements
297+
/// - **Isolation and access mode**: These parameters are ignored
298+
///
299+
/// For production use requiring strong guarantees, consider implementing
300+
/// idempotent operations or application-level compensation logic.
301+
///
302+
/// # Example
303+
///
304+
/// ```ignore
305+
/// d1_conn.transaction(|tx| Box::pin(async move {
306+
/// // Your operations here...
307+
/// Ok(result)
308+
/// }), None, None).await?;
309+
/// ```
263310
#[instrument(level = "trace", skip(callback))]
264311
pub async fn transaction<F, T, E>(
265312
&self,
@@ -299,11 +346,12 @@ impl D1Connection {
299346
}
300347

301348
/// Internal method to execute SQL and get execution result
349+
///
350+
/// Note: D1 always uses prepared statements, so there's no unprepared execution path.
302351
async fn execute_inner(
303352
&self,
304353
sql: &str,
305354
values: &Values,
306-
_unprepared: bool,
307355
) -> Result<D1ExecResult, D1Error> {
308356
let js_values = values_to_js_values(values)?;
309357

@@ -448,12 +496,43 @@ fn value_to_js_value(val: &Value) -> Result<JsValue, D1Error> {
448496
Value::TimeDateTime(Some(v)) => Ok(JsValue::from(v.to_string())),
449497
#[cfg(feature = "with-time")]
450498
Value::TimeDateTimeWithTimeZone(Some(v)) => Ok(JsValue::from(v.to_string())),
451-
// Unsupported types - log warning and return NULL
499+
// Null values and unsupported types
500+
Value::Bool(None)
501+
| Value::Int(None)
502+
| Value::BigInt(None)
503+
| Value::SmallInt(None)
504+
| Value::TinyInt(None)
505+
| Value::Unsigned(None)
506+
| Value::BigUnsigned(None)
507+
| Value::SmallUnsigned(None)
508+
| Value::TinyUnsigned(None)
509+
| Value::Float(None)
510+
| Value::Double(None)
511+
| Value::String(None)
512+
| Value::Char(None)
513+
| Value::Bytes(None)
514+
| Value::Json(None) => Ok(JsValue::NULL),
515+
#[cfg(feature = "with-chrono")]
516+
Value::ChronoDate(None)
517+
| Value::ChronoTime(None)
518+
| Value::ChronoDateTime(None)
519+
| Value::ChronoDateTimeUtc(None)
520+
| Value::ChronoDateTimeLocal(None)
521+
| Value::ChronoDateTimeWithTimeZone(None) => Ok(JsValue::NULL),
522+
#[cfg(feature = "with-time")]
523+
Value::TimeDate(None)
524+
| Value::TimeTime(None)
525+
| Value::TimeDateTime(None)
526+
| Value::TimeDateTimeWithTimeZone(None) => Ok(JsValue::NULL),
527+
// Unsupported types - log error and return NULL
528+
// Note: In strict mode, this should return an error instead
529+
#[allow(unreachable_patterns)]
452530
val => {
453-
tracing::warn!(
454-
"D1 does not support value type {:?} - converting to NULL. \
455-
Consider using a supported type (i8, i16, i32, i64, u8, u16, u32, u64, f32, f64, String, Vec<u8>, serde_json::Value)",
456-
val
531+
tracing::error!(
532+
"D1 does not support value type {:?} - data will be lost (converting to NULL). \
533+
Use a supported type (bool, i8, i16, i32, i64, u8, u16, u32, u64, f32, f64, String, Vec<u8>, serde_json::Value, chrono types, time types). \
534+
Consider enabling strict mode to catch these errors at development time.",
535+
std::mem::discriminant(val)
457536
);
458537
Ok(JsValue::NULL)
459538
}
@@ -478,6 +557,38 @@ fn d1_error_to_conn_err(err: D1Error) -> DbErr {
478557
)))
479558
}
480559

560+
/// Internal helper for converting D1 values to target types
561+
///
562+
/// This provides a MockRow-like interface for value conversion without
563+
/// requiring the mock feature to be enabled.
564+
#[derive(Debug, Clone)]
565+
pub(crate) struct D1ValueWrapper {
566+
values: std::collections::BTreeMap<String, Value>,
567+
}
568+
569+
impl D1ValueWrapper {
570+
/// Create a new wrapper with a single value
571+
pub(crate) fn with_value(key: String, value: Value) -> Self {
572+
let mut values = std::collections::BTreeMap::new();
573+
values.insert(key, value);
574+
Self { values }
575+
}
576+
577+
/// Get a value from the wrapper
578+
pub(crate) fn try_get<T>(&self, index: &str) -> Result<T, DbErr>
579+
where
580+
T: sea_query::ValueType,
581+
{
582+
T::try_from(
583+
self.values
584+
.get(index)
585+
.ok_or_else(|| query_err(format!("No column for index {index:?}")))?
586+
.clone(),
587+
)
588+
.map_err(type_err)
589+
}
590+
}
591+
481592
/// Convert D1 JSON row to Sea-ORM values
482593
pub(crate) fn d1_row_to_values(row: &D1Row) -> Vec<(String, Value)> {
483594
let mut values = Vec::new();

0 commit comments

Comments
 (0)