redash icon indicating copy to clipboard operation
redash copied to clipboard

Error message on insert/update/delete queries

Open jonahgreenthal opened this issue 1 year ago • 6 comments

Issue Summary

When a insert, update, or delete query is run in Redash, the result is an error message: image The query does not actually run, and the error message is completely inscrutable.

If the query is followed by a select query, like this—

update Contact set id=1 where id=1;
select 1;

—then the second query runs and returns the expected value, but the first query still doesn't actually run.

Here are some better ways this could be handled, from best to worst:

  1. When an insert, update, or delete query is run, actually run it and provide feedback like "1 row affected". Also support insert…output (SQL Server) and analogous operations with other DBMSes (update...returning) in which data are returned from a mutation.
  2. When an insert, update, or delete query is run, actually run it and provide no feedback
  3. When an insert, update, or delete query is run, actually run it, but still provide an error message because no results are returned (Redash 8 with Postgres behaved this way. I don't like it, but it's still better to run the query than not.)
  4. When an insert, update, or delete query is run, provide an error message that tells the user what's going on.

This occurs in all environments I have tested, using Redash 10.1.0.b50633 via the Docker image with a Microsoft SQL Server data source.

Steps to Reproduce

  1. Run a query like update Contact set id=1 where id=1;.
  2. Observe the error.

Expected behavior: The query runs as instructed, and there is no error message.

Technical details:

  • Redash Version: 10.1.0.b50633 via the Docker image.
  • Browser/OS: Firefox 130.0.1 on Windows 10, but I don't think it matters
  • How did you install Redash: Docker image

jonahgreenthal avatar Sep 26 '24 22:09 jonahgreenthal

Interesting. Redash is generally used as a reporting tool (ie SELECT queries), but I could see that being able to stuff that changes the database would indeed be useful.

This is probably an area of things that could be looked at and improved to support doing so more.

justinclift avatar Sep 26 '24 23:09 justinclift

Yeah, I realize it's not the normal use case, but I think it's worth supporting. We use our Redash installation not just for reporting, but also as a way to grant certain people access to the database (preferably including insert, update, and delete) without having to deal with SSH keys, establishing a tunnel, etc.

jonahgreenthal avatar Sep 27 '24 01:09 jonahgreenthal

We often run UPDATE ... RETURNING in queries, so I would consider this to be a normal use case. When using a PostgreSQL data source the following is returned if the query does not return a recordset:

Error running query: Query completed but it returned no data.

@jonahgreenthal what behavior are you looking for? I think expecting 0 or more rows in return is reasonable, but there is doubtless some better error handling that could be implemented for some data sources.

eradman avatar Oct 14 '24 14:10 eradman

I listed my preferences in the numbered list in my original message. Talking about update...returning is a good point, and I incorporated that.

jonahgreenthal avatar Oct 14 '24 14:10 jonahgreenthal

Ah, I overlooked the numbered list you posted. So for (1) I think you're suggesting that if the result list contains an expected result, check cursor.rowcount for number of affected rows and display that instead.

eradman avatar Oct 14 '24 14:10 eradman

I'm not sure how it would be implemented, but that certainly sounds plausible.

jonahgreenthal avatar Oct 14 '24 14:10 jonahgreenthal