Skip to content

Return 4xx instead of 500 for unsupported window functions#5587

Merged
dai-chen merged 1 commit into
opensearch-project:mainfrom
OVI3D0:fix-unsupported-window-function-status-code
Jun 25, 2026
Merged

Return 4xx instead of 500 for unsupported window functions#5587
dai-chen merged 1 commit into
opensearch-project:mainfrom
OVI3D0:fix-unsupported-window-function-status-code

Conversation

@OVI3D0

@OVI3D0 OVI3D0 commented Jun 25, 2026

Copy link
Copy Markdown
Member

Description

Window functions outside WINDOW_FUNC_MAPPING (e.g. RANK) were escaping the analytics-engine route as HTTP 500. The throw site in CalciteRexNodeVisitor#visitWindowFunction emitted a raw UnsupportedOperationException, which UnifiedQueryPlanner.plan rethrows unchanged, so it never got reclassified as a client error.

Switching the throw to CalciteUnsupportedException lets the 4xx wrapper from #5569 take over and normalize it to SemanticCheckException.

Repro:

SELECT RegionID, COUNT(*) AS cnt,
       RANK() OVER (ORDER BY COUNT(*) DESC) AS rnk
FROM clickbench GROUP BY RegionID LIMIT 5

Test plan

  • New UnifiedQueryPlannerTest#unsupportedWindowFunctionIsRethrownAsSemanticCheckException: fails on main, passes with the fix
  • Existing CalcitePPLEventstatsIT#testUnsupportedWindowFunctions still green
  • Manual repro on a local cluster:
mikeovi@80a997164b98 ~ % curl -sS -w "\nHTTP %{http_code}\n" -X POST "http://localhost:9200/_plugins/_ppl" \
    -H 'Content-Type: application/json' \
    -d '{"query":"source=demo | eventstats rank() | head 5"}'
{
  "error": {
    "context": {
      "stage": "analyzing",
      "stage_description": "Parsing and validating the query"
    },
    "reason": "Unexpected window function: rank",
    "details": "Unexpected window function: rank",
    "location": ["while preparing and validating the query plan"],
    "code": "UNKNOWN",
    "type": "CalciteUnsupportedException"
  },
  "status": 400
}
HTTP 400
mikeovi@80a997164b98 ~ %

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff.

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.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit bc9bc97)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@OVI3D0 OVI3D0 force-pushed the fix-unsupported-window-function-status-code branch from fdb42d7 to 5068696 Compare June 25, 2026 20:54
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 5068696

Window functions outside WINDOW_FUNC_MAPPING (e.g. RANK) used to escape
the AE route as HTTP 500 because the throw site emitted a raw
UnsupportedOperationException, which UnifiedQueryPlanner rethrows
unchanged. Switching to CalciteUnsupportedException lets the existing
4xx wrapper added in opensearch-project#5569 normalize it to SemanticCheckException.

Repro:
  SELECT RegionID, COUNT(*) AS cnt,
         RANK() OVER (ORDER BY COUNT(*) DESC) AS rnk
  FROM clickbench GROUP BY RegionID LIMIT 5
Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
@OVI3D0 OVI3D0 force-pushed the fix-unsupported-window-function-status-code branch from 5068696 to bc9bc97 Compare June 25, 2026 21:07
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit bc9bc97

@dai-chen dai-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@dai-chen dai-chen merged commit 1dc92d6 into opensearch-project:main Jun 25, 2026
40 of 42 checks passed
@OVI3D0 OVI3D0 deleted the fix-unsupported-window-function-status-code branch June 25, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants