You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "john-wagster (via GitHub)" <gi...@apache.org> on 2025/02/03 19:05:00 UTC
[PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
john-wagster opened a new pull request, #14192:
URL: https://github.com/apache/lucene/pull/14192
About four years ago ASCII-only case insensitive matching (https://github.com/apache/lucene-solr/pull/1541) was added to Lucene. In the past couple of a years a couple of requests have been made related to case insensitive matching in Elasticsearch across other parts of UTF-8 which uses the `RegExp` regex `Automaton` in Lucene. Previous discussions around this for the ASCII-only work suggested that this task may be controversial. So I've spent a bit of time exploring options and have submitted this PR as, I believe, the best direction to take related to that support, but welcome feedback on this approach.
tl;dr the approach I've taken is to mirror `java.util.regex.Pattern` with the belief that any downstream users of products like Elasticsearch would expect and welcome consistency in Unicode's edge-cases with case insensitivity being handled the same between both Java's `Pattern` class and Lucene's `RegExp` class
More specifically @jpountz brought up concerns around the handling of characters such as sigma and it's variants (Σ, σ, ς) (https://github.com/apache/lucene-solr/pull/1541#discussion_r441002695). I spent some time investigating all of the characters in Unicode and tried to explain edge cases within the `RegExp` class by enumerating the classes of characters and their behaviors so we can easily discuss or pivot as desired. I then opted to handle these special classes of characters the same as how `java.util.regex.Pattern` handles these. Often (though I haven't tested all code points) Perl regex seems to treat case insensitivity the same as the `Pattern` class. So for instance the `Pattern` class when using both `Pattern.CASE_INSENSITIVE` and `Pattern.UNICODE_CASE` matching flags will treat the three sigma characters (Σ, σ, ς) as the same for the purposes of matching; so for instance `σ` and `ς` are a positive match in a case insensitive regex even though they are not th
emselves in a case sensitive context (they are both lowercase).
For this PR I've opted to maintain the best performance possible scenario so special cases are handled with a lookup table while compiling the regex to an `Automaton`. Matching then must only consider the necessary alternative characters to ensure a match without any additional consideration for case sensitivity.
While most characters can be easily handled by uppercasing or lowercasing them appropriately outside the ASCII range, during my review of the Unicode spec I encountered three distinct classes of characters in Unicode that can be problematic.
* Class 1 is the set of characters such as sigma that match other characters outside of the immediate `toUpperCase` and `toLowerCase` forms. However, in spidering the Unicode table with some utility classes I found that no more than four characters were ever included in a set of matched alternatives.
* Class 2 is the set of the characters that have a both a different and distinct `toUpperCase` and `toLowerCase` form. An example of this is: `Dž` whose upper case form is `DŽ` and whose lower case form is `dž`.
* Class 3 is the set of characters that have some cased form that transitions the Basic Multilingual Plane (BMP). These sets of characters are typically not matched by `java.util.regex.Pattern` (for likely performance reasons as they transition from say 2 byte representations to 4 byte representations in UTF-8) and so are explicitly excluded from matching here as well, which as it turns out is relatively easy as the Java's `Character` class `toUpperCase` and `toLowerCase` methods for code points do not case these characters, which we were already using in RegExp. It's worth noting that supporting this is completely possible and Java's `String` class `toUpperCase` and `toLowerCase` methods do correctly transition characters across the BMP. Lastly it's worth noting that these classes can and do overlap and so if a character is in both class 3 and class 2 for instance then again we defer the behavior seen in `java.util.regex.Pattern` and ignore the casing across the BMP but do cas
e appropriately otherwise such as with `ῼ` which will match it's lowercase form `ῳ` in the BMP but not it's uppercase form outside the BMP `ΩΙ`.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2667183580
@john-wagster this looks great! Thank you for simplifying this down as first step. I will merge it in after CI checks.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2631833927
@jpountz, @jimczi, @mayya-sharipova ya'll may be interested in this PR so just tagging you here in case you are interested.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1946735208
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFoldingUtil.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.HexFormat;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Generates the case folding set of alternate character mappings based on this spec:
+ * https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
Review Comment:
i would lean on those, trust me. I think adding text files and multiple .java files is overkill to start with on this PR?
Main thing I wanted, was a proper explanation of what the "magic numbers are". The if-then-else or switch statement like you had is fine, it just needs to be explained so we can reason about it and laid out in a way we can maintain it if needed, and ensure correctness.
Here's a REALLY big switch statement as example: https://github.com/apache/lucene/blob/main/lucene/analysis/common/src/java/org/apache/lucene/analysis/miscellaneous/ASCIIFoldingFilter.java#L370-L376
See how the code is easy to review just because of how its formatted, with hex notation, comment with character, and character name? And if you look at top of the file it references where they come from.
So e.g. in your case, you just add comment linking to specialcasing or whatever the source is, format the exceptional cases better, and so on. My gut is, list of exceptions should be small and very stable and then it is easy on us.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1942009773
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
+ entry(924, new int[] {181, 956}),
+ entry(956, new int[] {181, 924}),
+ entry(304, new int[] {105, 73, 305}),
+ entry(105, new int[] {304, 73, 305}),
+ entry(73, new int[] {304, 105, 305}),
+ entry(305, new int[] {304, 105, 73}),
+ entry(383, new int[] {83, 115}),
+ entry(83, new int[] {383, 115}),
+ entry(115, new int[] {383, 83}),
+ entry(837, new int[] {921, 953, 8126}),
+ entry(921, new int[] {837, 953, 8126}),
+ entry(953, new int[] {837, 921, 8126}),
+ entry(8126, new int[] {837, 921, 953}),
+ entry(962, new int[] {931, 963}),
+ entry(931, new int[] {962, 963}),
+ entry(963, new int[] {962, 931}),
+ entry(976, new int[] {914, 946}),
+ entry(914, new int[] {976, 946}),
+ entry(946, new int[] {976, 914}),
+ entry(977, new int[] {920, 952, 1012}),
+ entry(920, new int[] {977, 952, 1012}),
+ entry(952, new int[] {977, 920, 1012}),
+ entry(1012, new int[] {977, 920, 952}),
+ entry(981, new int[] {934, 966}),
+ entry(934, new int[] {981, 966}),
+ entry(966, new int[] {981, 934}),
+ entry(982, new int[] {928, 960}),
+ entry(928, new int[] {982, 960}),
+ entry(960, new int[] {982, 928}),
+ entry(1008, new int[] {922, 954}),
+ entry(922, new int[] {1008, 954}),
+ entry(954, new int[] {1008, 922}),
+ entry(1009, new int[] {929, 961}),
+ entry(929, new int[] {1009, 961}),
+ entry(961, new int[] {1009, 929}),
+ entry(1013, new int[] {917, 949}),
+ entry(917, new int[] {1013, 949}),
+ entry(949, new int[] {1013, 917}),
+ entry(7296, new int[] {1042, 1074}),
+ entry(1042, new int[] {7296, 1074}),
+ entry(1074, new int[] {7296, 1042}),
+ entry(7297, new int[] {1044, 1076}),
+ entry(1044, new int[] {7297, 1076}),
+ entry(1076, new int[] {7297, 1044}),
+ entry(7298, new int[] {1054, 1086}),
+ entry(1054, new int[] {7298, 1086}),
+ entry(1086, new int[] {7298, 1054}),
+ entry(7299, new int[] {1057, 1089}),
+ entry(1057, new int[] {7299, 1089}),
+ entry(1089, new int[] {7299, 1057}),
+ entry(7300, new int[] {1058, 1090, 7301}),
+ entry(1058, new int[] {7300, 1090, 7301}),
+ entry(1090, new int[] {7300, 1058, 7301}),
+ entry(7301, new int[] {7300, 1058, 1090}),
+ entry(7302, new int[] {1066, 1098}),
+ entry(1066, new int[] {7302, 1098}),
+ entry(1098, new int[] {7302, 1066}),
+ entry(7303, new int[] {1122, 1123}),
+ entry(1122, new int[] {7303, 1123}),
+ entry(1123, new int[] {7303, 1122}),
+ entry(7304, new int[] {42570, 42571}),
+ entry(42570, new int[] {7304, 42571}),
+ entry(42571, new int[] {7304, 42570}),
+ entry(7835, new int[] {7776, 7777}),
+ entry(7776, new int[] {7835, 7777}),
+ entry(7777, new int[] {7835, 7776}),
+ entry(8486, new int[] {969, 937}),
+ entry(969, new int[] {8486, 937}),
+ entry(937, new int[] {8486, 969}),
+ entry(8490, new int[] {107, 75}),
+ entry(107, new int[] {8490, 75}),
+ entry(75, new int[] {8490, 107}),
+ entry(8491, new int[] {229, 197}),
+ entry(229, new int[] {8491, 197}),
+ entry(197, new int[] {8491, 229}),
Review Comment:
Yeah, I think we want the simple "L1" 1-1 case folding, no expansion/contraction/normalization/turkish-azeri alt/etc.
A big part of the problem is that java offers NO method to support this, which is why you are having to create lists of exceptions yourself. java only offers Character.toUpperCase()/toLowerCase()/toTitleCase(), which are for **display** purposes.
from i18n perspective, if i had one feature request for openjdk from the unicode, it would be to add `Character.foldCase()` which is for **case insensitivity** and is different. Imagine if you had this method in Character.java, your PR would be very tiny: https://unicode-org.github.io/icu-docs/apidoc/released/icu4j/com/ibm/icu/lang/UCharacter.html#foldCase-int-boolean-
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1939974637
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
+ entry(924, new int[] {181, 956}),
+ entry(956, new int[] {181, 924}),
+ entry(304, new int[] {105, 73, 305}),
+ entry(105, new int[] {304, 73, 305}),
+ entry(73, new int[] {304, 105, 305}),
+ entry(305, new int[] {304, 105, 73}),
+ entry(383, new int[] {83, 115}),
+ entry(83, new int[] {383, 115}),
+ entry(115, new int[] {383, 83}),
+ entry(837, new int[] {921, 953, 8126}),
+ entry(921, new int[] {837, 953, 8126}),
+ entry(953, new int[] {837, 921, 8126}),
+ entry(8126, new int[] {837, 921, 953}),
+ entry(962, new int[] {931, 963}),
+ entry(931, new int[] {962, 963}),
+ entry(963, new int[] {962, 931}),
+ entry(976, new int[] {914, 946}),
+ entry(914, new int[] {976, 946}),
+ entry(946, new int[] {976, 914}),
+ entry(977, new int[] {920, 952, 1012}),
+ entry(920, new int[] {977, 952, 1012}),
+ entry(952, new int[] {977, 920, 1012}),
+ entry(1012, new int[] {977, 920, 952}),
+ entry(981, new int[] {934, 966}),
+ entry(934, new int[] {981, 966}),
+ entry(966, new int[] {981, 934}),
+ entry(982, new int[] {928, 960}),
+ entry(928, new int[] {982, 960}),
+ entry(960, new int[] {982, 928}),
+ entry(1008, new int[] {922, 954}),
+ entry(922, new int[] {1008, 954}),
+ entry(954, new int[] {1008, 922}),
+ entry(1009, new int[] {929, 961}),
+ entry(929, new int[] {1009, 961}),
+ entry(961, new int[] {1009, 929}),
+ entry(1013, new int[] {917, 949}),
+ entry(917, new int[] {1013, 949}),
+ entry(949, new int[] {1013, 917}),
+ entry(7296, new int[] {1042, 1074}),
+ entry(1042, new int[] {7296, 1074}),
+ entry(1074, new int[] {7296, 1042}),
+ entry(7297, new int[] {1044, 1076}),
+ entry(1044, new int[] {7297, 1076}),
+ entry(1076, new int[] {7297, 1044}),
+ entry(7298, new int[] {1054, 1086}),
+ entry(1054, new int[] {7298, 1086}),
+ entry(1086, new int[] {7298, 1054}),
+ entry(7299, new int[] {1057, 1089}),
+ entry(1057, new int[] {7299, 1089}),
+ entry(1089, new int[] {7299, 1057}),
+ entry(7300, new int[] {1058, 1090, 7301}),
+ entry(1058, new int[] {7300, 1090, 7301}),
+ entry(1090, new int[] {7300, 1058, 7301}),
+ entry(7301, new int[] {7300, 1058, 1090}),
+ entry(7302, new int[] {1066, 1098}),
+ entry(1066, new int[] {7302, 1098}),
+ entry(1098, new int[] {7302, 1066}),
+ entry(7303, new int[] {1122, 1123}),
+ entry(1122, new int[] {7303, 1123}),
+ entry(1123, new int[] {7303, 1122}),
+ entry(7304, new int[] {42570, 42571}),
+ entry(42570, new int[] {7304, 42571}),
+ entry(42571, new int[] {7304, 42570}),
+ entry(7835, new int[] {7776, 7777}),
+ entry(7776, new int[] {7835, 7777}),
+ entry(7777, new int[] {7835, 7776}),
+ entry(8486, new int[] {969, 937}),
+ entry(969, new int[] {8486, 937}),
+ entry(937, new int[] {8486, 969}),
+ entry(8490, new int[] {107, 75}),
+ entry(107, new int[] {8490, 75}),
+ entry(75, new int[] {8490, 107}),
+ entry(8491, new int[] {229, 197}),
+ entry(229, new int[] {8491, 197}),
+ entry(197, new int[] {8491, 229}),
Review Comment:
For these big sets of numbers, can we express in some UnicodeSet(s) or properties from unicode data files (e.g. has full case-folding) to understand where they are coming from?
It helps to verify and keep the list updated.
For example, maybe this is some of them (maybe not, it is just an example, as I can't tell if this code is trying to handle characters that expand due to normalization, casing, or both)
```
[[:Expands_On_NFKC=Yes:]&[[:Changes_When_Casefolded:][:Changes_When_Uppercased:][:Changes_When_Lowercased:]]]
```
https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5B%5B%3AExpands_On_NFKC%3DYes%3A%5D%26%5B%5B%3AChanges_When_Casefolded%3A%5D%5B%3AChanges_When_Uppercased%3A%5D%5B%3AChanges_When_Lowercased%3A%5D%5D%5D&g=&i=
https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1939946041
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
Review Comment:
please, use hex for the unicode everywhere
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -424,6 +426,46 @@ public enum Kind {
/** Allows case insensitive matching of ASCII characters. */
public static final int ASCII_CASE_INSENSITIVE = 0x0100;
+ /**
+ * Allows case-insensitive matching of most Unicode characters.
+ *
+ * <p>In general the attempt is to reach parity with {@link java.util.regex.Pattern}
+ * Pattern.CASE_INSENSITIVE and Pattern.UNICODE_CASE flags when doing a case-insensitive match.
+ * This means characters like those representing the Greek symbol sigma (Σ, σ, ς) will all match
+ * one another
+ *
+ * <p>Some Unicode characters are difficult to correctly decode casing. In some cases Java's
+ * String class correctly handles decoding these but Java's {@link java.util.regex.Pattern} class
+ * does not. Again to keep parity and for performance reasons we are maintaining consistency with
+ * {@link java.util.regex.Pattern}. There are three known special classes of these characters we
+ * term unstable:
+ *
+ * <ul>
+ * <li>1. the set of characters whose casing matches across multiple characters such as the
+ * Greek sigma character mentioned above (Σ, σ, ς); we support these; notably some of these
+ * characters fall into the ASCII range and so will behave differently when this flag is
+ * enabled
+ * <li>2. the set of characters that are neither in an upper nor lower case stable state and can
+ * be both uppercased and lowercased from their current code point such as Dž which when
+ * uppercased produces DŽ and when lowercased produces dž; we support these
+ * <li>3. the set of characters that transition into or out of the Basic Multilingual Plane
+ * (BMP). For performance reasons we ignore characters that transition for now, which is
+ * consistent with {@link java.util.regex.Pattern}
+ * </ul>
+ *
+ * <p>Sometimes these classes of character will overlap; if a character is in both class 3 and any
+ * other case listed above it is ignored for the characters that transition; this is consistent
+ * with {@link java.util.regex.Pattern}. For instance: this character ῼ will match it's lowercase
+ * form ῳ in the BMP but not it's uppercase form outside the BMP: ΩΙ
+ *
+ * <p>These are the set of known characters that transition in or out of the BMP when cased such
+ * as ﬗ (code point: 64279 with 2 bytes) which when uppercased produces ՄԽ (code points: 1348 1341
+ * with 4 bytes) and are therefore ignored: 223, 304, 329, 496, 912, 944, 1415, 7830-7834, 8016,
+ * 8018, 8020, 8022, 8064-8111, 8114-8116, 8118, 8119, 8124, 8130-8132, 8134, 8135, 8140, 8146,
+ * 8147, 8150, 8151, 8162-8164, 8166-8180, 8182, 8183, 8188, 64256-64262, 64275-64279
+ */
Review Comment:
I don't understand this comment, where is this "in our out of BMP transitioning":`
ﬗ is U+FB17 `ARMENIAN SMALL LIGATURE MEN XEH`
ՄԽ is U+0544 `ARMENIAN CAPITAL LETTER MEN` followed by U+053D `ARMENIAN CAPITAL LETTER XEH`
All of this is in the BMP. So we need a better explanation of what this is all about since it isn't that.
Using hex would help when reading it, but even with the decimal numbers, you can see that nothing in this comment is outside of the BMP, it is all less than 64k.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1939944485
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -424,6 +426,46 @@ public enum Kind {
/** Allows case insensitive matching of ASCII characters. */
public static final int ASCII_CASE_INSENSITIVE = 0x0100;
+ /**
+ * Allows case-insensitive matching of most Unicode characters.
+ *
+ * <p>In general the attempt is to reach parity with {@link java.util.regex.Pattern}
+ * Pattern.CASE_INSENSITIVE and Pattern.UNICODE_CASE flags when doing a case-insensitive match.
+ * This means characters like those representing the Greek symbol sigma (Σ, σ, ς) will all match
+ * one another
+ *
+ * <p>Some Unicode characters are difficult to correctly decode casing. In some cases Java's
+ * String class correctly handles decoding these but Java's {@link java.util.regex.Pattern} class
+ * does not. Again to keep parity and for performance reasons we are maintaining consistency with
+ * {@link java.util.regex.Pattern}. There are three known special classes of these characters we
+ * term unstable:
+ *
+ * <ul>
+ * <li>1. the set of characters whose casing matches across multiple characters such as the
+ * Greek sigma character mentioned above (Σ, σ, ς); we support these; notably some of these
+ * characters fall into the ASCII range and so will behave differently when this flag is
+ * enabled
+ * <li>2. the set of characters that are neither in an upper nor lower case stable state and can
+ * be both uppercased and lowercased from their current code point such as Dž which when
+ * uppercased produces DŽ and when lowercased produces dž; we support these
+ * <li>3. the set of characters that transition into or out of the Basic Multilingual Plane
+ * (BMP). For performance reasons we ignore characters that transition for now, which is
+ * consistent with {@link java.util.regex.Pattern}
+ * </ul>
+ *
+ * <p>Sometimes these classes of character will overlap; if a character is in both class 3 and any
+ * other case listed above it is ignored for the characters that transition; this is consistent
+ * with {@link java.util.regex.Pattern}. For instance: this character ῼ will match it's lowercase
+ * form ῳ in the BMP but not it's uppercase form outside the BMP: ΩΙ
+ *
+ * <p>These are the set of known characters that transition in or out of the BMP when cased such
+ * as ﬗ (code point: 64279 with 2 bytes) which when uppercased produces ՄԽ (code points: 1348 1341
+ * with 4 bytes) and are therefore ignored: 223, 304, 329, 496, 912, 944, 1415, 7830-7834, 8016,
+ * 8018, 8020, 8022, 8064-8111, 8114-8116, 8118, 8119, 8124, 8130-8132, 8134, 8135, 8140, 8146,
+ * 8147, 8150, 8151, 8162-8164, 8166-8180, 8182, 8183, 8188, 64256-64262, 64275-64279
+ */
Review Comment:
I don't understand this comment, where is this "in our out of BMP transitioning":
ﬗ is U+FB17 `ARMENIAN SMALL LIGATURE MEN XEH`
ՄԽ is U+0544 `ARMENIAN CAPITAL LETTER MEN` followed by U+053D `ARMENIAN CAPITAL LETTER XEH`
All of this is in the BMP. So we need a better explanation of what this is all about since it isn't that.
Using hex would help when reading it, but even with the decimal numbers, you can see that nothing in this comment is outside of the BMP, it is all less than 64k.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1940403543
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
Review Comment:
didn't mean to come off short here,
But the hex is just a defacto standard, e.g. you'll see that representation in all the unicode data files. It helps to have an unambiguous way to refer to the same codepoint.
There's quite a few lucene code reviewers that can look at the hex digits, and recognize things, maybe how many bytes it takes up in UTF-8, maybe whether it is compatibility character that might explode (﷽), maybe what block it is in and so on.
Since they are all codepoints (`int`), it will help to do them all in hex like `0x1F602`, as you can represent all codepoints with it. Only bother with the `\uXXXX`, when dealing with java's UTF-16 `char` type.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2667222184
awesome! thank you sir.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1941963110
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -696,17 +896,52 @@ private Automaton toAutomaton(
return a;
}
- private Automaton toCaseInsensitiveChar(int codepoint) {
- Automaton case1 = Automata.makeChar(codepoint);
- // For now we only work with ASCII characters
- if (codepoint > 128) {
- return case1;
+ /**
+ * This function handles both ASCII and the remainder of the Unicode spec for generating
+ * case-insensitive alternatives. Specifically for Unicode some special handling is required
+ * particularly to ensure behavior is at parity with other regex engines like {@link
+ * java.util.regex.Pattern}
+ *
+ * <p>See the {@link #UNICODE_CASE_INSENSITIVE} flag for details on the set of known unstable
+ * alternative casings within the Unicode spec.
+ *
+ * @param codepoint the Character code point to encode as an Automaton
+ * @return the set of Automaton that represent the original code point and it's variants
+ */
+ private Automaton toCaseInsensitiveChar(
+ int codepoint, boolean isAsciiInsensitive, boolean isUnicodeInsensitive) {
+ Automaton result;
+ if (isAsciiInsensitive || isUnicodeInsensitive) {
+ if (isUnicodeInsensitive) {
+ int[] altCodepoints = unstableUnicodeCharacters.get(codepoint);
+ if (altCodepoints != null) {
+ List<Automaton> altAutomaton = new ArrayList<>(altCodepoints.length);
+ for (int i = 0; i < altCodepoints.length; i++) {
+ altAutomaton.add(Automata.makeChar(altCodepoints[i]));
+ }
+ altAutomaton.add(Automata.makeChar(codepoint));
+ result = Operations.union(altAutomaton);
Review Comment:
awesome; I took a brief look and it looked good to me. Part of the reason I was building these up in a List was related to a PR that @mayya-sharipova submitted recently where she found performance issues with unions: https://github.com/apache/lucene/pull/14169
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1940371117
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -696,17 +896,52 @@ private Automaton toAutomaton(
return a;
}
- private Automaton toCaseInsensitiveChar(int codepoint) {
- Automaton case1 = Automata.makeChar(codepoint);
- // For now we only work with ASCII characters
- if (codepoint > 128) {
- return case1;
+ /**
+ * This function handles both ASCII and the remainder of the Unicode spec for generating
+ * case-insensitive alternatives. Specifically for Unicode some special handling is required
+ * particularly to ensure behavior is at parity with other regex engines like {@link
+ * java.util.regex.Pattern}
+ *
+ * <p>See the {@link #UNICODE_CASE_INSENSITIVE} flag for details on the set of known unstable
+ * alternative casings within the Unicode spec.
+ *
+ * @param codepoint the Character code point to encode as an Automaton
+ * @return the set of Automaton that represent the original code point and it's variants
+ */
+ private Automaton toCaseInsensitiveChar(
+ int codepoint, boolean isAsciiInsensitive, boolean isUnicodeInsensitive) {
+ Automaton result;
+ if (isAsciiInsensitive || isUnicodeInsensitive) {
+ if (isUnicodeInsensitive) {
+ int[] altCodepoints = unstableUnicodeCharacters.get(codepoint);
+ if (altCodepoints != null) {
+ List<Automaton> altAutomaton = new ArrayList<>(altCodepoints.length);
+ for (int i = 0; i < altCodepoints.length; i++) {
+ altAutomaton.add(Automata.makeChar(altCodepoints[i]));
+ }
+ altAutomaton.add(Automata.makeChar(codepoint));
+ result = Operations.union(altAutomaton);
Review Comment:
I opened #14193 to try to improve this case. It is hard to think about the worst-cases and so on today, both the current ascii-only code and this patch could just use an `int[]` and get back a minimal automaton from what I see.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1939889352
##########
lucene/core/src/test/org/apache/lucene/util/automaton/TestRegExp.java:
##########
@@ -35,6 +43,320 @@ public void testSmoke() {
assertFalse(run.run("ad"));
}
+ public void testUnicodeAsciiInsensitiveFlags() {
+ RegExp r;
+ // ASCII behaves appropriately with different flags
+ r = new RegExp("A");
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("a"));
+
+ r = new RegExp("A", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("a"));
+
+ r = new RegExp("A", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("a"));
+
+ r =
+ new RegExp(
+ "A", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE | RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("a"));
+
+ // class 1 Unicode characters behaves appropriately with different flags
+ r = new RegExp("Σ");
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("σ"));
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r = new RegExp("σ");
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r = new RegExp("Σ", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE);
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("σ"));
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r = new RegExp("σ", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE);
+ assertFalse(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r = new RegExp("Σ", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("σ"));
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r = new RegExp("σ", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r =
+ new RegExp(
+ "Σ", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE | RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("σ"));
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ r =
+ new RegExp(
+ "σ", RegExp.ALL, RegExp.ASCII_CASE_INSENSITIVE | RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("ς"));
+
+ // class 2 Unicode characters behaves appropriately with different flags
+ r = new RegExp("ῼ", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertTrue(new CharacterRunAutomaton(r.toAutomaton()).run("ῳ"));
+
+ r = new RegExp("ῼ", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertFalse(
+ new CharacterRunAutomaton(r.toAutomaton()).run("ῼ".toUpperCase(Locale.ROOT))); // "ΩΙ"
+
+ // class 3 Unicode characters behaves appropriately with different flags
+ r = new RegExp("ﬗ", RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ assertFalse(
+ new CharacterRunAutomaton(r.toAutomaton()).run("ﬗ".toUpperCase(Locale.ROOT))); // "ՄԽ"
+ }
+
+ public void testUnicodeInsensitiveMatchPatternParity() {
+ // this ensures that if the Pattern class behavior were to change with a change to the Unicode
+ // spec then we would pick it up
+ // except new characters that were introduced (which would be a manual process; see tooling
+ // comments below)
+ for (Map.Entry<Integer, int[]> entry : RegExp.unstableUnicodeCharacters.entrySet()) {
+ int codePoint = entry.getKey();
+ int[] caseInsensititiveAlternatives = entry.getValue();
+ String pattern = new String(Character.toChars(codePoint));
+ Pattern javaRegex = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE);
+ RegExp r = new RegExp(pattern, RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ CharacterRunAutomaton cra = new CharacterRunAutomaton(r.toAutomaton());
+ for (int i = 0; i < caseInsensititiveAlternatives.length; i++) {
+
+ int alt = caseInsensititiveAlternatives[i];
+ String altString = new String(Character.toChars(alt));
+
+ assertTrue(javaRegex.matcher(altString).matches());
+ assertTrue(cra.run(altString));
+ }
+ }
+
+ // tooling code to validate manually to discover new characters that fall into the "unstable"
+ // set of Unicode characters
+ // variations of these can be used to discover the various classes themselves
+ // generateClass1();
+ // generateClass2();
+ // generateClass3();
+ }
+
+ public void testRandomUnicodeInsensitiveMatchPatternParity() {
+ int maxIters = 1000;
+ List<Integer> reservedCharacters =
+ Set.of(
+ '.', '^', '$', '*', '+', '?', '(', ')', '[', '{', '\\', '|', '-', '"', '<', '>',
+ '#', '@', '&', '~')
+ .stream()
+ .map(c -> (int) c)
+ .toList();
+ for (int i = 0; i < maxIters; i++) {
+ int nextCode1 = random().nextInt(0, Character.MAX_CODE_POINT + 1);
+ int nextCode2 = random().nextInt(0, Character.MAX_CODE_POINT + 1);
+
+ // skip if we select a reserved character that blows up .^$*+?()[{\|-]"<
+ if (reservedCharacters.contains(nextCode1)) {
+ continue;
+ }
+
+ String pattern = new String(Character.toChars(nextCode1));
+ String altString = new String(Character.toChars(nextCode2));
+
+ Pattern javaRegex = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE);
+ RegExp r = new RegExp(pattern, RegExp.ALL, RegExp.UNICODE_CASE_INSENSITIVE);
+ CharacterRunAutomaton cra = new CharacterRunAutomaton(r.toAutomaton());
+ assertEquals(
+ "Pattern and RegExp disagree on pattern: " + nextCode1 + " :text: " + nextCode2,
+ javaRegex.matcher(altString).matches(),
+ cra.run(altString));
+ }
+ }
+
+ public static void generateClass1() {
Review Comment:
Not sure where these utility functions should live; they are nice for validating and regenerating relevant code points so putting them here for now but open to suggestions for if / where they belong.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1940023844
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -424,6 +426,46 @@ public enum Kind {
/** Allows case insensitive matching of ASCII characters. */
public static final int ASCII_CASE_INSENSITIVE = 0x0100;
+ /**
+ * Allows case-insensitive matching of most Unicode characters.
+ *
+ * <p>In general the attempt is to reach parity with {@link java.util.regex.Pattern}
+ * Pattern.CASE_INSENSITIVE and Pattern.UNICODE_CASE flags when doing a case-insensitive match.
+ * This means characters like those representing the Greek symbol sigma (Σ, σ, ς) will all match
+ * one another
+ *
+ * <p>Some Unicode characters are difficult to correctly decode casing. In some cases Java's
+ * String class correctly handles decoding these but Java's {@link java.util.regex.Pattern} class
+ * does not. Again to keep parity and for performance reasons we are maintaining consistency with
+ * {@link java.util.regex.Pattern}. There are three known special classes of these characters we
+ * term unstable:
+ *
+ * <ul>
+ * <li>1. the set of characters whose casing matches across multiple characters such as the
+ * Greek sigma character mentioned above (Σ, σ, ς); we support these; notably some of these
+ * characters fall into the ASCII range and so will behave differently when this flag is
+ * enabled
+ * <li>2. the set of characters that are neither in an upper nor lower case stable state and can
+ * be both uppercased and lowercased from their current code point such as Dž which when
+ * uppercased produces DŽ and when lowercased produces dž; we support these
+ * <li>3. the set of characters that transition into or out of the Basic Multilingual Plane
+ * (BMP). For performance reasons we ignore characters that transition for now, which is
+ * consistent with {@link java.util.regex.Pattern}
+ * </ul>
+ *
+ * <p>Sometimes these classes of character will overlap; if a character is in both class 3 and any
+ * other case listed above it is ignored for the characters that transition; this is consistent
+ * with {@link java.util.regex.Pattern}. For instance: this character ῼ will match it's lowercase
+ * form ῳ in the BMP but not it's uppercase form outside the BMP: ΩΙ
+ *
+ * <p>These are the set of known characters that transition in or out of the BMP when cased such
+ * as ﬗ (code point: 64279 with 2 bytes) which when uppercased produces ՄԽ (code points: 1348 1341
+ * with 4 bytes) and are therefore ignored: 223, 304, 329, 496, 912, 944, 1415, 7830-7834, 8016,
+ * 8018, 8020, 8022, 8064-8111, 8114-8116, 8118, 8119, 8124, 8130-8132, 8134, 8135, 8140, 8146,
+ * 8147, 8150, 8151, 8162-8164, 8166-8180, 8182, 8183, 8188, 64256-64262, 64275-64279
+ */
Review Comment:
You are right; poking around a bit more to better understand what's happening here. I was debugging through the Pattern class to see how this was handled there and naively assumed that it was a single character but it's being uppercased into two characters within the BMP. Let me go back and double check these characters, revise the comment at least to be more generically calling out that we respect the same behavior as in Pattern related to not matching when going from a single code point to multiple code points and remove references to the BMP.
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
Review Comment:
will do
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1942013537
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -696,17 +896,52 @@ private Automaton toAutomaton(
return a;
}
- private Automaton toCaseInsensitiveChar(int codepoint) {
- Automaton case1 = Automata.makeChar(codepoint);
- // For now we only work with ASCII characters
- if (codepoint > 128) {
- return case1;
+ /**
+ * This function handles both ASCII and the remainder of the Unicode spec for generating
+ * case-insensitive alternatives. Specifically for Unicode some special handling is required
+ * particularly to ensure behavior is at parity with other regex engines like {@link
+ * java.util.regex.Pattern}
+ *
+ * <p>See the {@link #UNICODE_CASE_INSENSITIVE} flag for details on the set of known unstable
+ * alternative casings within the Unicode spec.
+ *
+ * @param codepoint the Character code point to encode as an Automaton
+ * @return the set of Automaton that represent the original code point and it's variants
+ */
+ private Automaton toCaseInsensitiveChar(
+ int codepoint, boolean isAsciiInsensitive, boolean isUnicodeInsensitive) {
+ Automaton result;
+ if (isAsciiInsensitive || isUnicodeInsensitive) {
+ if (isUnicodeInsensitive) {
+ int[] altCodepoints = unstableUnicodeCharacters.get(codepoint);
+ if (altCodepoints != null) {
+ List<Automaton> altAutomaton = new ArrayList<>(altCodepoints.length);
+ for (int i = 0; i < altCodepoints.length; i++) {
+ altAutomaton.add(Automata.makeChar(altCodepoints[i]));
+ }
+ altAutomaton.add(Automata.makeChar(codepoint));
+ result = Operations.union(altAutomaton);
Review Comment:
yeah that PR does not fix any hard problem (optimizing `union` and `concatenate` to be better in general). But it can fix your problem.
and we could test, but I'd guess that this PR should be very fast and we should consider the option of deprecating the previous ASCII option and just calling this one `CASE_INSENSITIVE`? because we can make the argument it is just as fast or faster than previous ASCII option and end out with a cleaner API and maintenance.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1949836434
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFolding.java:
##########
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.lucene.util.IOUtils;
+
+/**
+ * A utility class that allows downstream code to be consistent with how Lucene handles case folding
+ */
+public class CaseFolding {
Review Comment:
does this really need to be public? If so, can it offer an actual case-folding function: `fold(int) -> int`.
it doesn't offer that today, it just allows generating all the alternatives that have the same case folding: that method should have another name.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2667264982
```java
var re = new RegExp("παραστάσεις", RegExp.NONE, RegExp.CASE_INSENSITIVE);
System.out.println(re.toAutomaton().toDot());
```
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2665979362
Apologies @rmuir I forgot to request review after my last set of changes; just did so now.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1945855034
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFoldingUtil.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.HexFormat;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Generates the case folding set of alternate character mappings based on this spec:
+ * https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
Review Comment:
I think this one uses a too-huge file? We don't need all the casefolding mappings of unicode, just the exceptions?
Look at https://www.unicode.org/Public/16.0.0/ucd/SpecialCasing.txt which is only exceptional ones and should be much smaller to deal with.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2641159156
Iterated here a bit after the changes in https://github.com/apache/lucene/pull/14193 went in and also pivoted to using https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt. I still need to do some additional validation and work through a couple things here. So not done yet. But feedback of course is always welcome on this direction.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1946628495
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFoldingUtil.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.HexFormat;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Generates the case folding set of alternate character mappings based on this spec:
+ * https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
Review Comment:
Yep agreed. I want to take a look and compare. In the mean time I've been digging into the Character.toLowerCase and Character.toUpperCase code and it's more complex than I was hoping. I'm curious about the performance between them. I'm kind of leaning toward not using those though and the full CaseFolding.txt file is only around 24kb in memory and could be optimized if we are concerned about that size for any reason. Definitely appreciate your perspective here though. So curious about your thoughts. And in the meantime I'll continue to validate / explore the differences so we have more information.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1949691308
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFoldingUtil.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.HexFormat;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Generates the case folding set of alternate character mappings based on this spec:
+ * https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
Review Comment:
So relying on `SpecialCasing.txt` by itself is not going to work. It lacks several of the more common mappings like:
```
0x1C84 # CYRILLIC SMALL LETTER TALL TE
0x1C85 # CYRILLIC SMALL LETTER THREE-LEGGED TE
0x0422 # CYRILLIC CAPITAL LETTER TE
0x0442 # CYRILLIC SMALL LETTER TALL TE
```
I can combine `SpecialCasing.txt` with `UnicodeData.txt` or `CaseFolding.txt` but it really doesn't buy us anything. `SpecialCasing.txt` is just missing information about all of the "simple" folds. I could take `CaseFolding.txt` and then filter it by what `Character.toUpperCase` and `Character.toLowerCase` can map to but I was thinking about this and it strikes me that I would be doing that simply to reduce the map size from 24kb, which seems pretty small to me to begin with. And then at runtimes we'd be calling `Character.toUpperCase` and `Character.toLowerCase` to undo that and get those mappings back. I can see an argument that there's an upside to not maintaining the mappings that "Java already supports" but then it seems painful to know what version of the Unicode spec we actually support. If we build the mapping from `CaseFolding.txt` only it's simpler logic at regex build time and easier to document and say we support version 16.0 of the Unicode case folding spec. I c
an and already plan to update the `casefolding.txt` file with comments about which characters are folding. So I'm not sure it's a huge difference between hard coding it in a Java file vs loading via a text file. If there was a single source like we didn't need to generate the mapping and could read from say `CaseFolding.txt` or `SpecialCasing.txt` it would make more sense to hard code it to me, but I'm a little concerned it would be hard to see edits to the spec reflected appropriately without maintaining some of kind utility class like what I currently have as `CaseFoldingUtil`. But if you want to push back hard on some of this I don't mind building a utility as a one off that outputs a bunch of case statements or Map Entry lines instead of a checked in utility that outputs a bunch of text lines. The two seem pretty equivalent to me. It seems nice to maintain the mapping utility in Lucene itself imo. I'd prefer to get something in for Unicode support rather than nothing thoug
h. And particularly for my use case here (I'm trying to clean up and unify regex in Elasticsearch) it would be nice to expose a `CaseFolding` class that I can reference in ES instead of say using icu4j in ES and Lucene having a potentially slightly different opinion.
Lastly if the map size of 24kb is concerning I can also conditionally load that only when the flag is set. I'm kind of thinking this is a good idea anyway but didn't want to add unnecessary complexity.
To be confident about some of this I went and have code to do much of it locally. I can show you the differences between using `SpecialCasing.txt` vs all of `CaseFolding.txt` vs what I did original which is evaluate all of the matches that Pattern pulls back. Let me know if that would be helpful to review one of those and or see some snippets in comments here.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1940264096
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
+ entry(924, new int[] {181, 956}),
+ entry(956, new int[] {181, 924}),
+ entry(304, new int[] {105, 73, 305}),
+ entry(105, new int[] {304, 73, 305}),
+ entry(73, new int[] {304, 105, 305}),
+ entry(305, new int[] {304, 105, 73}),
+ entry(383, new int[] {83, 115}),
+ entry(83, new int[] {383, 115}),
+ entry(115, new int[] {383, 83}),
+ entry(837, new int[] {921, 953, 8126}),
+ entry(921, new int[] {837, 953, 8126}),
+ entry(953, new int[] {837, 921, 8126}),
+ entry(8126, new int[] {837, 921, 953}),
+ entry(962, new int[] {931, 963}),
+ entry(931, new int[] {962, 963}),
+ entry(963, new int[] {962, 931}),
+ entry(976, new int[] {914, 946}),
+ entry(914, new int[] {976, 946}),
+ entry(946, new int[] {976, 914}),
+ entry(977, new int[] {920, 952, 1012}),
+ entry(920, new int[] {977, 952, 1012}),
+ entry(952, new int[] {977, 920, 1012}),
+ entry(1012, new int[] {977, 920, 952}),
+ entry(981, new int[] {934, 966}),
+ entry(934, new int[] {981, 966}),
+ entry(966, new int[] {981, 934}),
+ entry(982, new int[] {928, 960}),
+ entry(928, new int[] {982, 960}),
+ entry(960, new int[] {982, 928}),
+ entry(1008, new int[] {922, 954}),
+ entry(922, new int[] {1008, 954}),
+ entry(954, new int[] {1008, 922}),
+ entry(1009, new int[] {929, 961}),
+ entry(929, new int[] {1009, 961}),
+ entry(961, new int[] {1009, 929}),
+ entry(1013, new int[] {917, 949}),
+ entry(917, new int[] {1013, 949}),
+ entry(949, new int[] {1013, 917}),
+ entry(7296, new int[] {1042, 1074}),
+ entry(1042, new int[] {7296, 1074}),
+ entry(1074, new int[] {7296, 1042}),
+ entry(7297, new int[] {1044, 1076}),
+ entry(1044, new int[] {7297, 1076}),
+ entry(1076, new int[] {7297, 1044}),
+ entry(7298, new int[] {1054, 1086}),
+ entry(1054, new int[] {7298, 1086}),
+ entry(1086, new int[] {7298, 1054}),
+ entry(7299, new int[] {1057, 1089}),
+ entry(1057, new int[] {7299, 1089}),
+ entry(1089, new int[] {7299, 1057}),
+ entry(7300, new int[] {1058, 1090, 7301}),
+ entry(1058, new int[] {7300, 1090, 7301}),
+ entry(1090, new int[] {7300, 1058, 7301}),
+ entry(7301, new int[] {7300, 1058, 1090}),
+ entry(7302, new int[] {1066, 1098}),
+ entry(1066, new int[] {7302, 1098}),
+ entry(1098, new int[] {7302, 1066}),
+ entry(7303, new int[] {1122, 1123}),
+ entry(1122, new int[] {7303, 1123}),
+ entry(1123, new int[] {7303, 1122}),
+ entry(7304, new int[] {42570, 42571}),
+ entry(42570, new int[] {7304, 42571}),
+ entry(42571, new int[] {7304, 42570}),
+ entry(7835, new int[] {7776, 7777}),
+ entry(7776, new int[] {7835, 7777}),
+ entry(7777, new int[] {7835, 7776}),
+ entry(8486, new int[] {969, 937}),
+ entry(969, new int[] {8486, 937}),
+ entry(937, new int[] {8486, 969}),
+ entry(8490, new int[] {107, 75}),
+ entry(107, new int[] {8490, 75}),
+ entry(75, new int[] {8490, 107}),
+ entry(8491, new int[] {229, 197}),
+ entry(229, new int[] {8491, 197}),
+ entry(197, new int[] {8491, 229}),
Review Comment:
Yeah I'm less concerned about what Pattern is doing. I am happy you have a solution though. I don't really want to look at Pattern and given that java STILL doesn't expose case-folding, I'm guessing I'd be sad if I looked.
From unicode perspective I think this is where we want to move towards?
https://unicode.org/reports/tr18/#Simple_Loose_Matches
So if the answer to this special set is that, they are the ones in https://www.unicode.org/Public/UCD/latest/ucd/SpecialCasing.txt, it will solve my problem. Then we have a way to keep everything up to date and understand what all the exceptions are for.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1940026321
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
+ entry(924, new int[] {181, 956}),
+ entry(956, new int[] {181, 924}),
+ entry(304, new int[] {105, 73, 305}),
+ entry(105, new int[] {304, 73, 305}),
+ entry(73, new int[] {304, 105, 305}),
+ entry(305, new int[] {304, 105, 73}),
+ entry(383, new int[] {83, 115}),
+ entry(83, new int[] {383, 115}),
+ entry(115, new int[] {383, 83}),
+ entry(837, new int[] {921, 953, 8126}),
+ entry(921, new int[] {837, 953, 8126}),
+ entry(953, new int[] {837, 921, 8126}),
+ entry(8126, new int[] {837, 921, 953}),
+ entry(962, new int[] {931, 963}),
+ entry(931, new int[] {962, 963}),
+ entry(963, new int[] {962, 931}),
+ entry(976, new int[] {914, 946}),
+ entry(914, new int[] {976, 946}),
+ entry(946, new int[] {976, 914}),
+ entry(977, new int[] {920, 952, 1012}),
+ entry(920, new int[] {977, 952, 1012}),
+ entry(952, new int[] {977, 920, 1012}),
+ entry(1012, new int[] {977, 920, 952}),
+ entry(981, new int[] {934, 966}),
+ entry(934, new int[] {981, 966}),
+ entry(966, new int[] {981, 934}),
+ entry(982, new int[] {928, 960}),
+ entry(928, new int[] {982, 960}),
+ entry(960, new int[] {982, 928}),
+ entry(1008, new int[] {922, 954}),
+ entry(922, new int[] {1008, 954}),
+ entry(954, new int[] {1008, 922}),
+ entry(1009, new int[] {929, 961}),
+ entry(929, new int[] {1009, 961}),
+ entry(961, new int[] {1009, 929}),
+ entry(1013, new int[] {917, 949}),
+ entry(917, new int[] {1013, 949}),
+ entry(949, new int[] {1013, 917}),
+ entry(7296, new int[] {1042, 1074}),
+ entry(1042, new int[] {7296, 1074}),
+ entry(1074, new int[] {7296, 1042}),
+ entry(7297, new int[] {1044, 1076}),
+ entry(1044, new int[] {7297, 1076}),
+ entry(1076, new int[] {7297, 1044}),
+ entry(7298, new int[] {1054, 1086}),
+ entry(1054, new int[] {7298, 1086}),
+ entry(1086, new int[] {7298, 1054}),
+ entry(7299, new int[] {1057, 1089}),
+ entry(1057, new int[] {7299, 1089}),
+ entry(1089, new int[] {7299, 1057}),
+ entry(7300, new int[] {1058, 1090, 7301}),
+ entry(1058, new int[] {7300, 1090, 7301}),
+ entry(1090, new int[] {7300, 1058, 7301}),
+ entry(7301, new int[] {7300, 1058, 1090}),
+ entry(7302, new int[] {1066, 1098}),
+ entry(1066, new int[] {7302, 1098}),
+ entry(1098, new int[] {7302, 1066}),
+ entry(7303, new int[] {1122, 1123}),
+ entry(1122, new int[] {7303, 1123}),
+ entry(1123, new int[] {7303, 1122}),
+ entry(7304, new int[] {42570, 42571}),
+ entry(42570, new int[] {7304, 42571}),
+ entry(42571, new int[] {7304, 42570}),
+ entry(7835, new int[] {7776, 7777}),
+ entry(7776, new int[] {7835, 7777}),
+ entry(7777, new int[] {7835, 7776}),
+ entry(8486, new int[] {969, 937}),
+ entry(969, new int[] {8486, 937}),
+ entry(937, new int[] {8486, 969}),
+ entry(8490, new int[] {107, 75}),
+ entry(107, new int[] {8490, 75}),
+ entry(75, new int[] {8490, 107}),
+ entry(8491, new int[] {229, 197}),
+ entry(229, new int[] {8491, 197}),
+ entry(197, new int[] {8491, 229}),
Review Comment:
good suggestion; let me poke around a bit more and see if I can somehow do that. My intention was to follow Pattern not the Unicode spec (and let Pattern follow some version of the spec) but it would be nice if there were obvious sets that all of these fell into rather than this hardcoded mapping.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1941960613
##########
lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java:
##########
@@ -436,6 +478,160 @@ public enum Kind {
*/
@Deprecated public static final int DEPRECATED_COMPLEMENT = 0x10000;
+ /**
+ * See {@link #UNICODE_CASE_INSENSITIVE} for more details on the set of known unstable alternative
+ * casings
+ */
+ static final Map<Integer, int[]> unstableUnicodeCharacters =
+ Map.ofEntries(
+ // these are the set of characters whose casing matches across multiple characters
+ entry(181, new int[] {924, 956}),
+ entry(924, new int[] {181, 956}),
+ entry(956, new int[] {181, 924}),
+ entry(304, new int[] {105, 73, 305}),
+ entry(105, new int[] {304, 73, 305}),
+ entry(73, new int[] {304, 105, 305}),
+ entry(305, new int[] {304, 105, 73}),
+ entry(383, new int[] {83, 115}),
+ entry(83, new int[] {383, 115}),
+ entry(115, new int[] {383, 83}),
+ entry(837, new int[] {921, 953, 8126}),
+ entry(921, new int[] {837, 953, 8126}),
+ entry(953, new int[] {837, 921, 8126}),
+ entry(8126, new int[] {837, 921, 953}),
+ entry(962, new int[] {931, 963}),
+ entry(931, new int[] {962, 963}),
+ entry(963, new int[] {962, 931}),
+ entry(976, new int[] {914, 946}),
+ entry(914, new int[] {976, 946}),
+ entry(946, new int[] {976, 914}),
+ entry(977, new int[] {920, 952, 1012}),
+ entry(920, new int[] {977, 952, 1012}),
+ entry(952, new int[] {977, 920, 1012}),
+ entry(1012, new int[] {977, 920, 952}),
+ entry(981, new int[] {934, 966}),
+ entry(934, new int[] {981, 966}),
+ entry(966, new int[] {981, 934}),
+ entry(982, new int[] {928, 960}),
+ entry(928, new int[] {982, 960}),
+ entry(960, new int[] {982, 928}),
+ entry(1008, new int[] {922, 954}),
+ entry(922, new int[] {1008, 954}),
+ entry(954, new int[] {1008, 922}),
+ entry(1009, new int[] {929, 961}),
+ entry(929, new int[] {1009, 961}),
+ entry(961, new int[] {1009, 929}),
+ entry(1013, new int[] {917, 949}),
+ entry(917, new int[] {1013, 949}),
+ entry(949, new int[] {1013, 917}),
+ entry(7296, new int[] {1042, 1074}),
+ entry(1042, new int[] {7296, 1074}),
+ entry(1074, new int[] {7296, 1042}),
+ entry(7297, new int[] {1044, 1076}),
+ entry(1044, new int[] {7297, 1076}),
+ entry(1076, new int[] {7297, 1044}),
+ entry(7298, new int[] {1054, 1086}),
+ entry(1054, new int[] {7298, 1086}),
+ entry(1086, new int[] {7298, 1054}),
+ entry(7299, new int[] {1057, 1089}),
+ entry(1057, new int[] {7299, 1089}),
+ entry(1089, new int[] {7299, 1057}),
+ entry(7300, new int[] {1058, 1090, 7301}),
+ entry(1058, new int[] {7300, 1090, 7301}),
+ entry(1090, new int[] {7300, 1058, 7301}),
+ entry(7301, new int[] {7300, 1058, 1090}),
+ entry(7302, new int[] {1066, 1098}),
+ entry(1066, new int[] {7302, 1098}),
+ entry(1098, new int[] {7302, 1066}),
+ entry(7303, new int[] {1122, 1123}),
+ entry(1122, new int[] {7303, 1123}),
+ entry(1123, new int[] {7303, 1122}),
+ entry(7304, new int[] {42570, 42571}),
+ entry(42570, new int[] {7304, 42571}),
+ entry(42571, new int[] {7304, 42570}),
+ entry(7835, new int[] {7776, 7777}),
+ entry(7776, new int[] {7835, 7777}),
+ entry(7777, new int[] {7835, 7776}),
+ entry(8486, new int[] {969, 937}),
+ entry(969, new int[] {8486, 937}),
+ entry(937, new int[] {8486, 969}),
+ entry(8490, new int[] {107, 75}),
+ entry(107, new int[] {8490, 75}),
+ entry(75, new int[] {8490, 107}),
+ entry(8491, new int[] {229, 197}),
+ entry(229, new int[] {8491, 197}),
+ entry(197, new int[] {8491, 229}),
Review Comment:
I'm sold. I was comparing the CaseFolding spec with the Pattern and String classes today and found a (albeit small) number of characters that neither correctly supports from the unicode spec which seemed to me that should pretty clearly be supported such as:
https://util.unicode.org/UnicodeJsps/character.jsp?a=1C8A&B1=Show
https://util.unicode.org/UnicodeJsps/character.jsp?a=0x1C89&B1=Show
I have some changes related to your other comments but I'm going to iterate a bit further and I'll likely need some additional feedback on an approach that incorporates that spec / we may need to talk about how to maintain the case folding as the unicode spec iterates. I was hoping to rely heavily on the Pattern class being updated so we didn't have to do as much maintenance on our end to make sure case insensitivity continued to work appropriately.
Also just to be clear the intention of the PR is just handling character casing not normalization (just forgot to address that yesterday).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14192:
URL: https://github.com/apache/lucene/pull/14192#discussion_r1949856748
##########
lucene/core/src/java/org/apache/lucene/util/automaton/CaseFoldingUtil.java:
##########
@@ -0,0 +1,338 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.util.automaton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.HexFormat;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Generates the case folding set of alternate character mappings based on this spec:
+ * https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt
Review Comment:
not worried about what Pattern does, or offering general purpose unicode apis to elasticsearch.
But mainly, if we want to bring all unicode casefolding data into lucene-core and offer some general API around it, I want it to work, where it can be performant and usable by analyzers and so on. boxing and unboxing and hashmaps on every character aren't great for that. the map is also not 24KB: this file has 3,000 lines and is using objects (Integer, int[]) per mapping, so it is much more.
for regexp, it is enough to just have it like the first patch, only documented where the magic numbers are coming from. And parsing regexp doesn't need to be fast, so it makes it easier to start. If we want to offer some general public api then it needs to look different.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "john-wagster (via GitHub)" <gi...@apache.org>.
john-wagster commented on PR #14192:
URL: https://github.com/apache/lucene/pull/14192#issuecomment-2651490994
@rmuir I made another pass based on your feedback and I'm good with and agree to keep this simple for a first pass. To that end I've done the following:
* CaseFolding is no longer externally exposed (can revisit this later)
* CaseFolding has one method to get the set of alternates renamed from `fold` which was inaccurate previously. The idea being to introduce a `fold` method in the future if it's valuable.
* CaseFolding only includes the set of edge cases that are not handled by `Character.toLowerCase` and `Character.toUpperCase` and is a switch/case statement now with hex codepoints and comments on ever character
* this is similar to ASCIIFoldingFilter
* it was built using https://www.unicode.org/Public/16.0.0/ucd/CaseFolding.txt and filtered by the set of things not already covered by `Character.toLowerCase` and `Character.toUpperCase`
* The utility class CaseFoldingUtil for generating the switch / case statement code in the CaseFolding class has been removed.
* ASCII_CASE_INSENSITIVE flag has been deprecated and has the same behavior as the CASE_INSENSITIVE flag
* Tried to minimize changes otherwise with less concern for performance during compilation of the regex
Thoughts at this point and whether I've adequately incorporated your feedback would be much appreciated.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Unicode Support for Case Insensitive Matching in RegExp [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir merged PR #14192:
URL: https://github.com/apache/lucene/pull/14192
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org