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

tests: bump mysql-tester version and update result #47358

Merged
merged 11 commits into from
Oct 8, 2023

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Issue Number: ref #45961

Problem Summary:

What is changed and how it works?

After mysql-tester commit pingcap/mysql-tester#105
Several bugs are found and the test result need to update.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@ti-chi-bot
Copy link

ti-chi-bot bot commented Sep 28, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 28, 2023
@tiprow
Copy link

tiprow bot commented Sep 28, 2023

Hi @tiancaiamao. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@@ -12,17 +12,15 @@ Insert N/A root N/A
explain format = 'brief' delete from t where a > 100;
id estRows task access object operator info
Delete N/A root N/A
└─SelectLock 3333.33 root for update 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The is bug of the old mysql-tester, there should not be a SelectLock unless in the 'select ... for update' statement.

@@ -55,14 +55,13 @@ HashJoin 4166.67 root left outer join, equal:[eq(explain_easy.t1.c2, explain_ea
explain format = 'brief' update t1 set t1.c2 = 2 where t1.c1 = 1;
id estRows task access object operator info
Update N/A root N/A
└─Point_Get 1.00 root table:t1 handle:1, lock
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto.

@@ -2075,7 +2075,7 @@ EXECUTE mystmt USING @a;
id a
select @@last_plan_from_cache;
@@last_plan_from_cache
0
1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be the correct behaviour --- plan cache been used.

@@ -896,7 +896,7 @@ Projection_3 10000.00 root Column#2, Column#4, Column#5, Column#6, Column#7, Co
└─MemTableScan_4 10000.00 root table:PROCESSLIST
select USER, DB, COMMAND, TIME, STATE, INFO, `DIGEST` from information_schema.processlist;
USER DB COMMAND TIME STATE INFO DIGEST
root planner__cascades__integration Query 0 in transaction; autocommit select USER, DB, COMMAND, TIME, STATE, INFO, `DIGEST` from information_schema.processlist 78f1c0b2fcd082b6504b8ba48706391f13aad730a9efb7ec295688010bfec477
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, catch the bug!

@@ -228,20 +228,6 @@ A 1
Y 1
a 1
y 1
create table strlist(a varchar(10) charset utf8mb4 collate utf8mb4_general_ci, b int) partition by list columns (a) (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

└─Limit_38 382.00 cop[tikv] offset:0, count:382
└─Selection_33 382.00 cop[tikv] or(and(eq(planner__core__issuetest__planner_issue.tbl_39.col_239, 1994), not(in(planner__core__issuetest__planner_issue.tbl_39.col_239, 2004, 2010, 2010))), and(gt(planner__core__issuetest__planner_issue.tbl_39.col_239, 1996), or(lt(cast(planner__core__issuetest__planner_issue.tbl_39.col_239, double UNSIGNED BINARY), 2026), gt(cast(planner__core__issuetest__planner_issue.tbl_39.col_239, double UNSIGNED BINARY), 2011))))
└─IndexRangeScan_32 477.50 cop[tikv] table:tbl_39, index:PRIMARY(col_239) range:[1994,1994], (1996,+inf], keep order:true, stats:pseudo
Projection_8 382.00 root planner__core__issuetest__planner_issue.tbl_39.col_239
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The result before and after are both correct, it seems the 'tidb_partition_prune_mode' differ.

CREATE TABLE IF NOT EXISTS t(a int);
IMPORT INTO t FROM '/file.csv';
Error 1105 (HY000): cannot run IMPORT INTO in explicit transaction
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old error is wrong ... catch the bug!

@tiancaiamao tiancaiamao marked this pull request as ready for review September 28, 2023 07:26
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 28, 2023
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

Rest LGTM

tests/integrationtest/t/planner/core/integration.test Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #47358 (4098ed2) into master (b431900) will increase coverage by 0.4237%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #47358        +/-   ##
================================================
+ Coverage   72.2972%   72.7209%   +0.4237%     
================================================
  Files          1352       1373        +21     
  Lines        401072     407412      +6340     
================================================
+ Hits         289964     296274      +6310     
- Misses        91884      92339       +455     
+ Partials      19224      18799       -425     
Flag Coverage Δ
integration 38.6221% <ø> (?)
unit 72.2810% <ø> (-0.0162%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 53.9913% <ø> (ø)
parser 84.7544% <ø> (-0.0108%) ⬇️
br 48.8649% <ø> (-4.3260%) ⬇️

@tiancaiamao
Copy link
Contributor Author

/test check-dev2
/test check-dev

@tiprow
Copy link

tiprow bot commented Sep 28, 2023

@tiancaiamao: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test check-dev2
/test check-dev

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Defined2014
Copy link
Contributor

Defined2014 commented Oct 7, 2023

@tiancaiamao should fix the check_dev and check_dev_2

We could use ./run-tests.sh -r all to generate all results.

Maybe forgot update run-tests.sh script.

@ti-chi-bot ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Oct 7, 2023
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Defined2014, YangKeao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Defined2014,YangKeao]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the lgtm label Oct 7, 2023
@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 7, 2023
@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 7, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-10-07 10:04:34.413505782 +0000 UTC m=+873872.000615929: ☑️ agreed by Defined2014.
  • 2023-10-07 12:09:47.338387151 +0000 UTC m=+881384.925497296: ☑️ agreed by YangKeao.

@YangKeao
Copy link
Member

YangKeao commented Oct 8, 2023

executor/write failed, but I cannot reproduce on my local machine 🤔

run test [executor/write] err: sql:update t set id=id+17 where id in (3,10);: failed to run query 
"update t set id=id+17 where id in (3,10);" 
 around line 612, 
we need(99):
update t set id=id+17 where id in (3,10);
Error 1526 (HY000): Table has no partition for value 27
s
but got(99):
update t set id=id+17 where id in (3,10);
Error 1062 (23000): Duplicate entry '20' for key 't.idx'

/retest

@Defined2014
Copy link
Contributor

executor/write failed, but I cannot reproduce on my local machine 🤔

run test [executor/write] err: sql:update t set id=id+17 where id in (3,10);: failed to run query 
"update t set id=id+17 where id in (3,10);" 
 around line 612, 
we need(99):
update t set id=id+17 where id in (3,10);
Error 1526 (HY000): Table has no partition for value 27
s
but got(99):
update t set id=id+17 where id in (3,10);
Error 1062 (23000): Duplicate entry '20' for key 't.idx'

/retest

A new PR merged. #47420

@ti-chi-bot ti-chi-bot bot merged commit 2d1d3fd into pingcap:master Oct 8, 2023
12 of 15 checks passed
@tiancaiamao tiancaiamao deleted the update-mysql-tester branch October 8, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants