[KarstenSchnitter]

Description

Fixes per-index analytics-engine routing for non-concrete index expressions.

• Injects IndexNameExpressionResolver into the SQL and PPL analytics routers.
• Resolves aliases, wildcards, comma-separated PPL sources, and data streams against one cluster-state snapshot.
• Routes only non-empty resolutions containing exclusively composite indices to the analytics engine.
• Preserves the default SQL/PPL path for Lucene, mixed-format, missing, empty, or failed resolutions.
• Adds PPL and SQL routing coverage for single- and multi-index aliases, wildcards, comma-separated sources, data streams, and conservative fallback cases.

Related Issues

• Origin: [TAG Observability] OpenSearch Observability Stack compatibility with columnar storage technical-steering#146
• Resolves [BUG] Analytics routing does not resolve aliases and wildcard index expressions #5916

Testing

./gradlew :opensearch-sql-plugin:spotlessCheck :opensearch-sql-plugin:test --tests org.opensearch.sql.plugin.rest.RestUnifiedQueryActionTest

Check List

• New functionality includes testing.
• New functionality has been documented in Javadoc.
• User manual documentation is not applicable; this corrects internal routing without changing syntax or APIs.
• New PPL command checklist is not applicable.
• API specification companion PR is not applicable.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR is not applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue The stripSchemaPrefix method incorrectly handles exclusion patterns (index names starting with '-'). When prefixLength is 1 and lastDot is 0, the substring logic produces "-" + indexName.substring(1), which removes the character after '-' instead of preserving the exclusion marker with the stripped name. For example, "-opensearch.logs" becomes "-logs" correctly, but "-.logs" would become "-logs" when it should remain "-.logs" or handle the edge case differently. int prefixLength = indexName.startsWith("-") ? 1 : 0; int lastDot = indexName.lastIndexOf('.'); return lastDot >= prefixLength ? indexName.substring(0, prefixLength) + indexName.substring(lastDot + 1) : indexName; } Incomplete Validation The method splits indexExpression by comma and trims each part, but does not validate that the resulting expressions are non-empty before passing them to concreteIndexNames. While there is a check for empty strings in the array, if all expressions are empty strings after trimming (e.g., " , , "), the method returns false at line 158 but does not prevent an empty array from reaching the resolver if the split produces non-empty whitespace-only strings that trim to empty. This could cause unexpected behavior depending on how concreteIndexNames handles such input. String[] expressions = Arrays.stream(indexExpression.split(",", -1)) .map(String::trim) .map(this::stripSchemaPrefix) .toArray(String[]::new); if (expressions.length == 0 || Arrays.stream(expressions).anyMatch(String::isEmpty)) { return false; }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix exclusion prefix handling logic The logic for handling exclusion patterns (starting with -) is incomplete. When an index name starts with -, the method preserves the prefix but doesn't correctly handle the schema stripping. For example, -opensearch.logs would become -logs, but the dot position check lastDot >= prefixLength would fail for -logs (lastDot=0, prefixLength=1), returning the original string instead. plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java [197-203] private String stripSchemaPrefix(String indexName) { - int prefixLength = indexName.startsWith("-") ? 1 : 0; - int lastDot = indexName.lastIndexOf('.'); - return lastDot >= prefixLength - ? indexName.substring(0, prefixLength) + indexName.substring(lastDot + 1) - : indexName; + boolean hasExclusionPrefix = indexName.startsWith("-"); + String nameWithoutPrefix = hasExclusionPrefix ? indexName.substring(1) : indexName; + int lastDot = nameWithoutPrefix.lastIndexOf('.'); + String stripped = lastDot >= 0 ? nameWithoutPrefix.substring(lastDot + 1) : nameWithoutPrefix; + return hasExclusionPrefix ? "-" + stripped : stripped; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a logic flaw in handling exclusion patterns (index names starting with -). The current implementation's condition lastDot >= prefixLength would incorrectly handle cases like -opensearch.logs, potentially returning the wrong result. The improved code properly separates the exclusion prefix handling from schema stripping, ensuring correct behavior for all cases.	Medium
General	Add early validation for empty input The empty string check after trimming and schema stripping may not catch all invalid cases. If indexExpression is an empty string, split(",", -1) returns an array with one empty element. After trimming, this becomes an empty string, which is correctly caught. However, the logic should validate the input before processing to avoid unnecessary stream operations. plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java [152-159] +if (indexExpression == null || indexExpression.trim().isEmpty()) { + return false; +} String[] expressions = Arrays.stream(indexExpression.split(",", -1)) .map(String::trim) .map(this::stripSchemaPrefix) + .filter(s -> !s.isEmpty()) .toArray(String[]::new); -if (expressions.length == 0 || Arrays.stream(expressions).anyMatch(String::isEmpty)) { +if (expressions.length == 0) { return false; } Suggestion importance[1-10]: 5 __ Why: The suggestion adds a null check and early validation, which improves code robustness. However, the method allIndicesArePluggableDataformat is called from isAnalyticsIndex which already checks for null/empty query strings, making the null check redundant. The .filter(s -> !s.isEmpty()) addition is useful for handling edge cases with comma-separated empty values, providing moderate value.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.38%. Comparing base (8087c66) to head (9939d64).

❌ Your project check has failed because the head coverage (75.38%) is below the target coverage (99.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #5917   +/-   ##
=========================================
  Coverage     75.38%   75.38%           
  Complexity    10383    10383           
=========================================
  Files           962      962           
  Lines         41079    41079           
  Branches       4679     4679           
=========================================
  Hits          30969    30969           
  Misses         8804     8804           
  Partials       1306     1306           

Flag	Coverage Δ
sql-engine	75.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.