feat(expr):support is_ipv4 and is_ipv6 expr to tiflash #6773

AntiTopQuark · 2023-02-07T15:17:33Z

pushdown to tiflash

What problem does this PR solve?

Issue Number: close #5120
close #5121

Problem Summary:
support is_ipv4() and is_ipv6() expr to pushdown to tiflash

What is changed and how it works?

register the cast function
implement the calculation for is_ipv4() and is_ipv6()
add tests of functions

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

Release note

support is_ipv4() and is_ipv6() expr to pushdown to tiflash

pushdown to tiflash

ti-chi-bot · 2023-02-07T15:17:35Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Lloyd-Pottiger
ywqzzy

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.

AntiTopQuark · 2023-02-07T15:46:45Z

/run-all-tests

AntiTopQuark · 2023-02-07T15:54:22Z

/run-all-tests

AntiTopQuark · 2023-02-08T00:25:02Z

/run-all-tests

AntiTopQuark · 2023-02-08T01:22:42Z

/cc @JaySon-Huang
/cc @CalvinNeo

Lloyd-Pottiger · 2023-02-08T02:24:38Z

/cc @ywqzzy @xzhangxian1008

dbms/src/Functions/FunctionsIsIPAddr.h

Revise according to review comments Co-authored-by: yanweiqi <592838129@qq.com>

Co-authored-by: yanweiqi <592838129@qq.com>

AntiTopQuark · 2023-02-14T00:42:04Z

/run-all-tests

SeaRise

good job! Others LGTM

SeaRise · 2023-02-14T01:47:38Z

dbms/src/Functions/FunctionsIsIPAddr.h

+    void executeImpl(Block & block, const ColumnNumbers & arguments, size_t result) const override
+    {
+        auto [column, nullmap] = removeNullable(block.getByPosition(arguments[0]).column.get());
+        if (const auto * col_input = checkAndGetColumn<ColumnString>(column))


Does this hold true for the case where the input is only null?
Is there an ut to guarantee this?

thank u very much~

Before there was a test case where the input was all NULL for testing ConstColumn
https://github.com/AntiTopQuark/support_ipv4_and_ipv6_func/blob/09fd7fd29c83f298ee654c9b9a416601a718b854/dbms/src/Functions/tests/gtest_is_ip_addr.cpp#L80

ASSERT_COLUMN_EQ(createConstColumn<UInt8>(4, 0), executeFunction("tiDBIsIPv6", {createConstColumn<Nullable<String>>(4, std::nullopt)}));

now I added another one

ASSERT_COLUMN_EQ(createColumn<UInt8>({0, 0, 0, 0, 0}), executeFunction("tiDBIsIPv6", {createColumn<Nullable<String>>({std::nullopt, std::nullopt, std::nullopt, std::nullopt, std::nullopt})}));

Can you test for DataTypeNullable<DataTypeNothing>?
You can use

ColumnWithTypeAndName createOnlyNullColumnConst(size_t size, const String & name = ""); ColumnWithTypeAndName createOnlyNullColumn(size_t size, const String & name = "");

in FunctionTestUtils.

You can refer to gtest_strings_concat_ws.cpp

AntiTopQuark · 2023-02-14T03:40:24Z

/run-all-tests

AntiTopQuark · 2023-02-14T05:05:56Z

/run-all-tests

AntiTopQuark · 2023-02-14T10:17:27Z

/run-all-tests

AntiTopQuark · 2023-02-14T10:41:45Z

/run-all-tests

AntiTopQuark · 2023-02-14T14:56:34Z

/run-all-tests

Lloyd-Pottiger · 2023-02-15T01:37:27Z

/merge

ti-chi-bot · 2023-02-15T01:37:28Z

@Lloyd-Pottiger: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-02-15T01:37:30Z

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

Commit hash: 32cef30

feat(expr):support is_ipv4 and is_ipv6 expr to

772c064

pushdown to tiflash

ti-chi-bot added release-note size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 7, 2023

AntiTopQuark mentioned this pull request Feb 7, 2023

Implement is_ipv4 and is_ipv6 function push down to TiFlash pingcap/tidb#41172

Closed

AntiTopQuark changed the title ~~feat(expr):support is_ipv4 and is_ipv6 expr to~~ feat(expr):support is_ipv4 and is_ipv6 expr to tiflash Feb 7, 2023

AntiTopQuark mentioned this pull request Feb 7, 2023

expression: implement is_ipv4 and is_ipv6 function pushdown pingcap/tidb#41173

Merged

12 tasks

style: format code

fae3bbf

Update is_ip_addr.test

9748c3e

ti-chi-bot requested review from CalvinNeo and JaySon-Huang February 8, 2023 01:22

ti-chi-bot requested review from xzhangxian1008 and ywqzzy February 8, 2023 02:24

Lloyd-Pottiger assigned ywqzzy Feb 8, 2023

Merge branch 'master' into master

359f882

JaySon-Huang removed request for JaySon-Huang and CalvinNeo February 8, 2023 04:06

ywqzzy reviewed Feb 8, 2023

View reviewed changes

dbms/src/Functions/FunctionsIsIPAddr.h Outdated Show resolved Hide resolved

dbms/src/Functions/FunctionsIsIPAddr.h Outdated Show resolved Hide resolved

dbms/src/Functions/FunctionsIsIPAddr.h Outdated Show resolved Hide resolved

ywqzzy self-requested a review February 8, 2023 07:37

ywqzzy reviewed Feb 8, 2023

View reviewed changes

dbms/src/Functions/FunctionsIsIPAddr.h Outdated Show resolved Hide resolved

dbms/src/Functions/FunctionsIsIPAddr.h Outdated Show resolved Hide resolved

AntiTopQuark and others added 5 commits February 8, 2023 16:43

style:Revise according to review comments for IsIPAddr func

9720869

Revise according to review comments Co-authored-by: yanweiqi <592838129@qq.com>

style:Revise according to review comments for IsIPAddr func

b587f19

Co-authored-by: yanweiqi <592838129@qq.com>

style:Revise according to review comments for IsIPAddr

47e68f7

Co-authored-by: yanweiqi <592838129@qq.com>

style:Revise according to review comments for IsIPAddr

d2287a5

Co-authored-by: yanweiqi <592838129@qq.com>

style:Revise according to review comments for IsIPAddr func

3d7a92b

Co-authored-by: yanweiqi <592838129@qq.com>

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 14, 2023

Update registerFunctions.cpp

2bc8de1

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 14, 2023

Update FunctionsIsIPAddr.h

dce17e8

SeaRise reviewed Feb 14, 2023

View reviewed changes

AntiTopQuark added 2 commits February 14, 2023 10:12

Update gtest_is_ip_addr.cpp

fb191ff

update testcase

09fd7fd

ywqzzy approved these changes Feb 14, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 14, 2023

Merge branch 'master' into master

ddf1114

AntiTopQuark added 2 commits February 14, 2023 17:53

Update gtest_is_ip_addr.cpp

c23be4f

Merge branch 'pingcap:master' into master

b66005f

Update FunctionsIsIPAddr.h

32cef30

SeaRise approved these changes Feb 15, 2023

View reviewed changes

AntiTopQuark requested a review from xzhangxian1008 February 15, 2023 00:50

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

Merge branch 'master' into master

07e4a10

ti-chi-bot merged commit 7042c7b into pingcap:master Feb 15, 2023

zanmato1984 mentioned this pull request Mar 16, 2023

Call For Participation: implement function push down #5092

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(expr):support is_ipv4 and is_ipv6 expr to tiflash #6773

feat(expr):support is_ipv4 and is_ipv6 expr to tiflash #6773

AntiTopQuark commented Feb 7, 2023 •

edited

ti-chi-bot commented Feb 7, 2023 •

edited

AntiTopQuark commented Feb 7, 2023

AntiTopQuark commented Feb 7, 2023

AntiTopQuark commented Feb 8, 2023

AntiTopQuark commented Feb 8, 2023

Lloyd-Pottiger commented Feb 8, 2023

AntiTopQuark commented Feb 14, 2023

SeaRise left a comment

SeaRise Feb 14, 2023

AntiTopQuark Feb 14, 2023

SeaRise Feb 14, 2023

SeaRise Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

Lloyd-Pottiger commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

feat(expr):support is_ipv4 and is_ipv6 expr to tiflash #6773

feat(expr):support is_ipv4 and is_ipv6 expr to tiflash #6773

Conversation

AntiTopQuark commented Feb 7, 2023 • edited

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Feb 7, 2023 • edited

AntiTopQuark commented Feb 7, 2023

AntiTopQuark commented Feb 7, 2023

AntiTopQuark commented Feb 8, 2023

AntiTopQuark commented Feb 8, 2023

Lloyd-Pottiger commented Feb 8, 2023

AntiTopQuark commented Feb 14, 2023

SeaRise left a comment

Choose a reason for hiding this comment

SeaRise Feb 14, 2023

Choose a reason for hiding this comment

AntiTopQuark Feb 14, 2023

Choose a reason for hiding this comment

SeaRise Feb 14, 2023

Choose a reason for hiding this comment

SeaRise Feb 14, 2023

Choose a reason for hiding this comment

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

AntiTopQuark commented Feb 14, 2023

Lloyd-Pottiger commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

ti-chi-bot commented Feb 15, 2023

AntiTopQuark commented Feb 7, 2023 •

edited

ti-chi-bot commented Feb 7, 2023 •

edited