From faff812bafe3a36ee868d48dc574c1079dc9b5cd Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 11 Nov 2022 19:31:54 -0800 Subject: [PATCH] fix bug Signed-off-by: Peng Huo --- .../opensearch/sql/executor/QueryService.java | 6 +++- .../sql/executor/QueryServiceTest.java | 28 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/executor/QueryService.java b/core/src/main/java/org/opensearch/sql/executor/QueryService.java index 32fda1ab2c..dcdf6bc010 100644 --- a/core/src/main/java/org/opensearch/sql/executor/QueryService.java +++ b/core/src/main/java/org/opensearch/sql/executor/QueryService.java @@ -39,7 +39,11 @@ public class QueryService { */ public void execute(UnresolvedPlan plan, ResponseListener listener) { - executePlan(analyze(plan), PlanContext.emptyPlanContext(), listener); + try { + executePlan(analyze(plan), PlanContext.emptyPlanContext(), listener); + } catch (Exception e) { + listener.onFailure(e); + } } /** diff --git a/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java b/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java index c65210e97e..d1ffa51fcc 100644 --- a/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java +++ b/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java @@ -8,6 +8,7 @@ package org.opensearch.sql.executor; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; @@ -64,7 +65,7 @@ class QueryServiceTest { @BeforeEach public void setUp() { lenient().when(analyzer.analyze(any(), any())).thenReturn(logicalPlan); - when(planner.plan(any())).thenReturn(plan); + lenient().when(planner.plan(any())).thenReturn(plan); queryService = new QueryService(analyzer, executionEngine, planner); } @@ -86,7 +87,7 @@ public void testExecuteShouldPass() { new ResponseListener<>() { @Override public void onResponse(ExecutionEngine.QueryResponse pplQueryResponse) { - + assertNotNull(pplQueryResponse); } @Override @@ -115,7 +116,7 @@ public void testExplainShouldPass() { new ResponseListener<>() { @Override public void onResponse(ExecutionEngine.ExplainResponse pplQueryResponse) { - + assertNotNull(pplQueryResponse); } @Override @@ -185,7 +186,7 @@ public void testExecutePlanShouldPass() { new ResponseListener<>() { @Override public void onResponse(ExecutionEngine.QueryResponse pplQueryResponse) { - + assertNotNull(pplQueryResponse); } @Override @@ -194,4 +195,23 @@ public void onFailure(Exception e) { } }); } + + @Test + public void analyzeExceptionShouldBeCached() { + when(analyzer.analyze(any(), any())).thenThrow(IllegalStateException.class); + + queryService.execute( + ast, + new ResponseListener<>() { + @Override + public void onResponse(ExecutionEngine.QueryResponse pplQueryResponse) { + fail(); + } + + @Override + public void onFailure(Exception e) { + assertTrue(e instanceof IllegalStateException); + } + }); + } }