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

expression: implement is_ipv4 and is_ipv6 function pushdown #41173

Merged
merged 15 commits into from Feb 15, 2023
Merged

expression: implement is_ipv4 and is_ipv6 function pushdown #41173

merged 15 commits into from Feb 15, 2023

Conversation

AntiTopQuark
Copy link
Contributor

@AntiTopQuark AntiTopQuark commented Feb 7, 2023

What problem does this PR solve?

Issue Number: close #41172
related issue: pingcap/tiflash#6773
Problem Summary:
implement is_ipv4 and is_ipv6 function pushdown

What is changed and how it works?

  1. open pushdown of is_ipv4 and is_ipv6
  2. add some test

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

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.

implement is_ipv4 and is_ipv6 function pushdown to tiflash

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 7, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • LittleFall
  • SeaRise

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test labels Feb 7, 2023
@ti-chi-bot
Copy link
Member

Hi @AntiTopQuark. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@AntiTopQuark
Copy link
Contributor Author

/run-all-tests

@hawkingrei
Copy link
Member

/ok-to-test

Comment on lines 8441 to 8453
tk.MustExec("insert into t values('123.123.123.123', 'F746:C349:48E3:22F2:81E0:0EA8:E7B6:8286')")
tk.MustExec("insert into t values('0.0.0.0', '0000:0000:0000:0000:0000:0000:0000:0000')")
tk.MustExec("insert into t values('127.0.0.1', '2001:0:2851:b9f0:6d:2326:9036:f37a')")
tk.MustExec("insert into t values('192.168.0.0/10', 'fe80::2dc3:25a5:49a1:6002%24')")
tk.MustExec("insert into t values('192.168.99.22.123', '4207:A33A:58D3:F2C3:8EDC:A548:3EC7:0D00:0D00')")
tk.MustExec("insert into t values('999.999.999.999', '4207:A33A:58D3:F2C3:8EDC:A548::0D00')")
tk.MustExec("insert into t values('3.2.1.', '4207::::8EDC:A548:3EC7:0D00')")
tk.MustExec("insert into t values('3..2.1', '4207:::::A548:3EC7:0D00')")
tk.MustExec("insert into t values('...', '::::::')")
tk.MustExec("insert into t values('4556456', '4556456')")
tk.MustExec("insert into t values('ajdjioa', 'ajdjioa')")
tk.MustExec("insert into t values('', '')")
tk.MustExec("insert into t values(null,null)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems useless here

Comment on lines 8486 to 8498
tk.MustExec("insert into t values('123.123.123.123', 'F746:C349:48E3:22F2:81E0:0EA8:E7B6:8286')")
tk.MustExec("insert into t values('0.0.0.0', '0000:0000:0000:0000:0000:0000:0000:0000')")
tk.MustExec("insert into t values('127.0.0.1', '2001:0:2851:b9f0:6d:2326:9036:f37a')")
tk.MustExec("insert into t values('192.168.0.0/10', 'fe80::2dc3:25a5:49a1:6002%24')")
tk.MustExec("insert into t values('192.168.99.22.123', '4207:A33A:58D3:F2C3:8EDC:A548:3EC7:0D00:0D00')")
tk.MustExec("insert into t values('999.999.999.999', '4207:A33A:58D3:F2C3:8EDC:A548::0D00')")
tk.MustExec("insert into t values('3.2.1.', '4207::::8EDC:A548:3EC7:0D00')")
tk.MustExec("insert into t values('3..2.1', '4207:::::A548:3EC7:0D00')")
tk.MustExec("insert into t values('...', '::::::')")
tk.MustExec("insert into t values('4556456', '4556456')")
tk.MustExec("insert into t values('ajdjioa', 'ajdjioa')")
tk.MustExec("insert into t values('', '')")
tk.MustExec("insert into t values(null,null)")
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

planner/core/integration_test.go Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 13, 2023
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 13, 2023
@AntiTopQuark
Copy link
Contributor Author

/reset

@AntiTopQuark
Copy link
Contributor Author

/test all

@AntiTopQuark
Copy link
Contributor Author

/cc @ywqzzy

@AntiTopQuark
Copy link
Contributor Author

/retest

@ti-chi-bot ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Feb 15, 2023
@ti-chi-bot ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 15, 2023
@LittleFall
Copy link
Contributor

/retest

1 similar comment
@ywqzzy
Copy link
Contributor

ywqzzy commented Feb 15, 2023

/retest

@guo-shaoge
Copy link
Collaborator

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 2780503

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 15, 2023
@SeaRise
Copy link
Contributor

SeaRise commented Feb 15, 2023

/retest

@guo-shaoge
Copy link
Collaborator

Please fix case: TestIsIPv6ToTiFlash @AntiTopQuark

25665 [2023/02/15 15:56:48.047 +08:00] [WARN] [2pc.go:1738] ["schemaLeaseChecker is not set for this transaction"] [sessionID=10] [startTS=439470734192672770] [checkTS=439470734192672771]
25666     result.go:131:
25667             Error Trace:    /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7660/execroot/__main__/bazel-out/k8-fastbuild/bin/planner/core/core_test_/core_test.runfiles/__main__/planner/core/result.go:131
25668                                         /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7660/execroot/__main__/bazel-out/k8-fastbuild/bin/planner/core/core_test_/core_test.runfiles/__main__/planner/core/integration_test.go:8523
25669             Error:          Not equal:
25670                             expected: "[[TableReader_9 root data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"
25671                             actual  : "[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"
25672
25673                             Diff:
25674                             --- Expected
25675                             +++ Actual
25676                             @@ -1 +1 @@
25677                             -[[TableReader_9 root data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]
25678                             +[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]
25679             Test:           TestIsIPv6ToTiFlash
25680             Messages:       sql:explain select is_ipv6(v6) from t;, args:[]

@AntiTopQuark
Copy link
Contributor Author

/reset

@AntiTopQuark
Copy link
Contributor Author

/reset

@guo-shaoge
Copy link
Collaborator

Looks like case still failed: @AntiTopQuark

[checkTS=439472502008184835]
    result.go:131:
            Error Trace:    /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7919/execroot/__main__/bazel-out/k8-fastbuild/bin/planner/ core/core_test_/core_test.runfiles/__main__/planner/core/result.go:131
                                        /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7919/execroot/__main__/bazel-out/k8-fastbuild/ bin/planner/core/core_test_/core_test.runfiles/__main__/planner/core/integration_test.go:8523
            Error:          Not equal:
                            expected: "[[TableReader_9 root   MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [     └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"
                            actual  : "[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [       └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -[[TableReader_9 root   MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [               └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]                                                                    +[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]
            Test:           TestIsIPv6ToTiFlash
            Messages:       sql:explain select is_ipv6(v6) from t;, args:[]

@guo-shaoge
Copy link
Collaborator

guo-shaoge commented Feb 15, 2023

You can run go test . --tags=intest -run TestIsIPv6ToTiFlash in your local env, and make sure case is ok. @AntiTopQuark

@AntiTopQuark
Copy link
Contributor Author

/test check-dev2

@ti-chi-bot
Copy link
Member

@AntiTopQuark: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test build
  • /test check-dev
  • /test check-dev2
  • /test mysql-test
  • /test unit-test

Use /test all to run all jobs.

In response to this:

/test check-dev2

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.

@AntiTopQuark
Copy link
Contributor Author

You can run go test . --tags=intest -run TestIsIPv6ToTiFlash in your local env, and make sure case is ok. @AntiTopQuark

thank u very much, now it is okey.
image

@AntiTopQuark
Copy link
Contributor Author

Looks like case still failed: @AntiTopQuark

[checkTS=439472502008184835]
    result.go:131:
            Error Trace:    /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7919/execroot/__main__/bazel-out/k8-fastbuild/bin/planner/ core/core_test_/core_test.runfiles/__main__/planner/core/result.go:131
                                        /home/jenkins/.tidb/tmp/63a9840cd0739f2c243bb46478607469/sandbox/linux-sandbox/7919/execroot/__main__/bazel-out/k8-fastbuild/ bin/planner/core/core_test_/core_test.runfiles/__main__/planner/core/integration_test.go:8523
            Error:          Not equal:
                            expected: "[[TableReader_9 root   MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [     └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"
                            actual  : "[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [       └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -[[TableReader_9 root   MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [               └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]                                                                    +[[TableReader_9 root MppVersion: 1, data:ExchangeSender_8] [└─ExchangeSender_8 mpp[tiflash] ExchangeType: PassThrough] [  └─Projection_4 mpp[tiflash] is_ipv6(test.t.v6)->Column#4] [    └─TableFullScan_7 mpp[tiflash] keep order:false, stats:pseudo]]
            Test:           TestIsIPv6ToTiFlash
            Messages:       sql:explain select is_ipv6(v6) from t;, args:[]

thank u very much~

@AntiTopQuark
Copy link
Contributor Author

it looks like that the test named "check_dev_2" is unstable .
The same problem also occurs with another PR :#41453
https://do.pingcap.net/jenkins/blue/organizations/jenkins/pingcap%2Ftidb%2Fghpr_check2/detail/ghpr_check2/14262/

@AntiTopQuark
Copy link
Contributor Author

/test check-dev2

@AntiTopQuark
Copy link
Contributor Author

**idc-jenkins-ci-tidb/check_dev_2 ** Pending — Jenkins job enqueued.                   BaseSHA:3d8b7c21f15b1d45d05169d1c81cd5e42233c894

image

@ti-chi-bot ti-chi-bot merged commit aa3b57c into pingcap:master Feb 15, 2023
ghazalfamilyusa pushed a commit to ghazalfamilyusa/tidb that referenced this pull request Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test release-note size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement is_ipv4 and is_ipv6 function push down to TiFlash
7 participants