Skip to content

Conversation

@jvictorhuguenin
Copy link
Contributor

No description provided.

@github-actions
Copy link

Copy link
Contributor

Choose a reason for hiding this comment

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

You can reuse the LikeHolder class and provide the case-sensitivity option in the make method

Copy link
Contributor Author

@jvictorhuguenin jvictorhuguenin Apr 28, 2021

Choose a reason for hiding this comment

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

Ok, I created a Make method and constructor for the ILIKE function on LikeHolder, and deleted the IlikeHolder

Copy link
Contributor

Choose a reason for hiding this comment

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

this seems incorrect, they should be case-insensitive

Copy link
Contributor Author

@jvictorhuguenin jvictorhuguenin Apr 28, 2021

Choose a reason for hiding this comment

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

not really, the pattern has (?i)(google/re2#197) to turn case-insensitive on for the pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

this matching doesn't matter..actual starts_with, ends_with, sub_str are not case-insensitive

Copy link
Contributor

Choose a reason for hiding this comment

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

Here we match on the pattern for starts_with, ends_with cases, and replace with them..which won't be case-insensitive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, now both like and ilike are using the same method.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not yet fixed. You are still replacing with starts_with, ends_with which are case-sensitive. You need to disable TryOptimise for ILIKE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, sorry I thought first that it had no difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need explicit here. explicit is used to prevent implicit conversions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Why create a new method for this. Modify the existing one take options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

You can just call holder = std::shared_ptr<LikeHolder>(new LikeHolder(pcre_pattern, regex_op));
No need for if here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@praveenbingo
Copy link
Contributor

@projjal needs a rebase

@jvictorhuguenin jvictorhuguenin force-pushed the feature/implement-sql-ilike branch from 608e08f to f160880 Compare June 7, 2021 13:00
jvictorhuguenin added a commit to s1mbi0se/arrow that referenced this pull request Sep 21, 2021
Closes apache#10179 from jvictorhuguenin/feature/implement-sql-ilike and squashes the following commits:

f160880 <frank400> Optimize holder constructor call
97e6e2d <frank400> Remove unnecessary Make method
c2363b1 <frank400> Disable TryOptimize for ilike
a484149 <frank400> Fix checkstyle on cmake file
c6a8372 <frank400> Delete unnecessary holder
4be6cc6 <frank400> Fix redefined function
b78085a <frank400> Fix miss include
2efd43e <frank400> Implement ilike function

Authored-by: frank400 <[email protected]>
Signed-off-by: Praveen <[email protected]>
(cherry picked from commit 0072c67)
@anthonylouisbsb anthonylouisbsb deleted the feature/implement-sql-ilike branch September 28, 2021 22:32
zhouyuan pushed a commit to zhouyuan/arrow that referenced this pull request Nov 24, 2021
Closes apache#10179 from jvictorhuguenin/feature/implement-sql-ilike and squashes the following commits:

f160880 <frank400> Optimize holder constructor call
97e6e2d <frank400> Remove unnecessary Make method
c2363b1 <frank400> Disable TryOptimize for ilike
a484149 <frank400> Fix checkstyle on cmake file
c6a8372 <frank400> Delete unnecessary holder
4be6cc6 <frank400> Fix redefined function
b78085a <frank400> Fix miss include
2efd43e <frank400> Implement ilike function

Authored-by: frank400 <[email protected]>
Signed-off-by: Praveen <[email protected]>
zhouyuan added a commit to oap-project/arrow that referenced this pull request Nov 25, 2021
* ARROW-11960: [C++][Gandiva] Support escape in LIKE

Add gdv_fn_like_utf8_utf8_int8 function in Gandiva to support escape char in LIKE. An escape char is stored in an int8 type which is compatible with char type in C++.

Closes apache#9700 from Crystrix/arrow-11960

Authored-by: crystrix <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>

* ARROW-12567: [C++][Gandiva] Implement ILIKE SQL function

Closes apache#10179 from jvictorhuguenin/feature/implement-sql-ilike and squashes the following commits:

f160880 <frank400> Optimize holder constructor call
97e6e2d <frank400> Remove unnecessary Make method
c2363b1 <frank400> Disable TryOptimize for ilike
a484149 <frank400> Fix checkstyle on cmake file
c6a8372 <frank400> Delete unnecessary holder
4be6cc6 <frank400> Fix redefined function
b78085a <frank400> Fix miss include
2efd43e <frank400> Implement ilike function

Authored-by: frank400 <[email protected]>
Signed-off-by: Praveen <[email protected]>

* ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva

Closes apache#10059 from rodrigojdebem/feature/implement-regexp-replace and squashes the following commits:

baf2778 <rodrigojdebem> Add implementation for REGEXP_REPLACE

Authored-by: rodrigojdebem <[email protected]>
Signed-off-by: Praveen <[email protected]>

Co-authored-by: crystrix <[email protected]>
Co-authored-by: frank400 <[email protected]>
Co-authored-by: rodrigojdebem <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants