-
Notifications
You must be signed in to change notification settings - Fork 382
test: add Java-generated RLIKE parity fixtures for regex upgrades #6173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5ae1e25
3cedd28
aea0d84
726a77f
d9ca29f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,225 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** Generates the Java oracle for Comet's admitted RLIKE subset. Run with JDK 17. */ | ||
| public class GenerateRegexFixtures { | ||
| private static final List<String> CASES = new ArrayList<>(); | ||
|
|
||
| private static String quote(String value) { | ||
| StringBuilder result = new StringBuilder("\""); | ||
| for (int i = 0; i < value.length(); i++) { | ||
| char c = value.charAt(i); | ||
| if (c == '"' || c == '\\') { | ||
| result.append('\\').append(c); | ||
| } else if (c < 0x20 || c > 0x7e) { | ||
| result.append(String.format("\\u%04x", (int) c)); | ||
| } else { | ||
| result.append(c); | ||
| } | ||
| } | ||
| return result.append('"').toString(); | ||
| } | ||
|
|
||
| private static void add(String category, String pattern, String... subjects) { | ||
| Pattern compiled = Pattern.compile(pattern); | ||
| for (String subject : subjects) { | ||
| boolean expected = compiled.matcher(subject).find(); | ||
| CASES.add( | ||
| " {\"category\": " | ||
| + quote(category) | ||
| + ", \"pattern\": " | ||
| + quote(pattern) | ||
| + ", \"subject\": " | ||
| + quote(subject) | ||
| + ", \"expected\": " | ||
| + expected | ||
| + "}"); | ||
| } | ||
| } | ||
|
|
||
| private static void addRange(String start, char lo, String end, char hi) { | ||
| // Probe both endpoints, an interior point, and the immediately adjacent nonmembers. | ||
| for (String prefix : new String[] {"[", "[^"}) { | ||
| add( | ||
| "escaped-range-boundaries", | ||
| prefix + start + "-" + end + "]", | ||
| "", | ||
| String.valueOf((char) (lo - 1)), | ||
| String.valueOf(lo), | ||
| String.valueOf((char) ((lo + hi) / 2)), | ||
| String.valueOf(hi), | ||
| String.valueOf((char) (hi + 1)), | ||
| "\n", | ||
| "\ud83d\ude00"); | ||
| } | ||
| } | ||
|
|
||
| private static void addRangeMatrix() { | ||
| // Every escape admitted by parseEscape(inClass = true), including the class-only hyphen. | ||
| char[] escapes = ".*+?()[]{}|^$\\-".toCharArray(); | ||
| for (char start : escapes) { | ||
| addRange("\\" + start, start, "~", '~'); | ||
| addRange("!", '!', "\\" + start, start); | ||
| for (char end : escapes) { | ||
| if (start <= end) { | ||
| addRange("\\" + start, start, "\\" + end, end); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void addQuantifierMatrix() { | ||
| String[] quantifiers = {"*", "+", "?", "{0}", "{1}", "{2}", "{0,}", "{1,}", "{1,2}"}; | ||
| for (String group : new String[] {"(", "(?:"}) { | ||
| // All ordered pairs, including nullable bodies. Required surrounding literals prevent | ||
| // find() from skipping the repeated input and matching only the trailing delimiter. | ||
| for (String atom : new String[] {"a", "a|"}) { | ||
| for (String inner : quantifiers) { | ||
| for (String outer : quantifiers) { | ||
| String pattern = "c" + group + group + atom + ")" + inner + ")" + outer + "b"; | ||
| add( | ||
| "nested-quantifier-pairs", | ||
| pattern, | ||
| "", "c", "ca", "cb", "cab", "caab", "caaaab", "caac", "xcabx"); | ||
| } | ||
| } | ||
| } | ||
| // Near-limit depths exercise real compilation, not only scanner admission. Keep subjects | ||
| // short for ambiguous unbounded repetitions to bound Java backtracking work. | ||
| for (String quantifier : quantifiers) { | ||
| for (int depth : new int[] {1, 2, 7, 8}) { | ||
| String pattern = group.repeat(depth) + "a" + (")" + quantifier).repeat(depth); | ||
| add("quantifier-depth-matrix", pattern, "", "a", "aa", "b"); | ||
| if (quantifier.equals("{2}")) { | ||
| int minimum = 1 << depth; | ||
| add("nested-counted-boundaries", pattern, "a".repeat(minimum - 1), "a".repeat(minimum)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public static void main(String[] args) throws Exception { | ||
| if (args.length != 1 || Runtime.version().feature() != 17) { | ||
| throw new IllegalArgumentException( | ||
| "Run with JDK 17: java dev/GenerateRegexFixtures.java OUTPUT"); | ||
| } | ||
| String[] subjects = { | ||
| "", "abc", "abc123", "ABC", "foo", "bar", "foobar", "xxbarxx", "a+b", "\\d", "a", "b", "aa", | ||
| "aaaa", "ab", "abab", "ac", "cd", "abcd", "xxabbxx", "def", "123", "(?=", ".", "-", "z", | ||
| "a b", "@", "[", "]", "A", "_", "~", "a-z", | ||
| // Escaped so the output does not depend on the JDK 17 default source encoding. | ||
| "\u03b1\u03b2\u03b3", "\u0661\u0662\u0663", "\u4f60\u597d", "\ud83d\ude00", "e\u0301", | ||
| "a\ud83d\ude00b", "\n", | ||
| "\r", "\r\n", "\t", "\u000b", "\f", "\u0000", "\u007f", "\u00a0", "\ufeff", "\u0085", | ||
| "\u2028", "\u2029", "\nabc", "abc\n", "\nabc\n" | ||
| }; | ||
| String[][] groups = { | ||
| {"literal", "abc", "a b"}, | ||
| { | ||
| "class", | ||
| "[0-9_]", | ||
| "[^0-9]", | ||
| "[a-zA-Z_][a-zA-Z0-9_]*", | ||
| "[^a]", | ||
| "[^;]+", | ||
| "[a-]", | ||
| "[-a]", | ||
| "[a\\-z]", | ||
| "[@-\\[]", | ||
| "[\\.-9]", | ||
| "[\\--/]", | ||
| "[\\\\-a]", | ||
| "[a~b]", | ||
| "[.]", | ||
| "[\\]a]" | ||
| }, | ||
| {"lexer", "[(?=]", "\\(\\?=", "\\\\d", "a\\+b"}, | ||
| {"group", "(foo)", "(?:foo|bar)", "abc|def", "(?:(?:foo)|bar)"}, | ||
| {"quantifier", "a*", "a+", "a?", "a{2}", "a{2,}", "a{2,4}"}, | ||
| { | ||
| "composition", | ||
| "abc[0-9]+", | ||
| "(foo|bar){1,3}", | ||
| "(ab)+", | ||
| "(a{2}){3}", | ||
| "(?:ab|cd){2,3}", | ||
| "a(?:b|c)+d?", | ||
| "([a-c]+|[0-9]{2})_?", | ||
| "(?:a?b)*" | ||
| }, | ||
| { | ||
| "empty", | ||
| "", | ||
| "()", | ||
| "(?:)", | ||
| "a|", | ||
| "|a", | ||
| "a||b", | ||
| "a{0}", | ||
| "a{0,}", | ||
| "a{0,2}", | ||
| "(?:a|){2}", | ||
| "(?:){2}" | ||
| } | ||
| }; | ||
| for (String[] group : groups) { | ||
| for (int i = 1; i < group.length; i++) { | ||
| add(group[0], group[i], subjects); | ||
| } | ||
| } | ||
| for (char c : ".*+?()[]{}|^$\\".toCharArray()) { | ||
| add("escape", "\\" + c, "", String.valueOf(c), "x" + c + "y", "abc"); | ||
| add("class-escape", "[\\" + c + "]", "", String.valueOf(c), "abc"); | ||
| } | ||
| // Finite compositions exercise grammar interactions without random or unbounded generation. | ||
| for (String atom : new String[] {"a", "[a-c]", "[^;]", "(?:ab|c)", "(a)"}) { | ||
| for (String quantifier : new String[] {"", "*", "+", "?", "{0}", "{2}", "{1,}", "{1,3}"}) { | ||
| add("generated-composition", "(?:" + atom + quantifier + ")b|cd", subjects); | ||
| } | ||
| } | ||
| addRangeMatrix(); | ||
| addQuantifierMatrix(); | ||
| add("group-depth-32", "(".repeat(32) + "a" + ")".repeat(32), "", "a", "b"); | ||
| add("counted-bound-256", "a{256}", "", "a".repeat(255), "a".repeat(256)); | ||
| add("quantifier-depth-8", "(?:".repeat(8) + "a" + "){1}".repeat(8), "", "a", "b"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please include subjects such as |
||
| // Unlike nested {1}, stacked stars exercise nullable compilation state. | ||
| add("star-depth-8", "(".repeat(8) + "a" + ")*".repeat(8), "", "a", "aa", "b"); | ||
| add("expansion-4095", "(?:" + "a".repeat(16) + "){255}" + "a".repeat(15), "", "b"); | ||
| add("expansion-4096", "(?:" + "a".repeat(16) + "){256}", "", "b", "a".repeat(4096)); | ||
| add("capture-expansion-4096", "(" + "a".repeat(14) + "){256}", "", "b"); | ||
| add("class-expansion-4096", "[abcdefghijklmnop]{256}", "", "z", "a".repeat(256)); | ||
| String json = | ||
| "{\n \"jdk_vendor\": " | ||
| + quote(System.getProperty("java.vendor")) | ||
| + ",\n \"jdk_runtime_version\": " | ||
| + quote(System.getProperty("java.runtime.version")) | ||
| + ",\n \"cases\": [\n" | ||
| + String.join(",\n", CASES) | ||
| + "\n ]\n}\n"; | ||
| Files.writeString(Path.of(args[0]), json, StandardCharsets.UTF_8); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| <!-- | ||
| 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. | ||
| --> | ||
|
|
||
| # Java regex parity fixtures | ||
|
|
||
| The committed `spark/src/test/resources/regex/rlike-java-fixtures.json` records | ||
| Java `Pattern.compile(pattern).matcher(subject).find()` results for patterns | ||
| admitted by `CometRegex`. Rust's `test_rlike_java_fixtures` exercises the actual | ||
| `RLike` expression with scalar and UTF-8 array inputs against these answers. | ||
| It neither starts a JVM nor generates expected results during the test. The | ||
| crate's existing JNI build/link requirements still apply; see [Development](development.md). | ||
|
|
||
| ## Regenerating | ||
|
|
||
| From the repository root, use JDK 17. First check the committed fixtures without | ||
| overwriting them: | ||
|
|
||
| ```shell | ||
| java dev/GenerateRegexFixtures.java /tmp/rlike-java-fixtures.json | ||
| cmp spark/src/test/resources/regex/rlike-java-fixtures.json /tmp/rlike-java-fixtures.json | ||
| ``` | ||
|
|
||
| Only when intentionally updating the baseline after reviewing a generator or JDK | ||
| change, regenerate in place and inspect the diff: | ||
|
|
||
| ```shell | ||
| java dev/GenerateRegexFixtures.java spark/src/test/resources/regex/rlike-java-fixtures.json | ||
| git diff -- spark/src/test/resources/regex/rlike-java-fixtures.json | ||
| ``` | ||
|
|
||
| The initial oracle was generated with Eclipse Adoptium JDK `17.0.20.1+1`. | ||
| The JSON records the actual vendor and runtime version. Output order is stable, | ||
| uses explicit UTF-8 and JSON escapes, and contains no timestamp. Supplementary | ||
| characters are encoded as JSON surrogate pairs and decoded as Unicode strings. | ||
| Regeneration with the same JDK produces identical bytes. | ||
|
|
||
| The generator has no external dependencies. Change its fixed case lists or | ||
| bounded composition loops to extend coverage, then regenerate and review the | ||
| JSON diff. Expected values must always come from Java, never from Rust or manual | ||
| edits. All subjects are non-null; the existing kernel tests cover null propagation. | ||
|
|
||
| ## Coverage | ||
|
|
||
| Cases cover literals, every admitted metacharacter escape, positive and negated | ||
| classes, ranges, capturing and non-capturing groups, alternation, concatenation, | ||
| greedy and counted quantifiers, empty groups/branches/matches, and combinations | ||
| of these constructs. Subjects include ASCII, non-ASCII, combining characters, | ||
| supplementary code points, control characters, and newline variants. | ||
|
|
||
| Dedicated cases reach group depth 32, counted bound 256, quantifier depth 8 | ||
| (including stacked unbounded stars), and structural expansion 4095/4096, including | ||
| capture and class costs. These use targeted subjects instead of the full subject | ||
| cross product to bound Java backtracking work. Structural costs are admission heuristics, not a proof of | ||
| Rust compilation success. Finite fixtures cannot prove equivalence for all patterns. | ||
|
|
||
| The escaped-range matrix covers every admitted class escape as a start and an | ||
| end, all ordered pairs of escaped endpoints (including equal endpoints), and | ||
| both positive and negated classes. Each range probes its endpoints, an interior | ||
| point, immediately adjacent nonmembers, empty input, newline, and supplementary | ||
| Unicode input. | ||
|
|
||
| The nested-quantifier matrix covers all ordered pairs of `*`, `+`, `?`, `{0}`, | ||
| `{1}`, `{2}`, `{0,}`, `{1,}`, and `{1,2}`, with capturing and non-capturing groups | ||
| and both non-nullable and nullable bodies. Required surrounding literals make | ||
| consumption differences observable while still exercising unanchored search. Homogeneous nesting is also exercised at | ||
| depths 1, 2, 7, and 8, with match-length boundaries for nested `{2}`. Scala tests | ||
| check that depth 9 is rejected for each quantifier and group kind. This is a | ||
| bounded matrix of grammar shapes and limits, not exhaustive regex enumeration. | ||
|
|
||
| `CometRegexSuite` checks that every fixture is still admitted and that the | ||
| current Java engine agrees with the stored oracle. `CometRegexParitySuite` | ||
| continues to verify Spark/native routing end to end. | ||
|
|
||
| ## Validation and upgrades | ||
|
|
||
| ```shell | ||
| (cd native && cargo test -p datafusion-comet-spark-expr rlike) | ||
| make core | ||
| ./mvnw test -Dtest=none -Dsuites="org.apache.comet.expressions.CometRegexSuite,org.apache.comet.CometRegexParitySuite" | ||
| ``` | ||
|
|
||
| When upgrading `regex`, run the Rust test against the existing committed oracle. | ||
| A compilation error or mismatched answer must fail with the pattern, subject, | ||
| expected answer, and actual answer/error. Investigate the difference before | ||
| changing either the whitelist or fixtures; do not regenerate answers merely to | ||
| make an upgrade pass. Likewise, a JVM test failure after a JDK upgrade requires | ||
| review of the Java behavior change before replacing the baseline. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whitelist admits ranges whose start is an escaped character, such as
[\.-9],[\--/]and[\\-a], but the fixtures only cover an escaped end ([@-\[]). Could you add a few of those? I checked them againstregex1.13.1 and they agree with Java today. Since the point of the table is to catch a future crate change, it would be good to have every admitted class shape in it.