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

"Invalid SQL" page should let you edit the SQL #619

Closed
simonw opened this issue Nov 10, 2019 · 14 comments
Closed

"Invalid SQL" page should let you edit the SQL #619

simonw opened this issue Nov 10, 2019 · 14 comments

Comments

@simonw
Copy link
Owner

simonw commented Nov 10, 2019

https://latest.datasette.io/fixtures?sql=select%0D%0A++*%0D%0Afrom%0D%0A++%5Bfoo%5D

Would be useful if this page showed you the invalid SQL you entered so you can edit it and try again.

@davidszotten
Copy link

just trying out datasette and quite like it, thanks! i found this issue annoying enough to have a go at a fix. have you any thoughts on a good approach? (i'm happy to dig in myself if you haven't thought about it yet, but wanted to check if you had an idea for how to fix when you raised the issue)

@obra
Copy link

obra commented Sep 23, 2020

I've just run into this after crafting a complex query and discovered that hitting back loses my query.

Even showing me the whole bad query would be a huge improvement over the current status quo.

@simonw
Copy link
Owner Author

simonw commented Sep 23, 2020

Yeah that sucks. Bumping this up the priority list.

@simonw simonw pinned this issue Sep 23, 2020
@simonw simonw added the bug label Sep 23, 2020
@simonw
Copy link
Owner Author

simonw commented Sep 23, 2020

This is a little tricky to solve, because of the location of the form and the need to return JSON as well as HTML. It would be weird if a JSON request came in and got back the standard output from https://latest.datasette.io/fixtures.json when they were expecting to get back JSON in the shape of https://latest.datasette.io/fixtures.json?sql=select%20*%20from%20sqlite_master

I'm going to return the HTML view that you would get for 0 results for a query - https://latest.datasette.io/fixtures?sql=select%201%20limit%200 - but with an error message.

@simonw
Copy link
Owner Author

simonw commented Sep 23, 2020

So the JSON (still served with a 500 code) will look something like this:

{
  "ok": false,
  "status": 500,
  "database": "fixtures",
  "query_name": null,
  "rows": [],
  "truncated": false,
  "error": "Error message goes here",
  "columns": [],
  "query": {
    "sql": "the query that broke goes here",
    "params": {}
  },
  "private": false,
  "allow_execute_sql": true,
  "query_ms": 0.8716583251953125,
  "source": "tests/fixtures.py",
  "source_url": "https://github.com/simonw/datasette/blob/master/tests/fixtures.py",
  "license": "Apache License 2.0",
  "license_url": "https://github.com/simonw/datasette/blob/master/LICENSE"
}

@simonw
Copy link
Owner Author

simonw commented Sep 23, 2020

I'll add this to the succesful JSON format:

{
  "ok": true,
  "error": null
}

@simonw
Copy link
Owner Author

simonw commented Sep 23, 2020

I'm going to have to untangle Datasette's error handling a bit for this - currently the expectation is that exceptions will be handled at a higher level, but I need to rethink that to make it cleaner for views like the "execute custom SQL" view to add their own error handling (and still be able to return the correct HTTP status codes, even with custom pages).

@simonw
Copy link
Owner Author

simonw commented Feb 19, 2021

Big usability improvement, see also #1236

@simonw
Copy link
Owner Author

simonw commented May 28, 2021

Came up on discussions here: #1334

@simonw
Copy link
Owner Author

simonw commented May 28, 2021

I'm going to return a 400 HTTP status code for "bad request", under the assumption that the client sent bad SQL.

@simonw
Copy link
Owner Author

simonw commented May 28, 2021

I nearly got this working, but I ran into one last problem: the code path for when an error is raised but the user specified ?_shape=array. I'll open a draft PR with where I've got to so far.

@simonw
Copy link
Owner Author

simonw commented May 28, 2021

Remaining work will happen in #1346

@simonw
Copy link
Owner Author

simonw commented Jun 2, 2021

A SQL error now looks like this: https://latest.datasette.io/fixtures?sql=select%0D%0A++*%0D%0Afrom%0D%0A++%5Bfoo%5D

fixtures__select___from__foo_

I'm going to get rid of that "0 results" message if an error is shown.

@simonw simonw reopened this Jun 2, 2021
@simonw
Copy link
Owner Author

simonw commented Jun 2, 2021

fixtures__select___from__foo__and_Facets_should_not_execute_for__shape_array_object_·_Issue__263_·_simonw_datasette

@simonw simonw closed this as completed Jun 2, 2021
@simonw simonw unpinned this issue Jun 5, 2021
simonw added a commit that referenced this issue Jun 5, 2021
@simonw simonw removed this from the Datasette Next milestone Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants