Skip to content

[BUG] forPattern() performs up to 120 redundant String.equals() comparisons on every date-field request #22567

Description

@rajat315315

Describe the bug

Is your change related to a problem? Please describe

Both Joda.forPattern(String input) and DateFormatters.forPattern(String input)
contain a chain of 60–70 else if branches. Each branch calls
FormatNames.XXX.matches(input), which does two String.equals() comparisons
(one for the camel-case name, one for the snake-case name).

A format name near the end of the chain — e.g. strict_year_month_day — must
walk through all previous branches first:

2 comparisons × 60+ branches = up to 120 string comparisons before a match
is found.

This code is on the hot path: it is invoked for every index and search
operation that involves a date-typed field.

Root cause: the enum lookup is done twice

Both methods already call FormatNames.forName(input) at the top to identify the
matching enum constant (used for the camel-case deprecation warning). After that,
the returned enum value is thrown away and the same linear scan is repeated
from scratch via the if-else chain.

// Both methods already do this …
FormatNames formatName = FormatNames.forName(input);    // ← scan #1

// … and then immediately do it again:
if (FormatNames.BASIC_DATE.matches(input)) {            // ← scan #2, branch 1
    ...
} else if (FormatNames.BASIC_DATE_TIME.matches(input)) {  // branch 2
    ...
}
// … 60+ more branches

Describe the solution you'd like

Replace the redundant if-else chain with a switch statement on the FormatNames
enum value that is already resolved by FormatNames.forName(input).

FormatNames formatName = FormatNames.forName(input);
// … camel-case deprecation warning (unchanged) …

if (formatName != null) {
    switch (formatName) {              // O(1) JVM tableswitch / lookupswitch
        case BASIC_DATE:               return BASIC_DATE;
        case BASIC_DATE_TIME:          return BASIC_DATE_TIME;
        // … all FormatNames values …
        case STRICT_YEAR_MONTH_DAY:    return STRICT_YEAR_MONTH_DAY;
        default: break;
    }
}
// raw-pattern fallback (unchanged)

A switch on an enum compiles to a JVM tableswitch or lookupswitch
instruction — O(1) dispatch regardless of the number of cases.


Benefits

1. 🚀 Performance — eliminates O(n) string comparisons on the hot path

Scenario Before After
Format near start of chain (e.g. basic_date) 2 comparisons O(1)
Format near end of chain (e.g. strict_year_month_day) up to 120 comparisons O(1)
Unknown / raw pattern (e.g. yyyy/MM/dd) 120+ comparisons + forName scan forName scan + O(1) default

In write-heavy or aggregation-heavy workloads where forPattern is called millions
of times per minute, eliminating these comparisons meaningfully reduces CPU cycles
and instruction-cache pressure.

2. ✅ Correctness — single authoritative lookup

Currently two independent code paths must stay in sync:

  • FormatNames.forName() (used for the deprecation check)
  • The if-else chain (used for the actual dispatch)

If a new FormatNames constant is added to one but not the other, behaviour
silently diverges. After this refactor there is one lookup and one dispatch
table — they cannot go out of sync.

3. 🛡️ Exhaustiveness checking — compiler detects missing cases

With a switch on an enum, IDEs and static analysis tools (-Xlint:switch,
SpotBugs) will warn when a FormatNames value has no case arm. In the old
if-else, a new enum value is silently skipped and falls through to the raw-pattern
path, producing wrong results with no diagnostic.

4. 📖 Readability and maintainability

  • The mapping from format name to formatter is a clean 1:1 list of case
    statements — immediately readable.
  • Adding a new format name requires one case line instead of a new
    } else if { block appended to a 200-line chain.

Affected files

File Method
server/src/main/java/org/opensearch/common/joda/Joda.java forPattern() — ~60 branch if-else
server/src/main/java/org/opensearch/common/time/DateFormatters.java forPattern() — ~70 branch if-else

Out of scope

The following similar-looking if-else chains are intentionally excluded:

Location Reason
SearchSourceBuilder.parseXContent Uses ParseField.match() which handles deprecated field-name aliases; a raw String switch would silently break those aliases
MockScriptEngine.compile Dispatches on context.instanceClazz (Class<?> objects), which are not a valid Java switch expression type

Implementation notes

  • No functional change. All existing behaviour — including the deprecation
    warning for WEEKYEAR / week_year and the two special early-return cases
    (DATE_OPTIONAL_TIME, STRICT_DATE_OPTIONAL_TIME) — is preserved.
  • The default case falls through to the existing raw-pattern fallback rather
    than throwing, so any future FormatNames value added without a matching case
    degrades gracefully.
  • Covered entirely by existing test suites. No new tests are needed.

Steps to verify

# Run relevant unit tests
./gradlew :server:test --tests "*.DateFormattersTests"
./gradlew :server:test --tests "*.JodaTests"
./gradlew :server:test --tests "*.FormatNamesTests"

# Full pre-commit check
./gradlew precommit

Related branch

refactor/date-format-if-else-to-switch

Related component

Search:Performance

To Reproduce

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior

Its a refactor.

Additional Details

Plugins
Please list all plugins currently enabled.

Screenshots
If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

  • OS: [e.g. iOS]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status
    🆕 New

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions