Skip to content

Conversation

@detule
Copy link
Owner

@detule detule commented Oct 26, 2023

No description provided.

meztez and others added 4 commits September 27, 2023 09:49
Compare to upstream a94cb6a3d236a4b5103442edf7e2b718c3660dde
Only set the current result to null if we are cleaning
up the current result.

What happens is that when a result is not cleaned up and then you make
another query, then the first result set is not freed, but it is left
dangling, with a reference to the connection:
https://github.com/r-dbi/odbc/blob/a0ba73c72ca8f4cf33181fead7644b9b90aa2d4c/src/odbc_connection.cpp#L19

We test that the first result is invalidated in DBItest, here:
https://github.com/r-dbi/DBItest/blob/2c29217bdd0b3314e6ec65216d99a3bf83a63464/R/spec-result-send-query.R#L92
So after that test there is still a result object, until the garbage
collector runs next.

If the garbage collector happens to run when there is another
active result for the same connection, we remove that result from
the connection, even though it is not the result we are finalizing.

The workaround in this PR check that we are finalizing the active
result in the destructor.

A better solution would be to finalize the old result when a new one
is set, but that seemed more difficult with the current code base.
* Update to testthat 3e

* Eliminate some expect_is() calls

* fixup tests

* SQL Server: Index local temp tables (r-dbi#600)

Update handling ( listing, writing ) of local temp tables.

---------

Co-authored-by: Hadley Wickham <[email protected]>

* SQL Server: Handle temporary flag in sqlCreateTable (r-dbi#601)

* SQL Server: Handle temporary flag in sqlCreateTable

* code-review: better warning + testthat usage

* code-review: simplify sql server specific method

* code-review: Add missing _snaps

* Update news

---------

Co-authored-by: detule <[email protected]>
@detule detule force-pushed the fixup/spark_schemas branch from 1f6bbac to 3c8f82e Compare October 26, 2023 12:29
@detule detule force-pushed the fixup/spark_schemas branch from 3c8f82e to 2c44151 Compare October 26, 2023 12:32
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.

5 participants