ARROW-13879: [C++] Mixed support for binary types in regex functions #11233

edponce · 2021-09-25T06:25:17Z

This PR extends variable-width binary types support for string functions:

find_substring[_regex]
count_substring[_regex]
match_substring[_regex]
split_pattern[_regex]
replace_substring[_regex]
match_like
starts/ends_with
extract_regex

Also, updates several scalar string kernel/function registrations.

github-actions · 2021-09-25T06:25:36Z

https://issues.apache.org/jira/browse/ARROW-13879

github-actions · 2021-09-25T06:25:37Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

edponce · 2021-09-27T02:24:38Z

@lidavidm Since you are working on ARROW-13878 which is related, please review this PR. This PR only adds variable-width binary support.

lidavidm

In general I don't think we should add binary support to utf8_ or ascii_ functions, as they make assumptions about input encoding that don't hold for binary values. (The user can perform a safe or unsafe cast if needed.) Also, we need to make sure we actually test with non-UTF8 values and we should make sure RE2 handles this properly.

cpp/src/arrow/compute/kernels/scalar_string.cc

cpp/src/arrow/compute/kernels/codegen_internal.h

cpp/src/arrow/compute/kernels/scalar_string_test.cc

westonpace

Looks good.

cpp/src/arrow/compute/kernels/scalar_string.cc

cpp/src/arrow/testing/gtest_util.h

edponce · 2021-10-14T07:09:11Z

@pitrou If you get a chance, please take a look at this PR. It is a draft mainly because it is missing additional tests, pending review fixes, and updates to comments/docs.

lidavidm

Thanks, overall this looks good, I left a few small comments.

cpp/src/arrow/compute/kernels/scalar_string.cc

cpp/src/arrow/compute/kernels/scalar_string_test.cc

westonpace

Some pretty minor nits but this looks good to me.

cpp/src/arrow/compute/kernels/scalar_string.cc

cpp/src/arrow/compute/kernels/scalar_string_test.cc

westonpace · 2021-10-22T10:46:18Z

@github-actions autotune

westonpace · 2021-10-22T11:44:16Z

It appears my suggestions introduced some lines too long. Can you run clang-format? I tried autotune but it didn't work for some reason.

…rts/ends_with, and split_pattern

ElenaHenderson · 2021-10-25T12:54:16Z

Benchmark runs are scheduled for baseline = e7158c6 and contender = be665ef. be665ef is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] ec2-t3-xlarge-us-east-2
[Scheduled] ursa-i9-9960x
[Scheduled] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

This PR extends variable-width binary types support for string functions: * find_substring[_regex] * count_substring[_regex] * match_substring[_regex] * split_pattern[_regex] * replace_substring[_regex] * match_like * starts/ends_with * extract_regex Also, updates several scalar string kernel/function registrations. Closes apache#11233 from edponce/ARROW-13879-Mixed-support-for-binary-types-in-regex- Authored-by: Eduardo Ponce <edponce00@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>

github-actions bot added the Component: C++ label Sep 25, 2021

lidavidm reviewed Sep 27, 2021

View reviewed changes

edponce force-pushed the ARROW-13879-Mixed-support-for-binary-types-in-regex- branch from 2c38f33 to c91af07 Compare October 2, 2021 01:09

edponce marked this pull request as draft October 2, 2021 07:05

edponce force-pushed the ARROW-13879-Mixed-support-for-binary-types-in-regex- branch 2 times, most recently from d0e414e to 050182e Compare October 13, 2021 19:15

westonpace reviewed Oct 13, 2021

View reviewed changes

edponce marked this pull request as ready for review October 15, 2021 03:08

edponce requested review from lidavidm and westonpace October 15, 2021 03:08

edponce force-pushed the ARROW-13879-Mixed-support-for-binary-types-in-regex- branch from 879fde1 to dc257e5 Compare October 15, 2021 03:14

lidavidm reviewed Oct 18, 2021

View reviewed changes

cpp/src/arrow/compute/kernels/scalar_string.cc Outdated Show resolved Hide resolved

cpp/src/arrow/compute/kernels/scalar_string_test.cc Outdated Show resolved Hide resolved

edponce force-pushed the ARROW-13879-Mixed-support-for-binary-types-in-regex- branch from dc257e5 to 8e26f2e Compare October 18, 2021 15:35

edponce requested a review from lidavidm October 18, 2021 16:10

lidavidm approved these changes Oct 18, 2021

View reviewed changes

westonpace approved these changes Oct 21, 2021

View reviewed changes

edponce added 8 commits October 22, 2021 14:19

use GD with binary types for match_substring and add tests

3253ffb

remove find_substring test for String-only types

dc617a2

support binary types for match_substring_regex and starts/ends_with

07c6c23

support binary types for match_like

b43f428

support binary types for split_pattern[_regex]

11bc310

support binary types for extract_regex

813499e

update binary/utf8_length for binary types

136bc80

add comment to GD for variable binary/string types

ec3f356

edponce added 23 commits October 22, 2021 14:19

trigger error if ignore_case option is enabled for binary types

1023e23

add tests for ignore_case in find_substring

ced5d0e

linter fixes

0222a48

make distinction between binary/string kernels

5dcd5fa

add tests

67b797d

add/update binary support for find/count/match/replace_substring, sta…

10d3389

…rts/ends_with, and split_pattern

update extract_regex and fixed capitalization of SubString

f12abc2

use std::move(exec)

d48721c

add/fix ARROW_WITH_RE2 macros

649ec68

fix linter errors

c888d90

update use of RE2::Options (for encoding)

f5527df

add/renamed XXXArrowTypes for binary/string tests

5bdfe1b

fix comment

373008b

add more tests

af87d11

inline ApplyPredicate

810617f

use input bytes consistently for non-UTF8 string tests

00960d3

update kernel input/output types in C++ docs

d001e49

minor fix/update comments

4ac0718

revert comment change

c28a331

fix doc variables

b70d6e2

remove std::move and unused StringPredicate

2f9be48

add ampersand to parameter in test utility

55739a3

fix linter errors

2ffacd6

edponce force-pushed the ARROW-13879-Mixed-support-for-binary-types-in-regex- branch from 35640e6 to 2ffacd6 Compare October 22, 2021 18:19

lidavidm approved these changes Oct 25, 2021

View reviewed changes

lidavidm closed this in be665ef Oct 25, 2021

edponce mentioned this pull request Dec 2, 2021

ARROW-13950: [C++] min_element_wise/max_element_wise missing support for some types #11793

Closed

asfimport mentioned this pull request Oct 28, 2021

[C++] Mixed support for binary types in regex functions #29497

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-13879: [C++] Mixed support for binary types in regex functions #11233

ARROW-13879: [C++] Mixed support for binary types in regex functions #11233

edponce commented Sep 25, 2021 •

edited

github-actions bot commented Sep 25, 2021

github-actions bot commented Sep 25, 2021

edponce commented Sep 27, 2021

lidavidm left a comment

westonpace left a comment

edponce commented Oct 14, 2021

lidavidm left a comment

westonpace left a comment

westonpace commented Oct 22, 2021

westonpace commented Oct 22, 2021

ElenaHenderson commented Oct 25, 2021

ARROW-13879: [C++] Mixed support for binary types in regex functions #11233

ARROW-13879: [C++] Mixed support for binary types in regex functions #11233

Conversation

edponce commented Sep 25, 2021 • edited

github-actions bot commented Sep 25, 2021

github-actions bot commented Sep 25, 2021

edponce commented Sep 27, 2021

lidavidm left a comment

Choose a reason for hiding this comment

westonpace left a comment

Choose a reason for hiding this comment

edponce commented Oct 14, 2021

lidavidm left a comment

Choose a reason for hiding this comment

westonpace left a comment

Choose a reason for hiding this comment

westonpace commented Oct 22, 2021

westonpace commented Oct 22, 2021

ElenaHenderson commented Oct 25, 2021

edponce commented Sep 25, 2021 •

edited