HISTORY_INCREMENTAL_SEARCH breaks line readering

[amosbird]

HISTORY_INCREMENTAL_SEARCH breaks line readering

[amosbird]

So you have newlines embedded in this particular item?

Exactly. The item is pasted via bracketed pasting.

[amosbird]

As far as I can tell there was a new feature added to Clickhouse version of replxx related to editing history entry behavior.
replxx behaves differently than readline in this regard.

Yes, and @azat also think it's better to use the upstream strategy.

[AmokHuginnsson]

Is length of the line in question longer then $LINES * $COLS - len(prompt)?

[amosbird]

Yes. Much longer (multiple lines)

[AmokHuginnsson]

Sorry, I did not get that. You have "multiple lines" of that length or did you mean multiple screens?

[amosbird]

Sorry for the confusion. Lemme post the history item.

This is one history item with multiple lines:

### 2021-03-24 12:22:02.257
drop table if exists x;

create table x (type Enum8('hourly' = 0, 'hourly_staging' = 1, 'daily' = 2, 'daily_staging' = 3), dt DateTime, i int)
engine MergeTree partition by (type, if(toUInt8(type) < 2, dt, toStartOfDay(dt))) order by i;

insert into x values ('daily', '2021-02-28 01:00:00', 10), ('daily', '2021-02-28 02:00:00', 20);
insert into x values ('hourly', '2021-02-28 05:00:00', 60), ('hourly', '2021-02-28 06:00:00', 70);
insert into x values ('hourly', '2021-02-28 07:00:00', 80), ('hourly', '2021-02-28 08:00:00', 90);
insert into x values ('hourly', '2021-02-29 05:00:00', 60), ('hourly', '2021-02-29 06:00:00', 70);

The screenshot shows the process of typing "c", "cr", "cre", "crea".

[AmokHuginnsson]

So you have newlines embedded in this particular item?

[AmokHuginnsson]

Hello.
Can you check if current master (eef0c4d) improves the situation?

[amosbird]

Awesome! Will try it asap.

[amosbird]

Oops, it crashed

#0  0x00007fdad78e766e in raise () from /tmp/gentoo/lib64/libc.so.6
#1  0x00007fdad78d1536 in abort () from /tmp/gentoo/lib64/libc.so.6
#2  0x00007fdad7b71e7c in std::__1::__libcpp_abort_debug_function (info=...) at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/libcxx/src/debug.cpp:35
#3  0x00007fdad929397a in std::__1::vector<char32_t, std::__1::allocator<char32_t> >::operator[] (this=0x7fdab7606b18, __n=0) at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/libcxx/include/vector:1549
#4  0x00007fdad9290d1d in replxx::UnicodeString::operator[] (this=0x7fdab7606b18, pos=0) at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/replxx/src/unicodestring.hxx:154
#5  0x00007fdad928c95a in replxx::Replxx::ReplxxImpl::prev_newline_position (this=0x7fdab7606b00, pos_=0) at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/replxx/src/replxx_impl.cxx:1678
#6  0x00007fdad92866e1 in replxx::Replxx::ReplxxImpl::history_previous (this=0x7fdab7606b00) at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/replxx/src/replxx_impl.cxx:1705
#7  0x00007fdad9285834 in replxx::Replxx::ReplxxImpl::action (this=0x7fdab7606b00, actionTrait_=2, handler_=@0x7ffef6bc3428: (replxx::Replxx::ACTION_RESULT (replxx::Replxx::ReplxxImpl::*)(replxx::Replxx::ReplxxImpl * const, char32_t)) 0x7fdad9286640 <replxx::Replxx::ReplxxImpl::history_previous(char32_t)>, code_=1114116 U'\x110004') at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/replxx/src/replxx_impl.cxx:1278
#8  0x00007fdad9284daa in replxx::Replxx::ReplxxImpl::invoke (this=0x7fdab7606b00, action_=replxx::Replxx::ACTION::HISTORY_PREVIOUS, code=1114116 U'\x110004') at /media/ssd1/clickhouse1/gentoo.new/home/amos/git/clickhouse-origin/src/contrib/replxx/src/replxx_impl.cxx:342

$1 = {
_data = std::vector of length 0, capacity 632
}

[AmokHuginnsson]

Can you provide input data and history file and key sequence before crash?

[amosbird]

Yes. I have bracketed paste mode enabled and pasting the following text:

drop table if exists x;

create table x (type Enum8('hourly' = 0, 'hourly_staging' = 1, 'daily' = 2, 'daily_staging' = 3), dt DateTime, i int)
engine MergeTree partition by (type, if(toUInt8(type) < 2, dt, toStartOfDay(dt))) order by i;

insert into x values ('daily', '2021-02-28 01:00:00', 10), ('daily', '2021-02-28 02:00:00', 20);
insert into x values ('hourly', '2021-02-28 05:00:00', 60), ('hourly', '2021-02-28 06:00:00', 70);
insert into x values ('hourly', '2021-02-28 07:00:00', 80), ('hourly', '2021-02-28 08:00:00', 90);
insert into x values ('hourly', '2021-02-29 05:00:00', 60), ('hourly', '2021-02-29 06:00:00', 70);

Then submit this text and press Up, crash.

[amosbird]

After restarting the client, pressing Up recalls this multi-line history correctly. However, press Up again crashes.

[AmokHuginnsson]

I cannot reproduce this crash :(
Can you reproduce this crash with ./build/debug/replxx-example-c-api B1?

[amosbird]

Can you reproduce this crash with ./build/debug/replxx-example-c-api B1?

Hmm, I can't. I'll investigate.

[amosbird]

This somehow works.. #120

[AmokHuginnsson]

You are completely right.
I have pushed your proposed fix to the master.
Thank you!

[amosbird]

Awesome! Now it works as expected. Btw, is it ok for me to submit this PR back to the upstream here ClickHouse#6 , so that we can switch back to this repo instead of maintaining the fork (•‿•)

[AmokHuginnsson]

Sure!
Though Clickhouse client has its own set of improvements that would have to be ported back to replxx if you were gonna switch to this repo.

If this issue is solved, can it closed?

[amosbird]

Though Clickhouse client has its own set of improvements that would have to be ported back to replxx if you were gonna switch to this repo.

The intense color is the only diff now)

If this issue is solved, can it closed?

Lemme do that for you. Thank you!

[AmokHuginnsson]

As far as I can tell there was a new feature added to Clickhouse version of replxx related to editing history entry behavior.
replxx behaves differently than readline in this regard.