Skip to content

Conversation

@LaurentRDC
Copy link
Member

The behavior of runInsertReturningList currently fails to return rows which had a conflict on insert. This is indirectly because beam-sqlite emulates the behavior of INSERT .. ON CONFLICT ... RETURNING.

SQLite 3.35 added support for a proper RETURNING clause on inserts. We should use this instead.

Fixes #773

@LaurentRDC LaurentRDC force-pushed the sqlite-insert-returning branch from e4e8993 to 30e58bd Compare October 16, 2025 00:41
@LaurentRDC LaurentRDC marked this pull request as ready for review October 18, 2025 00:07
@sheaf
Copy link

sheaf commented Oct 18, 2025

Fantastic, I'm really happy to see this.

I also looked into adding support for DELETE ... RETURNING, the following seems to work to return entire rows:

instance Beam.MonadBeamDeleteReturning Sqlite SqliteM where
  runDeleteReturningList = runDeleteReturningList

runDeleteReturningList :: (Beamable table, FromBackendRow Sqlite (table Identity))
                       => SqlDelete Sqlite table
                       -> SqliteM [ table Identity ]
runDeleteReturningList (SqlDelete _ deleteCommand) =
  runReturningList $ SqliteCommandSyntax $
    fromSqliteDelete deleteCommand <> emit " RETURNING *"

I don't understand how we are supposed to choose what to return in a "delete returning list", e.g. if we want to only return the ids of deleted rows for instance.

Your patch is working great on simple examples, but I will let you know how it goes on a more complicate database setup later today. (What I have is still pretty simple though, so it's not gonna be a great correctness stress test, especially as I don't use default values much.)

Comment on lines +351 to +355
forM_ es $ \row -> do
-- RETURNING is only supported by SQLite 3.35+, which requires direct-sqlite 2.3.27+
let returningClause = emit " RETURNING " <> commas (map quotedIdentifier fields)
(insertFields, insertRow) = unzip $ filter ((/= SqliteExpressionDefault) . snd) $ zip fields row
SqliteSyntax cmd vals = formatSqliteInsertOnConflict tbl insertFields (SqliteInsertExpressions [ insertRow ]) onConflict <> returningClause
cmdString = BL.unpack (toLazyByteString (withPlaceholders cmd))
Copy link

@sheaf sheaf Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a minor comment (that probably should be punted to a separate ticket), but: would it be possible to instead classify the rows by which fields have DEFAULT values? For example, if all the rows to be inserted have DEFAULT in the same position, then we can still insert all the rows at once (although we still need to filter out the DEFAULT fields).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's very true.
I considered doing something like it, but I started with this simpler implementation instead because the order of insertion is preserved this way. If you grouped rows by the positions of their default values, then you would have to reconstruct the return order.
This isn't impossible, but this is significantly more complex. As you mention, this could be punted to a new ticket.

Actually, I see you have made a prototype implementation for this below; would you like to submit a PR?

@sheaf
Copy link

sheaf commented Oct 18, 2025

Ah, I see that for DELETE ... RETURNING you need a separate function if you don't want to return the entire row. I tried to copy the Postgres implementation but it doesn't quite work yet:

instance Beam.MonadBeamDeleteReturning Sqlite SqliteM where
  runDeleteReturningList (SqlDelete _ deleteCommand) =
    runReturningList $
      SqliteCommandSyntax $ fromSqliteDelete deleteCommand

data SqliteInaccessible

-- | SQLite @DELETE ... RETURNING@ statement support. The last
-- argument takes the newly inserted row and returns the values to be
-- returned. Use 'runDeleteReturning' to get the results.
deleteReturning :: Projectible Sqlite a
                => DatabaseEntity Sqlite be (TableEntity table)
                -> (forall s. table (QExpr Sqlite s) -> QExpr Sqlite s Bool)
                -> (table (QExpr Sqlite SqliteInaccessible) -> a)
                -> SqlDelete Sqlite table
deleteReturning table@(DatabaseEntity (DatabaseTable { dbTableSettings = tblSettings }))
                mkWhere
                mkProjection =
  SqlDelete tblSettings $ SqliteDeleteSyntax $
    fromSqliteDelete sqliteDelete <>
    emit " RETURNING " <> commas (map fromSqliteExpression (project (Proxy @Sqlite) (mkProjection tblQ) "t"))
  where
    SqlDelete _ sqliteDelete = delete table $ \t -> mkWhere t
    tblQ = changeBeamRep (\(Columnar' f) -> Columnar' (QExpr (pure (fieldE (unqualifiedField (_fieldName f)))))) tblSettings
DELETE FROM "table" WHERE ("id")>=(?) RETURNING "id";
-- With values: [SQLInteger 5]
sqlite-bug: Uncaught exception beam-core-0.10.4.0-0a1612816156ecf01c2cd7d20b49527261746a58:Database.Beam.Backend.SQL.Row.BeamRowReadError:
BeamRowReadError {brreColumn = Just 0, brreError = ColumnTypeMismatch {ctmHaskellType = "Null", ctmSQLType = "INTEGER", ctmMessage = "conversion failed: data is not null"}}

@sheaf
Copy link

sheaf commented Oct 18, 2025

The following works for "DELETE ... RETURNING":

newtype SqliteDeleteReturning a = SqliteDeleteReturning SqliteDeleteSyntax

data SqliteInaccessible

runDeleteReturningList
  :: ( MonadBeam be m
     , BeamSqlBackendSyntax be ~ SqliteCommandSyntax
     , FromBackendRow be a
     )
  => SqliteDeleteReturning a
  -> m [a]
runDeleteReturningList (SqliteDeleteReturning (SqliteDeleteSyntax syntax)) =
  runReturningList $ SqliteCommandSyntax syntax

-- | SQLite @DELETE ... RETURNING@ statement support. The last
-- argument takes the newly inserted row and returns the values to be
-- returned. Use 'runDeleteReturningList' to get the results.
deleteReturning :: forall a table be. Projectible Sqlite a
                => DatabaseEntity Sqlite be (TableEntity table)
                -> (forall s. table (QExpr Sqlite s) -> QExpr Sqlite s Bool)
                -> (table (QExpr Sqlite SqliteInaccessible) -> a)
                -> SqliteDeleteReturning (QExprToIdentity a)
deleteReturning table@(DatabaseEntity (DatabaseTable { dbTableSettings = tblSettings }))
                mkWhere
                mkProjection =
  SqliteDeleteReturning $ SqliteDeleteSyntax $
    fromSqliteDelete sqliteDelete
      <>
    emit " RETURNING " <> commas (map fromSqliteExpression (project (Proxy @Sqlite) (mkProjection tblQ) "t"))
  where

    SqlDelete _ sqliteDelete = delete table mkWhere
    tblQ = changeBeamRep getFieldName tblSettings
    getFieldName (Columnar' fd) =
      Columnar' $ QExpr $ pure $ fieldE (unqualifiedField (_fieldName fd))

I think the monads such as MonadBeamDeleteReturning etc are incorrectly designed because their type signatures all return the entire table while one might want to project out only certain parts of the table. This is why the Postgres backend defines functions such as runPgDeleteReturningList where the return type is in terms of a (the thing we are returning) and not in terms of the original table.

@sheaf
Copy link

sheaf commented Oct 18, 2025

Here's my attempt to group all the insertions by the collection of defaults that should be omitted from the query:

newtype SqliteInsertReturning a = SqliteInsertReturning [SqliteSyntax]

runInsertReturningList
  :: ( MonadBeam be m
     , BeamSqlBackendSyntax be ~ SqliteCommandSyntax
     , FromBackendRow be a
     )
  => SqliteInsertReturning a
  -> m [a]
runInsertReturningList (SqliteInsertReturning es) =
  concat <$>
    traverse ( \ syntax -> runReturningList $ SqliteCommandSyntax syntax ) es

data SqliteInaccessible

insertOnConflictReturning
  :: forall s a table db. Projectible Sqlite a
  => DatabaseEntity Sqlite db (TableEntity table)
  -> SqlInsertValues Sqlite (table (QExpr Sqlite s))
  -> SqlConflictTarget Sqlite table
  -> SqlConflictAction Sqlite table
  -> (table (QExpr Sqlite SqliteInaccessible) -> a)
        -- ^ projection describing what to return
  -> SqliteInsertReturning (QExprToIdentity a)
insertOnConflictReturning
  table@(DatabaseEntity (DatabaseTable { dbTableSettings = tblSettings }))
  vals tgt action mkProjection =
  SqliteInsertReturning $
    case insertOnConflict table vals tgt action of
      SqlInsert _ (SqliteInsertSyntax tbl fields values onConflict) ->
        [ formatSqliteInsertOnConflict tbl fields' values' onConflict <>
          emit " RETURNING " <> commas (map fromSqliteExpression (project (Proxy @Sqlite) (mkProjection tblQ) "t"))
        | ( fields', values' ) <- sqliteGroupByDefaults fields values
        ]
      SqlInsertNoRows -> []
  where
    tblQ = changeBeamRep getFieldName tblSettings
    getFieldName (Columnar' fd) =
      Columnar' $ QExpr $ pure $ fieldE (unqualifiedField (_fieldName fd))

-- | SQLite cannot deal with DEFAULT in insertion. Instead, it expects these
-- fields to be omitted from the INSERT.
--
-- This means that, for any given INSERT query, the row value being inserted
-- must have a fixed set of fields with DEFAULT value.
--
-- This function groups the inserted values by the set of DEFAULT fields, so
-- that we can preform a separate INSERT for each set of DEFAULT fields.
sqliteGroupByDefaults :: [ Text ] -> SqliteInsertValuesSyntax -> [([Text], SqliteInsertValuesSyntax)]
sqliteGroupByDefaults flds ( SqliteInsertExpressions es ) =
  let
    partitionDefaultsFrom :: (IntSet, [SqliteExpressionSyntax]) -> Int -> [SqliteExpressionSyntax] -> (IntSet, [SqliteExpressionSyntax])
    partitionDefaultsFrom (defs, acc) _ [] = ( defs, reverse acc )
    partitionDefaultsFrom (defs, acc) i (x:xs)
      | x == SqliteExpressionDefault
      = partitionDefaultsFrom ( IntSet.insert i defs, acc ) ( i + 1 ) xs
      | otherwise
      = partitionDefaultsFrom ( defs, x : acc ) ( i + 1 ) xs

    grouped :: [ ( IntSet, [[SqliteExpressionSyntax]] ) ]
    grouped =
      Strict.Map.toList $
      Strict.Map.fromListWith (++)
        [ ( defaults, [ e' ] )
        | e <- es
        , let ( defaults, e' ) = partitionDefaultsFrom ( IntSet.empty, [] ) 0 e
        ]

    mkSyntax dflts xs =
      ( [ fld | ( i, fld ) <- zip [0..] flds
              , not $ i `IntSet.member` dflts
              ]
        , SqliteInsertExpressions xs )

  in
    map ( uncurry mkSyntax ) grouped
sqliteGroupByDefaults flds orig@( SqliteInsertFromSql{}) =
  -- Preserve manually-written queries
  [(flds, orig)]

@LaurentRDC LaurentRDC force-pushed the sqlite-insert-returning branch from 493a578 to 65cc470 Compare October 18, 2025 23:49
@LaurentRDC LaurentRDC changed the title Proper use of SQLite's RETURNING mechanism Proper use of SQLite's RETURNING mechanism for inserts Oct 18, 2025
@LaurentRDC
Copy link
Member Author

Thanks for the review. Since this is fixing a bug, I'll release this right now. I'll be happy to review a pull request if you want to make SQLite inserts more efficient by reducing the number of statements

@LaurentRDC LaurentRDC merged commit 24ecab6 into master Oct 18, 2025
9 of 10 checks passed
@LaurentRDC LaurentRDC deleted the sqlite-insert-returning branch October 19, 2025 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQLite INSERT ON CONFLICT RETURNING does not return conflicts

3 participants