Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-88311044
@gabrielreid I tried to do this, but messed it up and created a new Pull
request 61. I am closing this request.
---
If your project is set up for it, you can reply
Github user naveenmadhire closed the pull request at:
https://github.com/apache/phoenix/pull/47
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-88218650
Looks good, and I think that the test cases cover what needs to be covered.
Could you rebase the patch on the master branch, and squash the commits
into a si
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-87907541
@gabrielreid I didn't realize we have pre defined method already available
in stringUtil. I've modified the code accordingly and all the test cases are
working fine
Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-85666137
The tests you've added look good. It looks like the reason that you're
getting issues with the DESC tests is because you're returning the SortOrder of
the underlying
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-84472511
@gabrielreid I wrote the required test cases in InstrFunctionIT.java. All
ASC test cases are working fine. But for DESC I am getting inverted value as
output. For e
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-83573791
I am having some configuration issues when running a end-2-end test cases.
I will figure out today and run that.
---
If your project is set up for it, you can repl
Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-83401354
@naveenmadhire the added test cases look good, but it looks like your
most-recent commit also removed the integration test.
The integration test should includ
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-82716115
@gabrielreid I added all the changes which you mentioned. Also I created
testcases for all types of combinations, including multibytes and empty
strings.
---
If
GitHub user naveenmadhire reopened a pull request:
https://github.com/apache/phoenix/pull/47
PHOENIX-1712 Adding INSTR Function
PHOENIX-1712
I've added the INSTR function. These are initial changes.
Please review if anyone has some time.
Thanks.
You can merge this p
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-82606330
Thanks Gabriel. I will work on adding some more test cases to this and
changes mentioned.
---
If your project is set up for it, you can reply to this email and hav
Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-82584873
A couple of additional test cases which would also be good:
* testing with strings that contains multi-byte characters
* testing with empty (but not null) strin
Github user gabrielreid commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/47#discussion_r26612650
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apac
Github user gabrielreid commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/47#discussion_r26612500
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apac
Github user gabrielreid commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/47#discussion_r26612452
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apac
Github user gabrielreid commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/47#discussion_r26612378
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apac
Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-82582761
This code looks pretty good (with the exception of a few code-formatting
issues). I think that the test coverage should be extended a bit though, with
the following t
Github user naveenmadhire commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-82285730
Adding some more changes related to Test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your pr
Github user naveenmadhire closed the pull request at:
https://github.com/apache/phoenix/pull/47
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
GitHub user naveenmadhire opened a pull request:
https://github.com/apache/phoenix/pull/47
PHOENIX-1712 Adding INSTR Function
PHOENIX-1712
I've added the INSTR function. These are initial changes.
Please review if anyone has some time.
Thanks.
You can merge this pul
20 matches
Mail list logo
