pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1790609445
> Even better, since unicode standard seems to define a max sortable code
point `U+`, we should use that instead of `U+10`.
Erlang itself supports `U+10` and it is
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1790366824
thanks @nickva - this solution looks good in my tests using the CentoOS
container
--
This is an automated message from the Apache Git Service.
To respond to the message, please log
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1790228128
Thanks @nickva! I will experiment with the options you described above and
get back to you with the results.
--
This is an automated message from the Apache Git Service.
To respond to the
nickva commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1790141900
> Sure but what should be that value specifically? My impression is that
0x was created for that exact purpose because it was missing before. It
would be good to know how others have
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1789425187
> Another sentinel value might work, good idea.
Sure but what should be that value specifically? My impression is that
0x was created for that exact purpose because it was missing
nickva commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1789416560
Another sentinel value might work, good idea.
CentOS is EOL next year so then we can deprecate support for it and remove a
few hacks we have for it.
--
This is an automated
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788849130
> whether there's a pragmatically high code point we could use as the high
bound
I was thinking about the same. Unfortunately I am not that experienced
with Unicode collation and
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788606504
I wonder whether there's a pragmatically high code point we could use as the
high bound in `$beginsWith` until we drop support for Centos 7 (it goes out of
support next year)?
--
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788538868
Looking at https://icu.unicode.org/download/, it looks like there was a
[rewrite of the collator](https://icu.unicode.org/design/collation/v2) for ICU
53 which includes better support
nickva commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788364777
@pgj excellent find!
> Given that the other issue was fixed by unconditionally applying a shim on
top of the ICU string comparison call, we should do the same here? @nickva what
do
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1788169233
Yeah, basically [the lack of support for `0x` in older ICU
versions](https://www.unicode.org/reports/tr35/tr35-collation.html#tailored_noncharacter_weights)
strikes again here, similar
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1787413943
It is the derived string upper limit that does not work. Consider the
following:
```erlang
> Lower = <<"A">>.
<<"A">>.
> Upper = <>.
<<"A\377">>.
```
On CentOS
big-r81 commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1787391987
@pgj can you test on your local CentOS vm the state before the PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1787188105
> it definitely smells like a collation issue and likely an issue with the
special characters used in the tests
In my understanding, the two failing test cases do not feature any
big-r81 commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786846495
@willholley np, only a reminder. We will add this review-branch-protection
in the future. At the moment, we will get the "green" merge button, if the CI
runs through...
The
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786790838
curious that the failure didn't show in the PR checks - it definitely smells
like a collation issue and likely an issue with the special characters used in
the tests rather than the
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786788940
@big-r81 oops on the premature merge - I'm too used to working with repos
with branch protection enabled in GH and didn't think to manually check for the
+1.
--
This is an
nickva commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786501544
> It is not flaky, these errors happen every time -- but only on CentOS 7.
Oh very interesting, definitely worth investigating. One thing that
immediately jumps to mind is that
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786470992
It is not flaky, these errors happen every time -- but only on CentOS 7. I
was able to reproduce the issue locally with the help of a container created
from the
nickva commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1786375979
It probably a flaky test. We should investigate as we've been trying to
squash some of those lately. Is the race condition fairly obvious? Usually we
can add a retry loop.
--
This is
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785880172
Locally (with Dreyfus/Clouseau enabled) `make mango-test` works for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785876361
Hrm... Something is wrong with the merged version. CI says the following:
```console
[2023-10-30T18:57:57.544Z] FAIL: test_basic
(25-beginswith-test.BeginsWithOperator)
big-r81 commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785861835
@pgj It was only a small friendly reminder btw, this reminds me of #4772...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785849980
@big-r81 we have been in contact with @willholley about the PR out-of-band.
On merging the PR he was aware that I did not have any more pending comments on
the contents. I wanted to
big-r81 commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785845249
@willholley
>Typically, CouchDB uses the
[Review-Then-Commit](http://www.apache.org/foundation/glossary.html#ReviewThenCommit)
(RTC) model of code collaboration, not
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1785802556
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe,
willholley merged PR #4810:
URL: https://github.com/apache/couchdb/pull/4810
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail:
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1376324428
##
src/mango/test/03-operator-test.py:
##
@@ -10,12 +10,13 @@
# License for the specific language governing permissions and limitations under
# the License.
+from
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1375881371
##
src/docs/src/api/database/find.rst:
##
@@ -754,8 +760,10 @@ can itself be another operator with arguments of its own.
This enables us to
build up more complex
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1375875992
##
src/mango/test/03-operator-test.py:
##
@@ -141,6 +142,38 @@ def test_exists_false_returns_missing_but_not_null(self):
for d in docs:
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1783387589
_1.3.6.1.8. Condition Operators_ seems to be missing `$beginsWith`:
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1783384996
In the documentation, _1.3.6.1.1. Selectors Basics_ should also talk about
`$beginsWith`?
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1374883989
##
src/docs/src/api/database/find.rst:
##
@@ -754,8 +760,10 @@ can itself be another operator with arguments of its own.
This enables us to
build up more complex
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1781399634
@pgj I think I've addressed all the comments, though will need to squash the
commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1373322353
##
src/mango/src/mango_selector_text.erl:
##
@@ -142,6 +142,10 @@ convert(Path, {[{<<"$exists">>, ShouldExist}]}) ->
true -> FieldExists;
false ->
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1372066930
##
src/mango/src/mango_selector_text.erl:
##
@@ -142,6 +142,10 @@ convert(Path, {[{<<"$exists">>, ShouldExist}]}) ->
true -> FieldExists;
false ->
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1372064169
##
src/mango/src/mango_selector_text.erl:
##
@@ -142,6 +142,10 @@ convert(Path, {[{<<"$exists">>, ShouldExist}]}) ->
true -> FieldExists;
false ->
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1779595383
Okay. Right now there is a failing test case (for the `text` indexes):
```console
==
FAIL: test_beginswith
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1370565547
##
src/mango/src/mango_selector_text.erl:
##
@@ -142,6 +142,10 @@ convert(Path, {[{<<"$exists">>, ShouldExist}]}) ->
true -> FieldExists;
false ->
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1777336779
> Please add `text` tests for the integration test suite. I am fine if they
do not fully work (like a draft) I can test them and provide feedback. For your
information, I am currently
willholley commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1777115697
> Please add `text` tests for the integration test suite. I am fine if they
do not fully work (like a draft) I can test them and provide feedback. For your
information, I am currently
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1370081478
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1370074894
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1773770413
Please add `text` tests for the integration test suite. I am fine if they
do not fully work (like a draft) I can test them and provide feedback. For
your information, I am currently
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1367718710
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-1773768295
I miss the parts related to the documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1367716828
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,128 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1367716043
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1367715142
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1367714649
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1365581220
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1365554970
##
src/mango/test/25-beginswith-test.py:
##
@@ -0,0 +1,112 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except
nickva commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1364096728
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
nickva commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1364096728
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
willholley commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1363693652
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
pgj commented on PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#issuecomment-176756
Great work! I think it would be worth to cover the `text` indexes with
tests in this PR otherwise it can easily go forgotten.
--
This is an automated message from the Apache Git Service.
pgj commented on code in PR #4810:
URL: https://github.com/apache/couchdb/pull/4810#discussion_r1363488771
##
src/mango/src/mango_selector.erl:
##
@@ -1054,4 +1061,29 @@ fields_nor_test() ->
},
?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)).
willholley opened a new pull request, #4810:
URL: https://github.com/apache/couchdb/pull/4810
## Overview
Adds a `$beginsWith` operator to selectors, with json and text index
support. This is a compliment / precursor to optimising `$regex` support as
proposed in
58 matches
Mail list logo
