Re: Issues with sqlite3session_attach(conn, NULL) w/ duplicate table in temp

Dan Kennedy <[email protected]> Fri, 28 Feb 2020 00:22:44 +0700
Newsgroups gmane.comp.db.sqlite.general
Message-ID <[email protected]>
On 27/2/63 05:04, Adam Levy wrote:
> Hi all
>
> When I have a database connection with a temp table of the same name
> as a table in main, I am getting what I feel is unexpected behavior
> from the session extension.
>
> Most succinctly, I start a session on a connection on the main db with
> a table with an INTEGER PRIMARY KEY, attach it to all tables by
> passing NULL, make a change such as a single INSERT in aforementioned
> table, and then record a changeset. Then I invert this changeset, and
> apply that inverse to the same connection, to reverse the changes just
> made.
>
> Normally this works flawlessly. However, if a table by the same name
> exists in the temp database on that connection, I run into odd
> behavrior.
>
> If the temp table has different rows than the main table, a very odd
> changeset conflict occurs. The conflict type is SQLITE_CHANGESET_DATA
> but when I examine the changeset iter the values for the columns, they
> match the conflicting values. The inverse of an INSERT is a DELETE so
> this conflict type indicates that one of the non-primary key column
> values doesn't match, but in every examination of the database and the
> changeset I have performed they do match. The primary key exists and
> matches as well.
>
> If the temp table has the same rows at the main table (what the
> changeset expects), the changeset_apply does not raise a conflict, but
> it takes no affect on the main table.

Thanks for reporting this. As far as I can figure, it's a bug in 
sqlite3changeset_apply(), not sqlite3session_attach(). Now fixed here:

   https://sqlite.org/src/info/f71a13d072398c9f

If you get the chance, please check if this fixes your test script and 
let us know if it does not.

> This can all be avoided by being explicit about the database when
> calling sqlite3session_attach.
> For example passing "*" or "main.*" or "main.<table>" avoids the
> issue.

I think that's just because sqlite3session_attach() doesn't understand 
any of those shorthands. If you pass "*", it searches for a table named 
"*", not a table that matches the glob pattern "*".

Thanks,

Dan.





> But it occurs if NULL is passed. This is not obvious and
> arguably not expected given that the documentation about the Session
> extension makes it seem like it should be already scoped to a single
> database on a connection. After all, sqlite3session_create accepts the
> database as an argument. Also, sqlite3_changeset_apply appears to be
> specific to the "main" database. So it shouldn't care about whats in
> the temp database at all.
>
>  From all of the docs I have read on the Session extension this seems
> like misbehavior at the level of sqlite3session_attach being passed
> NULL for the zTab argument. At the minimum perhaps a note in the
> documentation of sqlite3session_attach would be appropriate.
>
> Below is a very simple example that can reproduce this behavior. It
> has a number of lines commented out that describe how the bug can be
> avoided to caused.
>
> Its written in Golang, but the library it uses is just a very thin
> wrapper around the C interface. It is using the latest 3.31.1
> amalgamation. I help maintain the SQLite library that it uses and can
> confirm that everything being called here is a very thin wrapper
> around the C interface. I could write a C example but it would take me
> more work.
>
> The library it calls to print the SQL of the changeset is a utility I
> wrote that is just using a thin wrapper around the changeset_iter to
> parse the changeset into human readable SQL. It of course is not aware
> of database names so it doesn't print "main" but this SQL is just for
> displaying to the user. The values returned by
> sqlite3changeset_conflict are formatted into the comments of that
> printed SQL.
>
> The code can be more easily downloaded here:
> https://github.com/AdamSLevy/sqlite-session-bug
>
> // main.go
>
> package main
>
> import (
>      "bytes"
>      "fmt"
>      "io"
>      "log"
>
>      "crawshaw.io/sqlite"
>      "crawshaw.io/sqlite/sqlitex"
>      "github.com/AdamSLevy/sqlitechangeset"
> )
>
> func run() error {
>      conn, err := sqlite.OpenConn(":memory:", 0)
>      if err != nil {
>          return fmt.Errorf("sqlite open: %w", err)
>      }
>      defer conn.Close()
>
>      fmt.Println("Creating tables...")
>      err = sqlitex.ExecScript(conn, `
> CREATE TABLE main.t(id INTEGER PRIMARY KEY, a,b,c);
> CREATE TABLE temp.t(id INTEGER PRIMARY KEY, a,b,c) --- remove this
> line or rename the table to avoid issue;
> `)
>      if err != nil {
>          return err
>      }
>      fmt.Println("Starting session on main...")
>      sess, err := conn.CreateSession("main")
>      if err != nil {
>          return err
>      }
>      defer sess.Delete()
>      // An empty string will pass NULL to sqlite3session_attach and allow
>      // the bug.
>      var attach string
>      // Any of these prevent the bug.
>      //attach = "*"
>      //attach = "main.*"
>      //attach = "main.t"
>      fmt.Printf("Attaching to %s ...\n", attach)
>      if err := sess.Attach(attach); err != nil {
>          return err
>      }
>
>      fmt.Println("Inserting into main.t ...")
>      commit := sqlitex.Save(conn)
>      err = sqlitex.ExecScript(conn, `
> INSERT INTO main.t(a,b,c) VALUES (1,2,3);
> INSERT INTO temp.t(a,b,c) VALUES (1,2,3) --- This line avoids the
> conflict, but the changeset doesn't apply to main.t like expected;
> `)
>      if err != nil {
>          return err
>      }
>      commit(&err)
>
>      sql, err := sqlitechangeset.SessionToSQL(conn, sess)
>      if err != nil {
>          return err
>      }
>      fmt.Println("changeset:", sql)
>
>      chgset := bytes.NewBuffer(nil)
>      if err := sess.Changeset(chgset); err != nil {
>          return err
>      }
>      invrt := bytes.NewBuffer(nil)
>      invrtCp := bytes.NewBuffer(nil)
>      if err := sqlite.ChangesetInvert(
>          io.MultiWriter(invrt, invrtCp), chgset); err != nil {
>          return err
>      }
>
>      sql, err = sqlitechangeset.ToSQL(conn, invrtCp)
>      if err != nil {
>          return err
>      }
>      fmt.Println("inverted changeset:", sql)
>
>      /*// Dropping the temp table avoids the issue...
>      fmt.Println("Dropping temp table...")
>          err = sqlitex.ExecScript(conn, `
>      DROP TABLE temp.t;
>      `)
>          if err != nil {
>              return err
>          }
>      */
>
>      fmt.Println("applying inverted changeset...")
>      conflictFn := func(cType sqlite.ConflictType,
>          iter sqlite.ChangesetIter) sqlite.ConflictAction {
>          fmt.Println("ConflictType:", cType)
>          sql, err := sqlitechangeset.ConflictChangesetIterToSQL(conn, iter)
>          if err != nil {
>              fmt.Println(err)
>          }
>          fmt.Println("Conflict:", sql)
>          return sqlite.SQLITE_CHANGESET_ABORT
>      }
>
>      if err := conn.ChangesetApply(invrt, filterFn, conflictFn); err != nil {
>          return err
>      }
>      fmt.Println("done.")
>
>      sql, err = sqlitechangeset.SessionToSQL(conn, sess)
>      if err != nil {
>          return err
>      }
>      if len(sql) > 0 {
>          return fmt.Errorf("Error: current session changeset not empty: %v", sql)
>      }
>      fmt.Println("success")
>
>      return nil
> }
>
> func filterFn(string) bool { return true }
>
> func main() {
>      if err := run(); err != nil {
>          log.Fatal(err)
>      }
> }
> _______________________________________________
> sqlite-users mailing list
> [email protected]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
_______________________________________________
sqlite-users mailing list
[email protected]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users