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

request: cancel query. #27

Closed
hobord opened this issue Jul 3, 2023 · 6 comments
Closed

request: cancel query. #27

hobord opened this issue Jul 3, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@hobord
Copy link

hobord commented Jul 3, 2023

Add option to cancel long run query. context cancelation...

@hobord hobord changed the title cancel query. request: cancel query. Jul 3, 2023
@kndndrj
Copy link
Owner

kndndrj commented Jul 3, 2023

makes sense, will look into it once I have some time.

@kndndrj kndndrj added the enhancement New feature or request label Jul 3, 2023
@MattiasMTS
Copy link
Collaborator

I plan to add this to this branch https://github.com/MattiasMTS/nvim-dbee/tree/mattias/progress_tick.

However, the ticker works as expected when the query is successful, i.e. the query function isn't exiting here https://github.com/kndndrj/nvim-dbee/blob/master/dbee/main.go#L128-L130. Because if the query is "malformed", e.g. you're querying a table that doesn't exist, then the timer will continue counting until you execute another query... :(

This is of course a behaviour we don't want. I tried to add boolean channels in the query function but that delayed the point of having go routine in the background while the frontend waits.

@kndndrj do you have any ideas on how you would implement this? I'm thinking I'd have to add this behaviour in the backend using something like:

for {
        select {
        case <-ticker.C:
          elapsed := time.Since(start)
          // Convert elapsed time to milliseconds
          elapsedTimeMillis := float64(elapsed) / float64(time.Millisecond)

          // Update the Neovim buffer with the elapsed time
          // You might need to implement this part according to your Neovim setup
          updateProgressInBuffer(elapsedTimeMillis)

          // Check if the query execution has completed
          if queryCompleted {
            logger.Debugf("%q executed successfully", method)
            err := callbacker.TriggerCallback(callbackId)
            if err != nil {
              logger.Error(err.Error())
            }
            logger.Debugf("%q successfully triggered callback", method)
            return
          }
        }
      }
    }()

    logger.Debugf("%q returned successfully", method)
    return nil
  }

and then add an option if you press <C-c> The progress bar cancels -> and the query cancels.

Perhaps adding a method that checks if the go-routine-query is still running might be worth doing, or? Polling this method with reasonable intervals would allow us to control this behaviour and we separate the control flows better.

@kndndrj
Copy link
Owner

kndndrj commented Sep 1, 2023

@kndndrj do you have any ideas on how you would implement this?

Well, afaik the best way in go to cancel something is to cancel the context. So the first thing about this is to replace all the crappy context.TODOs with the actual context, which is created on query execution and passed down the chain.

This also means that pretty much all Conn methods should recieve context as a parameter.

@MattiasMTS
Copy link
Collaborator

@kndndrj do you have any ideas on how you would implement this?

Well, afaik the best way in go to cancel something is to cancel the context. So the first thing about this is to replace all the crappy context.TODOs with the actual context, which is created on query execution and passed down the chain.

This also means that pretty much all Conn methods should recieve context as a parameter.

Are we sure enough that cancellation of the context leads to cancellation of the Query? I think this is the best general approach to it. Any idea on how you want to craft this out? Using channels, callbacks, etc :)

@kndndrj
Copy link
Owner

kndndrj commented Sep 2, 2023

If I recall correctly, the cancelation of context is interpreted by different drivers in different ways - so there is not a 100% guarantee that the query gets cancled. However I think that is also the only way of canceling a database/sql request.

as for the implementation here are my thoughts:

  • context passing part shouldn't be too hard to do
  • a new context gets created in Conn on every exexution and is stored until this query is the active one. then we can add a method to conn that cancles the current context
  • the tricky part (not necessarly just a problem with this issue) is that there can be multiple parallel queries running on the same connection. I've been thinking about a sort of "active queries" dashboard for some time, vut didnt come up with anything concrete yet.

These are just my overall thoughts, unfortunately I can't spend the time I want on this project, so it might have to wait for a while though :/

@kndndrj kndndrj mentioned this issue Dec 28, 2023
@kndndrj
Copy link
Owner

kndndrj commented Jan 4, 2024

This has been addressed in refactor pull request. Closing.

@kndndrj kndndrj closed this as completed Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants