Collapse repeated wildcard asterisks

[vismaytiwari]

Description

Fixes #16134.

WildcardQuery.toAutomaton created a separate Automata.makeAnyString() automaton for every unescaped *. Consecutive * wildcards match the same language as a single *, but the repeated form made Operations.concatenate build a much larger automaton.

This collapses consecutive unescaped * tokens while preserving escaped \* as a literal. It also adds a regression test that checks repeated asterisks produce the same automaton shape as a single asterisk.

Before the change, WildcardQuery.toAutomaton(new Term("field", "*".repeat(10000))) built 50,005,000 transitions locally. After the change, the same input builds 1 transition.

Validation:

• ./gradlew tidy
• ./gradlew :lucene:core:test --tests TestWildcardQuery -Dtests.seed=5EEC297244EB01E6 -Dtests.asserts=true -Dtests.file.encoding=UTF-8
• ./gradlew :lucene:core:check -Dtests.seed=5EEC297244EB01E6 -Dtests.asserts=true -Dtests.file.encoding=UTF-8

[amalatlas]

Could you add a test for "*?" x 5000 ? I am just curious what would be the behavior.

[uschindler]

I am not oppsed to that. We should maybe have another variant of the same test where we have also an escaped wildcard and some other characters. Basically can't we compare the automatons produced for equality instead of only counting states?

Otherwise OK!

[uschindler]

Just a nitpick: You may also convert the switch statement with the old-style break and possible fallthrough to the modern form (introduced around Java 17). Whenever I toiuch code using old switch statements, I convert them!

[vismaytiwari]

Pushed 9dc9ac5.

@uschindler — converted the switch in toAutomaton to the arrow form, and dropped the @SuppressWarnings("fallthrough") while at it (the trailing-\ case is an explicit else now).

@amalatlas — added the *? × 5000 case you asked about, and it doesn't blow up: the *s aren't consecutive so nothing collapses, but it still builds a linearly-sized automaton (the repeated-* case this PR fixes was ~50M transitions at that length). While there I also switched the collapse test to compare automaton language equality via AutomatonTestUtil.sameLanguage instead of only counting states/transitions, and added a variant that mixes an escaped \* with literals and a ?.

[uschindler]

Great, thanks!

[uschindler]

Would you add a changes entry. We have not yet decided if that can be backported to 10.x.

I know for 11.x we have the optimizations regarding determinization, so with 10.x we may need some additional changes the to the test. Can you figure that out?

If we shoudl backport it, please add it to the 10.x changes list.

[rmuir]

This change requires the entire wildcard parsing to be focused on handling an abusive-case that isn't a real use-case at all

[vismaytiwari]

Added the CHANGES entry under 11.0.0 → Optimizations (276e3c7).

On the backport: it's doable. The collapse itself ports cleanly, and it's arguably more valuable on 10.x since toAutomaton there determinizes eagerly. The one wrinkle you flagged is real — 10.x's toAutomaton(Term, int determinizeWorkLimit) takes a work limit and determinizes inside (11.x dropped that), so the test would call toAutomaton(term, DEFAULT_DETERMINIZE_WORK_LIMIT) and skip the extra determinize. Small change. Happy to open the 10.x backport with that tweak + a 10.6.0 CHANGES entry if you'd like it there.

Otherwise I think this is good to go from my side.

[vismaytiwari]

@rmuir fair point that *-spam isn't a real use case, and I don't want to add complexity for garbage input. But I think this one's worth it because it costs real queries nothing: the change is a single boolean flag in the parse loop, so for any pattern without consecutive unescaped * it builds the exact same automaton as before — identical work. The only path that changes is the degenerate one, where it goes from ~50M transitions (an OOM footgun, and a TooComplexToDeterminize crash on 10.x) to 1. It's really just collapsing ** → *, which are equivalent anyway. So: zero cost for everyone, removes a crash for the unlucky. Happy to make it leaner if you see a tidier way — but would that change your mind?

[uschindler]

I think there is a use case. Many of our query parsers by default support wildcards, also the default Solr one (e.g., dismax). If it uses way too much memory for degenerate cases, it should be handled in the most ideal case.

The code introduced here is just the default pattern used for removing duplicates in such parsers. Yes, it is an additional state variable. Maybe it can be similified a bit so you don't have the boolean setter everywhere. If you have an idea, apply it to the patch. Otherwise for me it is fine!

[uschindler]

P.S.: Thanks for changing the switch statement to a more modern arrow-switch without breaks! This alone is worth the change!

[danmuzi]

Thanks for your contribution :)
Just one small nit.

[uschindler]

I am thinking about a possible better alternative inside the automation API. This would also improve regex queries with multiple .*.

Can't we modify the addition of anyString inside the automation, so it only adds it if there's not one before?

I am just thinking. If that's possible, we could catch more cases.

[vismaytiwari]

@uschindler I like it — pushed into the automaton layer that'd also catch regex .*.*, not just wildcard **.

One wrinkle: dropping a segment is only safe when it's the total automaton (Σ*·Σ* = Σ*), and detecting that mid-concatenate isn't free — makeAnyString() is a fresh instance each call and isTotal wants a minimized automaton, so it'd be a scan on the hot path. I'd keep it intent-driven instead: a small helper the builders call to skip an anyString when the previous segment was already one, reused in the regex builder. Same win, no per-query cost, and it drops the boolean you flagged.

Want me to try that here, or keep it separate so this one can land?

[uschindler]

I think you can do it here. You can revert the branch if it won't work.

[vismaytiwari]

@uschindler done — dropped the boolean in the wildcard loop; the * case now just checks Operations.isTotal on the previous segment, so it only appends anyString when the prior one wasn't already total. Pushed.

On extending it to regex: both wildcard and regex actually flatten into the same Operations.concatenate(List), so there is a shared spot — but I'd keep the dedup out of that generic method (it'd run an isTotal scan on every concatenate caller), and it wouldn't catch regex anyway since .* builds as repeat(anyChar), not the canonical total form isTotal recognizes without determinizing first. Cleanest for regex is a small collapse of adjacent .*/anystring at the RegExp AST level — happy to do that as a follow-up so this PR stays focused. Sound ok?

[uschindler]

Cool. I will check later, but the tests confirm it works.