sunchao commented on code in PR #5415: URL: https://github.com/apache/datafusion-comet/pull/5415#discussion_r3839017590
########## 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: [P2] `{0,}` still bypasses the compile-size budget Rechecked at `0f98989095dee698db71b366998e112bdd1e15a6`. The aggregate checks address the earlier sibling cases, but `(([^;]{256}){0,}){256}` is still classified as `Compatible`. The `{0,}` branch returns the lower bound `0`, so `multiplyWithinBudget` reduces the inner cost from 256 to 1. Unlike `{0}` or `{0,0}`, this is unbounded repetition and the inner expression still has to be compiled. I reproduced this with the unmodified current scanner on Scala 2.12 and 2.13. Spark 3.5.9 and 4.0.4 both return `true` for a non-foldable subject containing `a`, in interpreted evaluation and forced generated projections. The exact locked Rust regex 1.13.1 instead rejects the pattern with `Compiled regex exceeds size limit of 10485760 bytes.` The equivalent `(([^;]{256})*){256}` is correctly rejected by the scanner. Because the admitted form now selects native execution without opt-in and the native builder propagates the compilation error, this still changes a successful default-config query into a failure. Could we retain the inner compilation cost for unbounded `{0,}` separately from exact-zero repetitions and add this case to the dispatcher-routing regressions? These checks used real Spark expressions and the locked regex engine plus source-traced native routing, not a full Comet/JNI query run. -- 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]
