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

ClickHouse doesn't return affected rows via MySQL protocol for INSERT queries after 20.5+ #16605

Closed
Slach opened this issue Nov 2, 2020 · 8 comments · Fixed by #16715
Closed
Labels
bug Confirmed user-visible misbehaviour in official release

Comments

@Slach
Copy link
Contributor

Slach commented Nov 2, 2020

Describe the bug
ClickHouse doesn't return affected rows via MySQL protocol for INSERT queries after 20.5+

How to reproduce
i create clear docker-compose environment for reproduce behavior MySQL + clickhouse + php + python
https://gist.github.com/Slach/aa67440ce856a3a53f64f92eeddfbc1b

Queries to run that lead to unexpected result

INSERT INTO default.t1(n) SELECT * FROM numbers(1000)

Expected behavior
MySQL protocol shall return affected rows field for INSERT queries (it works successful in 20.3 and 20.4)
image

Actual behavior
image

@Slach Slach added the bug Confirmed user-visible misbehaviour in official release label Nov 2, 2020
@den-crane
Copy link
Contributor

CH simply does not know this.

@den-crane den-crane added comp-mysql feature and removed bug Confirmed user-visible misbehaviour in official release labels Nov 2, 2020
@Slach
Copy link
Contributor Author

Slach commented Nov 2, 2020

@den-crane
why CH knew it in 20.3, 20.4 but forget in 20.5+?
and why it successfully save into system.query_log result_rows field?
it will break MUCH cases for users which expect affected rows to ensure some data was inserted

@den-crane den-crane added bug Confirmed user-visible misbehaviour in official release and removed comp-mysql feature labels Nov 2, 2020
@den-crane
Copy link
Contributor

Not sure Why it was working before.


create table A ( A String) Engine=Memory;
insert into A values('a'),('b');
2 rows in set.                  <<<<<<<-----------

create table A1 as A
insert into A1 select * from A
0 rows in set

insert into A1 SELECT toString(number) FROM numbers(1000)
0 rows in set.

@den-crane
Copy link
Contributor

@Slach Check this in 20.3

INSERT INTO default.t1(n) SELECT * FROM numbers(1000) where number%3

@Slach
Copy link
Contributor Author

Slach commented Nov 2, 2020

@den-crane
done
look to https://gist.github.com/Slach/aa67440ce856a3a53f64f92eeddfbc1b/revisions?diff=split

string(26) "CLICKHOUSE #2 INSERT QUERY"
string(70) "INSERT INTO default.t1(n) SELECT * FROM numbers(1000) where number % 3"
string(6) "RESULT"
bool(true)
string(27) "CLICKHOUSE #2 affected_rows"
int(666)

@Slach
Copy link
Contributor Author

Slach commented Nov 3, 2020

add steps to reproduce for python in https://gist.github.com/Slach/aa67440ce856a3a53f64f92eeddfbc1b

@zhang2014 @BohuTANG could you help us resolve this issue?

@BohuTANG
Copy link
Contributor

BohuTANG commented Nov 4, 2020

The problem does exist, but it's not the MySQLHandler/MySQL problem.
When we execute INSERT INTO default.t1(n) SELECT * FROM numbers(1000) where number % 3, the packet send to MySQL client is not through the MySQLOutputFormat.cpp:

const auto & header = getPort(PortKind::Main).getHeader();
if (header.columns() == 0)
packet_endpoint->sendPacket(OKPacket(0x0, context->mysql.client_capabilities, affected_rows, 0, 0, "", human_readable_info), true);
else
if (context->mysql.client_capabilities & CLIENT_DEPRECATE_EOF)
packet_endpoint->sendPacket(OKPacket(0xfe, context->mysql.client_capabilities, affected_rows, 0, 0, "", human_readable_info), true);

Instead it send directly here:

if (!with_output)
packet_endpoint->sendPacket(OKPacket(0x00, client_capability_flags, 0, 0, 0), true);

I am not sure why, maybe @alexey-milovidov could give some hints.

@Slach
Copy link
Contributor Author

Slach commented Nov 5, 2020

@zhang2014 could you look to #16605 (comment)
and help us resolve this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed user-visible misbehaviour in official release
Projects
None yet
3 participants