Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

current_query() crashes CLI and Python kernel #3029

Closed
2 tasks done
Alex-Monahan opened this issue Feb 3, 2022 · 6 comments
Closed
2 tasks done

current_query() crashes CLI and Python kernel #3029

Alex-Monahan opened this issue Feb 3, 2022 · 6 comments
Assignees

Comments

@Alex-Monahan
Copy link
Contributor

Alex-Monahan commented Feb 3, 2022

What happens?

(As a note, I found this when doing documentation - I am not actively using this!)
This statement causes DuckDB to crash on either the CLI (0.3.2-dev1304) or the Python client (0.3.2-dev1201). It does not fail, but does not return the correct result in version 0.3.1 in the Python client.

SELECT current_query();

However, this query is a part of a test here, so I don't know how it is failing.

Here is a Python example:

import duckdb
new_duckdb = duckdb.connect()
new_duckdb.execute("SELECT 1, current_query() as my_column").fetchdf()

The Python 0.3.1 client returns:

1 my_column
1

To Reproduce

Run the below command on the 0.3.2 dev version and it will fail:

SELECT current_query();

Environment (please complete the following information):

  • OS: Windows 10
  • DuckDB CLI (0.3.2-dev1304), and Python 3.9.7 (0.3.2-dev1201)

Before Submitting

  • Have you tried this on the latest master branch?

  • Have you tried the steps to reproduce? Do they include all relevant data and configuration? Does the issue you report still appear there?

@hawkfish hawkfish self-assigned this Feb 3, 2022
@hawkfish
Copy link
Contributor

hawkfish commented Feb 3, 2022

Repros on OSX too. But not in the test harness.

@rsund
Copy link
Contributor

rsund commented Feb 4, 2022

The following crashes also with R client (0.3.2-dev1310) on Windows 10, but not on Ubuntu 20.04:

con <- DBI::dbConnect(duckdb::duckdb())
DBI::dbGetQuery(con, "SELECT current_query();") 

FYI, on Ubuntu, the following behavior is seen (i.e. current_query() appears to point to the previous query):

con <- DBI::dbConnect(duckdb::duckdb())
DBI::dbGetQuery(con, "SELECT 1;") 
#>   1
#> 1 1
DBI::dbGetQuery(con, "SELECT current_query();") 
#>   current_query()
#> 1       SELECT 1;
DBI::dbGetQuery(con, "SELECT 1, 2, 3, current_query();")
#>   1 2 3         current_query()
#> 1 1 2 3 SELECT current_query();
DBI::dbGetQuery(con, "SELECT current_query();") 
#>                    current_query()
#> 1 SELECT 1, 2, 3, current_query();

Created on 2022-02-04 by the reprex package (v2.0.1.9000)

Just a guess, but maybe the "previous query" resides on a (partially) non-protected memory area and is likely to crash with bad luck that may also depend on the compiler version (or to avoid crash with good luck as is probably the case while running tests).

@Mytherin
Copy link
Collaborator

Mytherin commented Feb 5, 2022

@hawkfish I can't seem to reproduce this on the latest master, neither in Python nor in the SQLite shell. Did you have any luck? If so could you have a look at fixing this?

@hawkfish
Copy link
Contributor

hawkfish commented Feb 5, 2022

I can reproduce it on master under Xcode running the console debug build. ClientContext::active_query is nullptr. ClientContext::PrepareInternal is trying to do constant folding, the value is not there and so we get an assertion failure in ClientContext::GetCurrentQuery.

@hawkfish
Copy link
Contributor

hawkfish commented Feb 5, 2022

Proposed fix is to mark current_query() as having side effects.

hawkfish pushed a commit to hawkfish/duckdb that referenced this issue Feb 5, 2022
Since the value is not always available in some contexts,
marking the function as having side effects
prevents it from being computed during preparation.
hawkfish pushed a commit to hawkfish/duckdb that referenced this issue Feb 5, 2022
Mytherin added a commit that referenced this issue Feb 6, 2022
@Mytherin
Copy link
Collaborator

Mytherin commented Feb 7, 2022

Should be fixed with #3042. Feel free to re-open if the issue persists.

@Mytherin Mytherin closed this as completed Feb 7, 2022
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

No branches or pull requests

4 participants