[R-sig-DB] RSQLite: ATTACH statement not executed when the db connection is holding a resultSet

[Herve Pages]

Hi Seth,

Seth Falcon wrote:
> Hi Herve,
> 
> Herve Pages <hpages using fhcrc.org> writes:
>> OK I get the warning now:
>>
>>   library(RSQLite)
>>   db1 <- dbConnect(SQLite(), "db1.sqlite")
>>   dbGetQuery(db1, "CREATE TABLE t1 (a integer, aa text)")
>>   db2 <- dbConnect(SQLite(), "db2.sqlite")
>>   dbGetQuery(db2, "CREATE TABLE t2 (b integer, bb text)")
>>   dbGetQuery(db2, "ATTACH 'db1.sqlite' AS db1")
>>   dbSendQuery(db2, "SELECT * FROM db1.t1")
>>
>>   > dbGetQuery(db2, "SELECT * FROM db1.t1")
>>   Error in sqliteExecStatement(new.con, statement, bind.data) :
>>     RS-DBI driver: (error in statement: no such table: db1.t1)
>>   In addition: Warning message:
>>   In sqliteQuickSQL(conn, statement, ...) :
>>     There is an open, incomplete result set; executing query on a temporary connection
>>
>>   > dbGetQuery(db2, "DETACH db1")
>>   Error in sqliteExecStatement(new.con, statement, bind.data) :
>>     RS-DBI driver: (RS_SQLite_exec: could not execute1: no such database: db1)
>>   In addition: Warning message:
>>   In sqliteQuickSQL(conn, statement, ...) :
>>     There is an open, incomplete result set; executing query on a temporary connection
>>
>> but dbGetQuery() doesn't seem to be reporting what one would expect.
> 
> I'm not sure what you mean.  What do you expect dbGetQuery to report?
> 
> The above looks "right" to me.  You got a warning that told you that
> your ATTACH occured on a temp connection.

Are you sure? My understanding is that the ATTACH didn't occur on a temp connection
but really occurred on the db2 connection itself (I mean the real one). It's my
dbGetQuery(db2, "SELECT * FROM db1.t1") that was sent later thru a temporary connection
and thus was not aware that db1 was attached to db2. Or am I missing something?

>  Since ATTACH is called for
> its side-effect on the _connection_ this means your ATTACH was
> useless.

I agree. But the ATTACH could have been sent a long time before (many queries before
in the query history), and not necessarily by me, so I don't really know the who/how/when
of it. And then suddenly, db1 doesn't seem to be attached to db2 anymore, even if everything
so far seemed to indicate that it was attached (let's say I've sent
dbGetQuery(db2, "SELECT * FROM db1.t1") many times before with no problems because there
was no pending results in db2).

So we have a situation where depending on whether I use dbGetQuery() or dbSendQuery() and
whether there is a pending result set or not, db1 will sometimes appear as attached to db2,
and sometimes not. That's what I mean by dbGetQuery() not reporting what one would expect.

Maybe the motivation behind the temporary connection "feature" feature was to provide
some convenience to the user so s/he can do:

  dbGetQuery(db2, myquery)

even if there is a pending result set. But then it should not affect the semantic of
dbGetQuery() i.e. one might expect the same result as with:

  ## clear all pending results first
  while (length(dbListResults(db2)) >= 1) dbClearResult(dbListResults(db2)[[1]])
  ## send the query
  dbGetQuery(db2, myquery)

which is not the case.

I understand that this is due to the temporary connection "feature" and that it might
look right to anybody who knows all the internal gears, but still...

BTW, according to the documentation it seems that the 'cache_size' and 'synchronous'
values that the user can specify when opening an SQLite connection with dbConnect()
are not used for the temporary connection. Any reason behind this?


> 
>> Even if there is a warning now, I still find the "temporary connection"
>> feature confusing...
> 
> Yes, the temporary connection "feature" _is_ confusing.  I would like
> to remove this feature and have dbGetQuery behave like dbSendQuery:
> error if there is an open and incomplete result set, close and process
> a complete, but open result set, and encourage users to explicitly
> close all result sets that they open.

Sounds good.

Thanks,
H.

> 
> I haven't done this yet, because making non-backwards compatible
> changes should be done with some thought ... and some time for
> interested parties to weigh in... anyone?
> 
> + seth
>