sam-1112 commented on code in PR #5415: URL: https://github.com/apache/datafusion-comet/pull/5415#discussion_r3840499955
########## spark/src/main/scala/org/apache/comet/expressions/CometRegex.scala: ########## @@ -0,0 +1,279 @@ +/* + * 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.comet.expressions + +import org.apache.comet.serde.{Compatible, Incompatible, SupportLevel} + +/** + * Regex flavor for [[CometRegex]]. The first version only implements [[RegexFlavor.RLike]]; later + * flavors (for example `regexp_replace` / `split`) can add extra reject rules such as empty-match + * divergence without changing the scanner's whitelist core. + */ +sealed trait RegexFlavor + +object RegexFlavor { + case object RLike extends RegexFlavor +} + +/** + * Plan-time whitelist analyzer for literal Java regex patterns. A pattern is [[Compatible]] only + * when every construct is one the analyzer positively recognizes as equivalent on Spark's + * `java.util.regex` engine and Comet's Rust `regex` crate. Anything unrecognized is + * [[Incompatible]]: the safe direction, so a missed construct never silently takes the native + * path. + * + * This is a recursive-descent scan, not a search for forbidden substrings. `[(?=]` is a character + * class of literals, not a lookahead; `\\d` is a literal backslash plus `d`, not a digit class. + */ +object CometRegex { + + def supportLevel(pattern: String, flavor: RegexFlavor = RegexFlavor.RLike): SupportLevel = { + flavor match { + case RegexFlavor.RLike => + val scanner = new Scanner(pattern) + if (scanner.parseExpr() && !scanner.remaining) { + Compatible() Review Comment: Thanks, that was a real gap. `{0,}` was using the lower bound `0` as the expansion multiplier, so `multiplyWithinBudget` collapsed the inner cost to 1 and admitted `(([^;]{256}){0,}){256}`. That pattern is Java-valid but rejected by the locked Rust regex crate, so the native path would fail a query that Spark currently succeeds. The unbounded `{n,}` branch now uses `math.max(1, n)`, so `{0,}` keeps the inner compile cost. `{0}` and `{0,0}` stay exact-zero and are still Compatible. Coverage: - analyzer: `(([^;]{256}){0,}){256}` is Incompatible; `{0}` / `{0,0}` remain Compatible - routing: the unbounded pattern stays on the JVM codegen dispatcher and matches Spark - exact-zero counterparts stay native and match Spark Pushed in `c62b529a7`. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
