Sql sessions have correct history manager #18933
Conversation
sjanusz-r7
force-pushed
the
sql-sessions-have-correct-history-manager
branch
from
1a31f2d to
8e1074a
Compare
| name = session.type | ||
| query = {} | ||
|
|
||
| # Multiline (Reline) and fallback (Readline) have separate history contexts as they are two different libraries. |
There was a problem hiding this comment.
Just curious, what's the need for both libraries? If one session uses both at different times, would those two histories be merged somewhere/should they be/can they be?
There was a problem hiding this comment.
what's the need for both libraries?
Readline is our current library for prompting the user for input and providing tab suggestions on the command line; We're now using the Ruby Reline library (https://github.com/ruby/reline) for multi-line support for SQL queries
In the future we might only have reline - but that's not a blocker for this PR
| clear_readline | ||
| if File.exist?(history_file) | ||
| File.readlines(history_file).each do |e| | ||
| ::Readline::HISTORY << safe_undump(e.chomp) |
There was a problem hiding this comment.
I think this might be a weird hack/workaround; Maybe we should just align with the approach chosen by IRB (Which is coincidentally also backed by different backends of reline/readline)
https://github.com/ruby/irb/blob/1a1fbba020f1f504b2cb4d832ee4aa7af6aee0d3/lib/irb/history.rb#L71
sjanusz-r7
force-pushed
the
sql-sessions-have-correct-history-manager
branch
from
8e1074a to
e2814d6
Compare
|
We might want to start putting all these history files into a single folder - as well as maybe having 
But looks good to me 👍 |
Release NotesUpdates the new SQL session types to correctly remember previous commands that the user has entered |
| def store_history_file(history_file) | ||
| return unless readline_available? | ||
| cmds = [] | ||
| history_diff = ::Readline::HISTORY.length < MAX_HISTORY ? ::Readline::HISTORY.length : MAX_HISTORY |
There was a problem hiding this comment.
Looks like we've lost this logic, which is causing memory issues here: #19098
I think we should look into investigating and solving the underlying issue here instead of the 2000 history limit workaround, but we can tackle that separately
This PR addresses the interactive query mode not having correctly separated input history between various SQL session types.
Now, each session type will have its own input history for normal
queryinput in the session, as well as the interactive query mode. This support is provided for Reline and Readline as fallback.Verification
query SELECT ...etc.querycalls are not shared between session types.query -i) by usingselect version();etc.