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

No error message for invalid sql statements #73

Closed
seryj opened this issue Apr 8, 2022 · 12 comments · Fixed by #76
Closed

No error message for invalid sql statements #73

seryj opened this issue Apr 8, 2022 · 12 comments · Fixed by #76
Milestone

Comments

@seryj
Copy link
Contributor

seryj commented Apr 8, 2022

Error description
When executing an invalid query there is no feedback to the user that something went wrong. The result tab just shows the result of previous query. Also in the log tab, there is no indication that something went wrong.

An invalid query can be

  • a wrong SQL statement
  • valid SQL but with wrong table name

Expected behavior
Some kind of error message/warning. Maybe for beginning, it is possible to show the error message from DataFusion itself?

@matthewmturner
Copy link
Collaborator

Can you provide an example query? I had been working that and got it to work correctly, for example on parser errors(this is on latest main):

image

@seryj
Copy link
Contributor Author

seryj commented Apr 9, 2022

Your latest PR seems to fix this issue - cool!

Now, I have another issue. After executing an incorrect query (wrong path to the file with data), next query is not executed anymore. At least, it does not appear in the history and the error message that the path is wrong does not disappear as well (cf. screenshot)
Screenshot from 2022-04-09 11-07-47-2
.

@matthewmturner
Copy link
Collaborator

Got it - will look into.

@matthewmturner
Copy link
Collaborator

So i think its actually because of the semicolon. If you delete the semicolon after the bad query and add another one again i think it will work. I suppose the expected behavior would be keeping the semicolon status (i.e. execute after hitting enter).

@matthewmturner
Copy link
Collaborator

I will need to think about this a little more. Off the top of my head im not sure how straight forward this is to handle in a simple way as i could always need to be looking through all the editor text for a semicolon to determine if the flag should be set.

@matthewmturner
Copy link
Collaborator

matthewmturner commented Apr 10, 2022

the status is tracked in app.editor.sql_terminated. I dont think my current logic is correct but i still need to think a little more on what a good flow would be. let me know if you have any ideas.

@matthewmturner
Copy link
Collaborator

I think I have an idea. Rather than using enter in edit mode and tracking semicolon to execute the query I use enter mode in normal mode to trigger the query. If no semicolon it simply won't execute and I don't have to worry about tracking status. Maybe I could provide a message to inform user if we wanted to be extra friendly but don't think that's required. User would just have to remember to go from edit to normal mode to execute but I think that's okay. What do you think?

@seryj
Copy link
Contributor Author

seryj commented Apr 10, 2022

I wonder if one needs a semicolon at all. Right now, the editor always contains a single query and unless this changes in the future, having a semicolon is only required to recognize that the user wants to execute a query. Thinking about other tools, the same problem is often solved by using another key combination e.g. "shift+enter". Could that maybe be a more user-friendly solution?

On the pro-side, if chosen carefully, this "execute query" key combination could be used in the normal and edit modes such that the users will do less mistakes.

On the contra-side, the complexity of key handling increases.

How do you see it?

@seryj seryj closed this as completed Apr 10, 2022
@seryj seryj reopened this Apr 10, 2022
@seryj
Copy link
Contributor Author

seryj commented Apr 10, 2022

sorry, pressed wrong button and closed the issue accidently

@matthewmturner
Copy link
Collaborator

I had in mind that people could do DDL plus a query in the same editor which would require the semicolon in the editor. i thought there was value in this for interactive data analysis and it provides a good interface before doing DDL through ~/.datafusion.

I definitely also had an interest in something like shift-enter, as i use that in datagrip all the time. However, i think that would be non-trivial to implement (although i do have an idea for this already). But i will create a separate issue for this point.

Switching to only executing in normal mode i think provides a good tactical solution to this issue for 0.1 release (thats my focus right now) while a longer term solution can be investigated further. hope thats ok!

@seryj
Copy link
Contributor Author

seryj commented Apr 11, 2022

Sure - good idea! And you are absolutely right about caring more about 0.1 release. If one does not stop thinking about improvements, one will never come to a release... this happens all the time to me. :)

@matthewmturner matthewmturner linked a pull request Apr 12, 2022 that will close this issue
@matthewmturner
Copy link
Collaborator

aha same thing happens to me - thats why i forced myself to draw a line in the sand and get a first cut out.

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 a pull request may close this issue.

2 participants