kndndrj / nvim-dbee

Interactive database client for neovim
GNU General Public License v3.0
710 stars 48 forks source link

request: cancel query. #27

Closed hobord closed 7 months ago

hobord commented 1 year ago

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

kndndrj commented 1 year ago

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

MattiasMTS commented 1 year ago

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 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

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:

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 commented 7 months ago

This has been addressed in refactor pull request. Closing.