Enable sqlite returning with feature flag (#2070)

* Enable sqlite returning with feature flag

* Add runtime check for sqlite version

* Apply suggestions from code review

* Fix clippy

* Rename feature

* Fix tests when feature flag is not enabled

---------

Co-authored-by: Chris Tsang <chris.2y3@outlook.com>

.github/workflows/rust.yml

@@ -308,6 +308,7 @@ jobs:
       matrix:
         runtime: [async-std]
         tls: [native-tls, rustls]
+        sqlite_flavor: ["", sqlite-use-returning-for-3_35]
     steps:
       - uses: actions/checkout@v3
       - uses: dtolnay/rust-toolchain@stable
@@ -319,8 +320,8 @@ jobs:
             Cargo.lock
             target
           key: ${{ github.sha }}-${{ github.run_id }}-${{ runner.os }}-sqlite-${{ matrix.runtime }}-${{ matrix.tls }}
-      - run: cargo test --test '*' --features default,sqlx-sqlite,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
-      - run: cargo test --manifest-path sea-orm-migration/Cargo.toml --test '*' --features sqlx-sqlite,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
+      - run: cargo test --test '*' --features default,sqlx-sqlite,runtime-${{ matrix.runtime }}-${{ matrix.tls }},${{ matrix.sqlite_flavor }}
+      - run: cargo test --manifest-path sea-orm-migration/Cargo.toml --test '*' --features sqlx-sqlite,runtime-${{ matrix.runtime }}-${{ matrix.tls }},${{ matrix.sqlite_flavor }}
 
   mysql:
     name: MySQL

Cargo.toml

@@ -90,6 +90,7 @@ sqlx-all = ["sqlx-mysql", "sqlx-postgres", "sqlx-sqlite"]
 sqlx-mysql = ["sqlx-dep", "sea-query-binder/sqlx-mysql", "sqlx/mysql"]
 sqlx-postgres = ["sqlx-dep", "sea-query-binder/sqlx-postgres", "sqlx/postgres", "postgres-array"]
 sqlx-sqlite = ["sqlx-dep", "sea-query-binder/sqlx-sqlite", "sqlx/sqlite"]
+sqlite-use-returning-for-3_35 = []
 runtime-async-std = []
 runtime-async-std-native-tls = [
     "sqlx?/runtime-async-std-native-tls",
@@ -128,4 +129,4 @@ seaography = ["sea-orm-macros/seaography"]
 
 # This allows us to develop using a local version of sea-query
 # [patch.crates-io]
-# sea-query = { path = "../sea-query" }
+# sea-query = { path = "../sea-query" }

sea-orm-migration/Cargo.toml

@@ -39,6 +39,7 @@ cli = ["clap", "dotenvy", "sea-orm-cli/cli"]
 sqlx-mysql = ["sea-orm/sqlx-mysql"]
 sqlx-postgres = ["sea-orm/sqlx-postgres"]
 sqlx-sqlite = ["sea-orm/sqlx-sqlite"]
+sqlite-use-returning-for-3_35 = ["sea-orm/sqlite-use-returning-for-3_35"]
 runtime-actix-native-tls = ["sea-orm/runtime-actix-native-tls"]
 runtime-async-std-native-tls = ["sea-orm/runtime-async-std-native-tls"]
 runtime-tokio-native-tls = ["sea-orm/runtime-tokio-native-tls"]

src/database/db_connection.rs

@@ -578,7 +578,15 @@ impl DbBackend {
 
     /// Check if the database supports `RETURNING` syntax on insert and update
     pub fn support_returning(&self) -> bool {
-        matches!(self, Self::Postgres)
+        #[cfg(not(feature = "sqlite-use-returning-for-3_35"))]
+        {
+            matches!(self, Self::Postgres)
+        }
+
+        #[cfg(feature = "sqlite-use-returning-for-3_35")]
+        {
+            matches!(self, Self::Postgres | Self::Sqlite)
+        }
     }
 }
 

src/driver/sqlx_sqlite.rs

@@ -12,8 +12,9 @@ use sea_query_binder::SqlxValues;
 use tracing::{instrument, warn};
 
 use crate::{
-    debug_print, error::*, executor::*, AccessMode, ConnectOptions, DatabaseConnection,
-    DatabaseTransaction, IsolationLevel, QueryStream, Statement, TransactionError,
+    debug_print, error::*, executor::*, sqlx_error_to_exec_err, AccessMode, ConnectOptions,
+    DatabaseConnection, DatabaseTransaction, IsolationLevel, QueryStream, Statement,
+    TransactionError,
 };
 
 use super::sqlx_common::*;
@@ -68,12 +69,20 @@ impl SqlxSqliteConnector {
             options.max_connections(1);
         }
         match options.pool_options().connect_with(opt).await {
-            Ok(pool) => Ok(DatabaseConnection::SqlxSqlitePoolConnection(
-                SqlxSqlitePoolConnection {
+            Ok(pool) => {
+                let pool = SqlxSqlitePoolConnection {
                     pool,
                     metric_callback: None,
-                },
-            )),
+                };
+
+                #[cfg(feature = "sqlite-use-returning-for-3_35")]
+                {
+                    let version = get_version(&pool).await?;
+                    ensure_returning_version(&version)?;
+                }
+
+                Ok(DatabaseConnection::SqlxSqlitePoolConnection(pool))
+            }
             Err(e) => Err(sqlx_error_to_conn_err(e)),
         }
     }
@@ -268,3 +277,80 @@ pub(crate) async fn set_transaction_config(
     }
     Ok(())
 }
+
+#[cfg(feature = "sqlite-use-returning-for-3_35")]
+async fn get_version(conn: &SqlxSqlitePoolConnection) -> Result<String, DbErr> {
+    let stmt = Statement {
+        sql: "SELECT sqlite_version()".to_string(),
+        values: None,
+        db_backend: crate::DbBackend::Sqlite,
+    };
+    conn.query_one(stmt)
+        .await?
+        .ok_or_else(|| {
+            DbErr::Conn(RuntimeErr::Internal(
+                "Error reading SQLite version".to_string(),
+            ))
+        })?
+        .try_get_by(0)
+}
+
+#[cfg(feature = "sqlite-use-returning-for-3_35")]
+fn ensure_returning_version(version: &str) -> Result<(), DbErr> {
+    let mut parts = version.trim().split('.').map(|part| {
+        part.parse::<u32>().map_err(|_| {
+            DbErr::Conn(RuntimeErr::Internal(
+                "Error parsing SQLite version".to_string(),
+            ))
+        })
+    });
+
+    let mut extract_next = || {
+        parts.next().transpose().and_then(|part| {
+            part.ok_or_else(|| {
+                DbErr::Conn(RuntimeErr::Internal("SQLite version too short".to_string()))
+            })
+        })
+    };
+
+    let major = extract_next()?;
+    let minor = extract_next()?;
+
+    if major > 3 || (major == 3 && minor >= 35) {
+        Ok(())
+    } else {
+        Err(DbErr::Conn(RuntimeErr::Internal(
+            "SQLite version does not support returning".to_string(),
+        )))
+    }
+}
+
+#[cfg(all(test, feature = "sqlite-use-returning-for-3_35"))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_ensure_returning_version() {
+        assert!(ensure_returning_version("").is_err());
+        assert!(ensure_returning_version(".").is_err());
+        assert!(ensure_returning_version(".a").is_err());
+        assert!(ensure_returning_version(".4.9").is_err());
+        assert!(ensure_returning_version("a").is_err());
+        assert!(ensure_returning_version("1.").is_err());
+        assert!(ensure_returning_version("1.a").is_err());
+
+        assert!(ensure_returning_version("1.1").is_err());
+        assert!(ensure_returning_version("1.0.").is_err());
+        assert!(ensure_returning_version("1.0.0").is_err());
+        assert!(ensure_returning_version("2.0.0").is_err());
+        assert!(ensure_returning_version("3.34.0").is_err());
+        assert!(ensure_returning_version("3.34.999").is_err());
+
+        // valid version
+        assert!(ensure_returning_version("3.35.0").is_ok());
+        assert!(ensure_returning_version("3.35.1").is_ok());
+        assert!(ensure_returning_version("3.36.0").is_ok());
+        assert!(ensure_returning_version("4.0.0").is_ok());
+        assert!(ensure_returning_version("99.0.0").is_ok());
+    }
+}

tests/crud/error.rs

@@ -40,6 +40,10 @@ pub async fn test_cake_error_sqlx(db: &DbConn) {
             }
             _ => panic!("Unexpected sqlx-error kind"),
         },
+        #[cfg(all(feature = "sqlx-sqlite", feature = "sqlite-use-returning-for-3_35"))]
+        DbErr::Query(RuntimeErr::SqlxError(Error::Database(e))) => {
+            assert_eq!(e.code().unwrap(), "1555");
+        }
         _ => panic!("Unexpected Error kind"),
     }
     #[cfg(feature = "sqlx-postgres")]

tests/returning_tests.rs

@@ -75,7 +75,13 @@ async fn main() -> Result<(), DbErr> {
     feature = "sqlx-postgres"
 ))]
 #[cfg_attr(
-    any(feature = "sqlx-mysql", feature = "sqlx-sqlite"),
+    any(
+        feature = "sqlx-mysql",
+        all(
+            feature = "sqlx-sqlite",
+            not(feature = "sqlite-use-returning-for-3_35")
+        )
+    ),
     should_panic(expected = "Database backend doesn't support RETURNING")
 )]
 async fn update_many() {