Dan, I tried the above patch and it resolves the issue! Also you were right about use of "*" in attach. I misinterpreted the output of my test in that case.
Thank you so much for taking the time to fix this! When can I expect this patch to make it into a release? Adam Levy On Thu, Feb 27, 2020 at 8:23 AM Dan Kennedy <danielk1...@gmail.com> wrote: > > > 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 > > sqlite-users@mailinglists.sqlite.org > > http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > _______________________________________________ > sqlite-users mailing list > sqlite-users@mailinglists.sqlite.org > http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users _______________________________________________ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
