Remove GetAlias Call

[aparajita31pandey]

Description

This diff is removes a redundant alias-resolution call via the GET /_alias/<alias> API that requires the caller to have extra indices:admin/aliases/get privileges, which can cause permission issues when executing read queries.

It instead leverages existing GetFieldMapping call that works for both index and alias.

Tested Functionality

With Index Name

curl -X POST "localhost:9200/_plugins/_sql"  -H 'Content-Type: application/json' \
  -d '{
    "query": "SELECT * FROM my-index",
    "fetch_size": 1,
    "filter": {
      "term": {
        "another_field": "hello"
      }
    }
  }'
Output - 
{
  "schema": [
    {
      "name": "new_field",
      "type": "text"
    },
    {
      "name": "another_field",
      "type": "keyword"
    }
  ],
  "total": 1,
  "datarows": [[
    "Some text",
    "hello"
  ]],
  "size": 1,
  "status": 200
}

With Alias Name - Same Output

curl -X POST "localhost:9200/_plugins/_sql"   -H 'Content-Type: application/json' \
  -d '{
    "query": "SELECT * FROM my-alias",
    "fetch_size": 1,
    "filter": {
      "term": {
        "another_field": "hello"
      }
    }
  }'
Output - 
{
  "schema": [
    {
      "name": "new_field",
      "type": "text"
    },
    {
      "name": "another_field",
      "type": "keyword"
    }
  ],
  "total": 1,
  "datarows": [[
    "Some text",
    "hello"
  ]],
  "size": 1,
  "status": 200
}

Related Issues

Resolves
#2960, opensearch-project/security#5871

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[coderabbitai]

Warning

Rate limit exceeded

@aparajita31pandey has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 66a475b and d8aa89b.

📒 Files selected for processing (1)

• legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java

📝 Walkthrough

Walkthrough

Removed runtime alias-resolution and alias-based indexName override in SelectResultSet; mapping validation now requires exactly one mapping entry and retrieves that mapping directly, with updated error messages and control flow adjusted to the single-mapping assumption.

Changes

Cohort / File(s)	Change Summary
Alias resolution & mapping validation legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java	Removed GetAliasesRequest/GetAliasesResponse usage and alias-based indexName override in loadFromEsState. Enforced mappings.size() == 1 and throw IllegalArgumentException when not met. Retrieve type mappings via mappings.values().iterator().next() and updated error text and subsequent control flow for the single-mapping assumption.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Focus areas:
  • Confirm callers and tests don't rely on previous alias-to-original index substitution.
  • Verify the single-mapping requirement is correct for multi-mapping indices or legacy types.
  • Check error handling and log/messages consistency.

Possibly related PRs

• Refactor alias type field by adding another project with alias #4881 — Related changes to alias-resolution behavior (removal of runtime alias-to-original-path replacement).

Suggested labels

enhancement

Suggested reviewers

• kavithacm
• derek-ho
• penghuo
• seankao-az
• anirudha

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: removing a GetAlias API call that was unnecessary for the query execution flow.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check	✅ Passed	The PR description clearly explains the change: removing a redundant GetAlias API call that required extra permissions, and leveraging an existing GetFieldMapping call instead. It includes tested examples demonstrating identical behavior with both index and alias names.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbcdbd6 and 8b7cb6a.

📒 Files selected for processing (1)

• legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java

🧰 Additional context used

📓 Path-based instructions (1)

**/*.java

📄 CodeRabbit inference engine (.rules/REVIEW_GUIDELINES.md)

**/*.java: Use PascalCase for class names (e.g., QueryExecutor)
Use camelCase for method and variable names (e.g., executeQuery)
Use UPPER_SNAKE_CASE for constants (e.g., MAX_RETRY_COUNT)
Keep methods under 20 lines with single responsibility
All public classes and methods must have proper JavaDoc
Use specific exception types with meaningful messages for error handling
Prefer Optional<T> for nullable returns in Java
Avoid unnecessary object creation in loops
Use StringBuilder for string concatenation in loops
Validate all user inputs, especially queries
Sanitize data before logging to prevent injection attacks
Use try-with-resources for proper resource cleanup in Java
Maintain Java 11 compatibility when possible for OpenSearch 2.x
Document Calcite-specific workarounds in code

Files:

• legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java

---

⚙️ CodeRabbit configuration file

**/*.java: - Flag methods >50 lines as potentially too complex - suggest refactoring

• Flag classes >500 lines as needing organization review
• Check for dead code, unused imports, and unused variables
• Identify code reuse opportunities across similar implementations
• Assess holistic maintainability - is code easy to understand and modify?
• Flag code that appears AI-generated without sufficient human review
• Verify Java naming conventions (PascalCase for classes, camelCase for methods/variables)
• Check for proper JavaDoc on public classes and methods
• Flag redundant comments that restate obvious code
• Ensure proper error handling with specific exception types
• Check for Optional usage instead of null returns
• Validate proper use of try-with-resources for resource management

Files:

• legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)

• GitHub Check: security-it-linux (25)
• GitHub Check: security-it-linux (21)
• GitHub Check: bwc-tests-full-restart (21)
• GitHub Check: build-linux (21, doc)
• GitHub Check: build-linux (25, integration)
• GitHub Check: build-linux (21, integration)
• GitHub Check: bwc-tests-rolling-upgrade (25)
• GitHub Check: build-linux (25, unit)
• GitHub Check: bwc-tests-rolling-upgrade (21)
• GitHub Check: build-linux (21, unit)
• GitHub Check: build-linux (25, doc)
• GitHub Check: bwc-tests-full-restart (25)
• GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, unit)
• GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, unit)
• GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, integration)
• GitHub Check: build-windows-macos (macos-14, 21, integration)
• GitHub Check: build-windows-macos (macos-14, 21, unit)
• GitHub Check: build-windows-macos (macos-14, 25, integration)
• GitHub Check: build-windows-macos (macos-14, 25, doc)
• GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, integration)
• GitHub Check: build-windows-macos (macos-14, 25, unit)
• GitHub Check: build-windows-macos (macos-14, 21, doc)
• GitHub Check: security-it-windows-macos (macos-14, 21)
• GitHub Check: security-it-windows-macos (macos-14, 25)
• GitHub Check: security-it-windows-macos (windows-latest, 21)
• GitHub Check: security-it-windows-macos (windows-latest, 25)
• GitHub Check: test-sql-cli-integration (21)
• GitHub Check: CodeQL-Scan (java)
• GitHub Check: WhiteSource Security Check

🔇 Additional comments (1)

legacy/src/main/java/org/opensearch/sql/legacy/executor/format/SelectResultSet.java (1)

175-181: Verify that the mapping validation correctly handles index aliases and patterns that resolve to multiple underlying indices in JOIN queries.

The change from checking index name presence to requiring mappings.size() == 1 is a significant functional change. While the review correctly notes that loadFromEsState() is called per table in JOIN queries (which should isolate each table), the concern about index patterns and aliases resolving to multiple indices remains valid. Since aliases can point to one or more indices, you need to verify:

1. How response.mappings() behaves when a user specifies a table name that is an alias pointing to multiple indices (does ES return separate entries for each underlying index or one consolidated entry?)
2. Whether integration tests cover scenarios where table names resolve to multi-index aliases
3. How the previous mapping resolution logic handled this case and why it was changed

If the mappings response consolidates multiple underlying indices into a single entry when queried by alias name, the change is safe. If it returns separate entries per underlying index, the validation will incorrectly reject valid multi-index alias queries.

[aparajita31pandey]

@LantaoJin Can I please get a review ?