You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Venkata krishnan Sowrirajan <vs...@maprtech.com> on 2015/01/07 20:43:10 UTC

Review Request 29666: DRILL-1757 Add support for wildcards within REPEATED_CONTAINS

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29666/
-----------------------------------------------------------

Review request for drill and Mehant Baid.


Repository: drill-git


Description
-------

Support for wildcards with in repeated_contains() is implemented similar to MS SQL contains(). MS SQL contains() documentation http://msdn.microsoft.com/en-us/library/ms187787.aspx 


Diffs
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java f7f0e91 

Diff: https://reviews.apache.org/r/29666/diff/


Testing
-------


Thanks,

Venkata krishnan Sowrirajan


Re: Review Request 29666: DRILL-1757 Add support for wildcards within REPEATED_CONTAINS

Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29666/#review71766
-----------------------------------------------------------


Code changes look ok. Could you please add a couple of unit tests.

- Mehant Baid


On Jan. 7, 2015, 7:43 p.m., Venkata krishnan Sowrirajan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29666/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 7:43 p.m.)
> 
> 
> Review request for drill and Mehant Baid.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Support for wildcards with in repeated_contains() is implemented similar to MS SQL contains(). MS SQL contains() documentation http://msdn.microsoft.com/en-us/library/ms187787.aspx 
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java f7f0e91 
> 
> Diff: https://reviews.apache.org/r/29666/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Venkata krishnan Sowrirajan
> 
>


Re: Review Request 29666: DRILL-1757 Add support for wildcards within REPEATED_CONTAINS

Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29666/#review74686
-----------------------------------------------------------

Ship it!



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java
<https://reviews.apache.org/r/29666/#comment121356>

    I think it would be better if you use matches() instead of find(). Since you are invoking reset() above it will get the same result however invoking matches() explicitly states the intention of looking for the pattern in the entire string input.


Looks ok, just one very minor comment.

- Mehant Baid


On Feb. 19, 2015, 8:17 p.m., Venkata krishnan Sowrirajan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29666/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2015, 8:17 p.m.)
> 
> 
> Review request for drill and Mehant Baid.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Support for wildcards with in repeated_contains() is implemented similar to MS SQL contains(). MS SQL contains() documentation http://msdn.microsoft.com/en-us/library/ms187787.aspx 
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java f7f0e91 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewSimpleRepeatedFunctions.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29666/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Venkata krishnan Sowrirajan
> 
>


Re: Review Request 29666: DRILL-1757 Add support for wildcards within REPEATED_CONTAINS

Posted by Venkata krishnan Sowrirajan <vs...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29666/
-----------------------------------------------------------

(Updated Feb. 19, 2015, 8:17 p.m.)


Review request for drill and Mehant Baid.


Changes
-------

Last patch is not completely viewable. So added a new working one.


Repository: drill-git


Description
-------

Support for wildcards with in repeated_contains() is implemented similar to MS SQL contains(). MS SQL contains() documentation http://msdn.microsoft.com/en-us/library/ms187787.aspx 


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java f7f0e91 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewSimpleRepeatedFunctions.java PRE-CREATION 

Diff: https://reviews.apache.org/r/29666/diff/


Testing
-------


Thanks,

Venkata krishnan Sowrirajan


Re: Review Request 29666: DRILL-1757 Add support for wildcards within REPEATED_CONTAINS

Posted by Venkata krishnan Sowrirajan <vs...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29666/
-----------------------------------------------------------

(Updated Feb. 19, 2015, 8:13 p.m.)


Review request for drill and Mehant Baid.


Changes
-------

Added new couple test cases for the changes


Repository: drill-git


Description
-------

Support for wildcards with in repeated_contains() is implemented similar to MS SQL contains(). MS SQL contains() documentation http://msdn.microsoft.com/en-us/library/ms187787.aspx 


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java f7f0e91 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewSimpleRepeatedFunctions.java PRE-CREATION 

Diff: https://reviews.apache.org/r/29666/diff/


Testing
-------


Thanks,

Venkata krishnan Sowrirajan