query_runner -> query_results: improve logging, handle unhandled data types #6905

vtatarin · 2024-04-18T00:19:17Z

What type of PR is this?

Description

enhance debug logging for QueryResults data source
fix an error during running queries with QueryResults data source, similar to Query Results: querying a column with a dictionary or array fails #3887

[2024-04-17 23:26:24,952][PID:227][DEBUG][redash.query_runner.query_results] CREATE TABLE query: CREATE TABLE query_10000 ("a", "b", "c", "d", "e", "f", "g", "h", "j", "k", "l", "m", "n", "b", "v", "c", "x", "z", "q", "w", "e", "r", "t", "y", "u", "i", "o", "p", "q", "w", "e", "r", "t", "y", "u")
[2024-04-17 23:26:24,953][PID:227][WARNING][rq.job.redash.tasks.queries.execution] job.func_name=redash.tasks.queries.execution.execute_query job.id=0ae029e6-27ef-4482-93fc-1e7498cf08f7 Unexpected error while running query:
Traceback (most recent call last):
  File "/app/redash/tasks/queries/execution.py", line 182, in run
    data, error = query_runner.run_query(annotated_query, self.user)
  File "/app/redash/query_runner/query_results.py", line 172, in run_query
    create_tables_from_query_ids(user, connection, query_ids, query_params, cached_query_ids)
  File "/app/redash/query_runner/query_results.py", line 100, in create_tables_from_query_ids
    create_table(connection, table_name, results)
  File "/app/redash/query_runner/query_results.py", line 140, in create_table
    connection.execute(insert_template, values)
sqlite3.InterfaceError: Error binding parameter 6 - probably unsupported type.

Values:

[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: 18227135, Type: <class 'int'>
[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: redacted, Type: <class 'str'>
[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07, Type: <class 'datetime.date'>
[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07, Type: <class 'datetime.date'>
[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-06, Type: <class 'datetime.date'>
[2024-04-18 03:40:40,086][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-01, Type: <class 'datetime.date'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 12:52:54, Type: <class 'datetime.time'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 12:57:38, Type: <class 'datetime.time'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07 12:52:54+00:00, Type: <class 'datetime.datetime'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07 12:57:38+00:00, Type: <class 'datetime.datetime'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07 20:52:54+00:00, Type: <class 'datetime.datetime'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: 2023-03-07 20:57:38+00:00, Type: <class 'datetime.datetime'>
[2024-04-18 03:40:40,087][PID:118][DEBUG][redash.query_runner.query_results] Value: redacted, Type: <class 'str'>

How is this tested?

Unit tests (pytest, jest)
E2E Tests (Cypress)
Manually
N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

justinclift · 2024-04-18T07:20:04Z

What type of data source is it trying to use when this happens, and which version of Redash is it happening on? 😄

AndrewChubatiuk · 2024-04-18T08:13:19Z

@justinclift this is a version from master

justinclift · 2024-04-18T08:22:04Z

@AndrewChubatiuk Ahhh. Do you want to take a look at it then? You've probably been doing stuff with that part of things recently, whereas I haven't (yet). 😄

vtatarin · 2024-04-18T18:01:31Z

@justinclift that's from QueryResults data source

…nner

vtatarin · 2024-04-19T23:27:07Z

Hello @justinclift @AndrewChubatiuk
I've tested my changes in our installation Redash running from the latest master branch- it fixes the issue.

Please review this change. Thank you!

redash/query_runner/query_results.py

AndrewChubatiuk · 2024-04-20T05:21:13Z

LGTM
please just update PR name and fix tests

redash/query_runner/query_results.py

Co-authored-by: Andrii Chubatiuk <andrew.chubatiuk@gmail.com>

codecov · 2024-04-29T22:36:26Z

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 63.77%. Comparing base (372adfe) to head (5e69e3f).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6905   +/-   ##
=======================================
  Coverage   63.76%   63.77%           
=======================================
  Files         161      161           
  Lines       13085    13089    +4     
  Branches     1812     1814    +2     
=======================================
+ Hits         8344     8347    +3     
  Misses       4438     4438           
- Partials      303      304    +1

Files	Coverage Δ
redash/query_runner/query_results.py	`63.57% <80.00%> (+0.33%)`	⬆️

vtatarin · 2024-04-30T04:54:06Z

hey @justinclift @AndrewChubatiuk
please review and merge this PR, all checks should be green now. It's an important fix for my team

justinclift · 2024-04-30T05:36:32Z

redash/query_runner/query_results.py

@@ -134,6 +137,7 @@ def create_table(connection, table_name, query_results):
        column_list=column_list,
        place_holders=",".join(["?"] * len(columns)),
    )
+    logger.debug("INSERT template: %s", insert_template)


Pretty sure we don't want debugging statements being unconditionally run. At least with the change above it, it seems to be done conditionally there.

@justinclift thank you for the reply! So there are 2 things:

logger only logs (prints to stdout/stderr) messages, nothing is actually run/executed. The default log level is INFO, so logger.debug is actually a condition which logs the data only at a more verbose level

for another added logger there is a condition that checks the current log level and prevents going through too many checks unless its DEBUG (which is not default)

ping @justinclift

@vtatarin Sorry, but I'm short on time for the next few weeks.

Am getting some (Redash related) stuff deployed to a data centre, and that's taking the majority of my focus time. When that's done I'll be able to look at PRs properly. 😄

vtatarin added 6 commits April 17, 2024 17:11

query_runner: add debug log

aa7b93c

query_runner: add debug log

fb4163c

handle datetime.time

4e7e468

query_runner: add debug log

11e4f0b

fmt

1f4853e

add debug

e0ae3ba

vtatarin added 9 commits April 18, 2024 19:50

athena ds: fallback to default creds if RoleArn is not provided

051ed07

pull changes

71e433a

pull changes

17c00d6

Merge branch 'master' of github.com:vtatarin/redash into fix-query-ru…

512bde7

…nner

expose error messae

be111d3

expose error messae

26b5f7c

revert additional changes

8d155bf

revert additional changes

374e6bf

revert additional changes

1a7f204

vtatarin changed the title ~~[WIP] Fix query_runner~~ Fix query_runner Apr 19, 2024

AndrewChubatiuk reviewed Apr 20, 2024

View reviewed changes

redash/query_runner/query_results.py Outdated Show resolved Hide resolved

AndrewChubatiuk reviewed Apr 21, 2024

View reviewed changes

redash/query_runner/query_results.py Outdated Show resolved Hide resolved

vtatarin and others added 3 commits April 29, 2024 15:27

Update redash/query_runner/query_results.py

d22df7c

Co-authored-by: Andrii Chubatiuk <andrew.chubatiuk@gmail.com>

review edits

30776b3

Merge branch 'getredash:master' into fix-query-runner

c9feda5

vtatarin added 2 commits April 30, 2024 00:53

add tests

a181ba9

add tests

d3f8cad

vtatarin added 2 commits April 30, 2024 02:32

fmt

0778b71

fix

a8023fb

vtatarin changed the title ~~Fix query_runner~~ query_runner -> query_results: improve logging, handle unhandled data types Apr 30, 2024

vtatarin added 2 commits April 30, 2024 03:32

fix lint

4e3cbdb

fix lint

5e69e3f

justinclift reviewed Apr 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

query_runner -> query_results: improve logging, handle unhandled data types #6905

query_runner -> query_results: improve logging, handle unhandled data types #6905

vtatarin commented Apr 18, 2024 •

edited

justinclift commented Apr 18, 2024

AndrewChubatiuk commented Apr 18, 2024

justinclift commented Apr 18, 2024

vtatarin commented Apr 18, 2024

vtatarin commented Apr 19, 2024

AndrewChubatiuk commented Apr 20, 2024 •

edited

codecov bot commented Apr 29, 2024 •

edited

vtatarin commented Apr 30, 2024

justinclift Apr 30, 2024

vtatarin Apr 30, 2024

vtatarin May 9, 2024

justinclift May 10, 2024

query_runner -> query_results: improve logging, handle unhandled data types #6905

Are you sure you want to change the base?

query_runner -> query_results: improve logging, handle unhandled data types #6905

Conversation

vtatarin commented Apr 18, 2024 • edited

What type of PR is this?

Description

How is this tested?

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

justinclift commented Apr 18, 2024

AndrewChubatiuk commented Apr 18, 2024

justinclift commented Apr 18, 2024

vtatarin commented Apr 18, 2024

vtatarin commented Apr 19, 2024

AndrewChubatiuk commented Apr 20, 2024 • edited

codecov bot commented Apr 29, 2024 • edited

Codecov Report

vtatarin commented Apr 30, 2024

justinclift Apr 30, 2024

Choose a reason for hiding this comment

vtatarin Apr 30, 2024

Choose a reason for hiding this comment

vtatarin May 9, 2024

Choose a reason for hiding this comment

justinclift May 10, 2024

Choose a reason for hiding this comment

vtatarin commented Apr 18, 2024 •

edited

AndrewChubatiuk commented Apr 20, 2024 •

edited

codecov bot commented Apr 29, 2024 •

edited