Re: [sqlite] Issues with sqlite3session_attach(conn, NULL) w/ duplicate table in temp
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 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." 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
Re: [sqlite] Issues with sqlite3session_attach(conn, NULL) w/ duplicate table in temp
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." 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
