Skip to content

*: refine text protocol multiple query response#11263

Merged
jackysp merged 3 commits into
pingcap:masterfrom
lysu:dev-multisql
Jul 17, 2019
Merged

*: refine text protocol multiple query response#11263
jackysp merged 3 commits into
pingcap:masterfrom
lysu:dev-multisql

Conversation

@lysu
Copy link
Copy Markdown
Contributor

@lysu lysu commented Jul 15, 2019

What problem does this PR solve?

fixes #11235

the current multi-query response has some problem

  1. multi NoDelay executor(update/insert/delete) only return 1 ok packet, but mysql return N ok packets for N stmts

in tidb:

image

in mysql:

image

  1. multi select return addition ok packet
s.addBatch("select 1");
s.addBatch("select 1");
System.out.println(Arrays.toString(s.executeBatch()));

tidb will return -1, -1, 0, but mysql need -1, -1.

this PR fix both problems, but leave #4617 to next (because it need change too many code)

What is changed and how it works?

  • no-delay executor in multi-query also return a special recordSet - -
  • writeMultiResultset take care special recordSet and write ok-packet for each record-set
  • refine ServerMoreResultsExists set logic writeMultiResultset

Check List

Tests

Code changes

  • Has exported function/method change
  • Has interface methods change

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch ?

@lysu
Copy link
Copy Markdown
Contributor Author

lysu commented Jul 15, 2019

/run-all-tests

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2019

Codecov Report

Merging #11263 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11263   +/-   ##
===========================================
  Coverage   81.5609%   81.5609%           
===========================================
  Files           423        423           
  Lines         91301      91301           
===========================================
  Hits          74466      74466           
  Misses        11512      11512           
  Partials       5323       5323

@lysu lysu removed the status/WIP label Jul 16, 2019
@lysu lysu requested review from jackysp and tiancaiamao July 16, 2019 08:21
@lysu
Copy link
Copy Markdown
Contributor Author

lysu commented Jul 16, 2019

/run-all-tests tidb-test=pr/859

Copy link
Copy Markdown
Contributor

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lysu lysu added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 17, 2019
Copy link
Copy Markdown

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lysu
Copy link
Copy Markdown
Contributor Author

lysu commented Jul 17, 2019

/run-all-tests tidb-test=pr/859

@jackysp jackysp merged commit 77b6858 into pingcap:master Jul 17, 2019
@imtbkcat
Copy link
Copy Markdown

@lysu does it need to be cherry-picked to release-3.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/server status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug. type/compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Result of JDBC batch update is different from MySQL

3 participants