This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 186
Handle the elasticsearch exceptions in JDBC formatted outputs #362
Merged
chloe-zh
merged 19 commits into
opendistro-for-elasticsearch:master
from
chloe-zh:es-exception
Mar 17, 2020
Merged
Handle the elasticsearch exceptions in JDBC formatted outputs #362
chloe-zh
merged 19 commits into
opendistro-for-elasticsearch:master
from
chloe-zh:es-exception
Mar 17, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing details method
dai-chen
reviewed
Feb 4, 2020
src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/Protocol.java
Outdated
Show resolved
Hide resolved
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Outdated
Show resolved
Hide resolved
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Outdated
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/ErrorMessage.java
Outdated
Show resolved
Hide resolved
penghuo
reviewed
Feb 4, 2020
@@ -42,6 +42,7 @@ | |||
public final static String TEST_INDEX_JOIN_TYPE = TEST_INDEX + "_join_type"; | |||
public final static String TEST_INDEX_BANK = TEST_INDEX + "_bank"; | |||
public final static String TEST_INDEX_BANK_TWO = TEST_INDEX_BANK + "_two"; | |||
public final static String TEST_INDEX_BANK_WITH_NULL_VALUES = TEST_INDEX_BANK + "_with_null_values"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to merge with the existing bank index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if that will cause broken tests that use the bank index if inject null value into it.
src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java
Show resolved
Hide resolved
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/Protocol.java
Outdated
Show resolved
Hide resolved
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Outdated
Show resolved
Hide resolved
davidcui1225
reviewed
Feb 4, 2020
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Outdated
Show resolved
Hide resolved
…pe for ErrorMessage
dai-chen
reviewed
Feb 11, 2020
src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/ErrorMessage.java
Show resolved
Hide resolved
...main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/ErrorMessageFactory.java
Show resolved
Hide resolved
penghuo
reviewed
Feb 13, 2020
src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/Protocol.java
Outdated
Show resolved
Hide resolved
...ava/com/amazon/opendistroforelasticsearch/sql/executor/format/ElasticsearchErrorMessage.java
Outdated
Show resolved
Hide resolved
penghuo
reviewed
Feb 28, 2020
src/main/java/com/amazon/opendistroforelasticsearch/sql/plugin/RestSqlAction.java
Outdated
Show resolved
Hide resolved
penghuo
reviewed
Feb 28, 2020
src/main/java/com/amazon/opendistroforelasticsearch/sql/plugin/RestSqlStatsAction.java
Outdated
Show resolved
Hide resolved
davidcui1225
approved these changes
Mar 2, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
penghuo
approved these changes
Mar 2, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the change!
penghuo
pushed a commit
to penghuo/sql
that referenced
this pull request
May 2, 2020
…stro-for-elasticsearch#362) * Caught ES exception * Added details in errMsgs to enrich the behavior; added IT * Handled cases where ES exceptions are wrapped up; added default fetching details method * Added factory method to construct ErrorMessage; extended exception type for ErrorMessage * Added UT for ErrorMessageFactory * addressed comments (cherry picked from commit 71aba38)
penghuo
added a commit
that referenced
this pull request
May 5, 2020
* Update the opendistro sql 1.4.0 release notes (#359) (cherry picked from commit d9fe9dc) * adding DATETIME cast support (#310) Adding full support for CAST(), and adding it as a function. Fixed Datetime casting to be UTC-timezone default. (cherry picked from commit 68b971f) * Documentation for simple query (#366) * Add doc test for simple query * Add to index * Fix format * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add documentation * Add rdd * Add rdd * Add rdd * Add rdd * Fix order by example * Add more diagrams * Add more diagrams * Skip doctest in gradlew build (cherry picked from commit 57b379c) * Return Correct Type Information for Fields (#365) Return correct type information for fields in JDBC format and changed the return type of trig functions to return DOUBLE. (cherry picked from commit 9769c30) * Report date data as a standardized format (#367) * expose date fields using a standardized date format * move date format logic to new class * add tests for DateTimeFormatter * added some negative tests * style fixes * testing locale fixes * addressed code review comments and build failures * post-merge fix * additional fixes * get CAST alias info from result set class, rather than cluster state * remove unused import * remove unused import * reduce duplication & reference enum value * setting default timezone to UTC while parsing date values * add support for custom & multiple formats * add case for Kibana flights date data with T but no time field (cherry picked from commit 7c57d72) * Integration test with external ES cluster (#374) * Fix build issue by migrating to RestTestCase * Fix query IT * Fix subquery IT * Fix csv formatter and SQL functions IT * Fix aggregation and delete IT * Fix date IT * Fix jdbc IT * Fix show and metadata IT * Fix subquery IT * Fix date functions UT * Fix test data setup and cleanup issue * Fix correctness IT * Fix doctest IT * Add JavaDoc on base class * Address PR comments * Address PR comments (cherry picked from commit 56a2f44) * Bug fix, return object type for field which has implicit object datatype when describe the table (#377) * Bug fix, return object type for field which has implicit object datatype when describe the table (cherry picked from commit c30e342) * Pagination doc (#379) * Add design details in doc * Added images for pagination doc * Add image links and fix formatting * Add cursor setting details * Add Salient Points section in Detailed Design section (cherry picked from commit ac8a020) * Handle the elasticsearch exceptions in JDBC formatted outputs (#362) * Caught ES exception * Added details in errMsgs to enrich the behavior; added IT * Handled cases where ES exceptions are wrapped up; added default fetching details method * Added factory method to construct ErrorMessage; extended exception type for ErrorMessage * Added UT for ErrorMessageFactory * addressed comments (cherry picked from commit 71aba38) * Modified the wording of exception messages and created the troubleshooting page (#372) * Edit the wording of exception messages * Created troubleshooting page * Revised troubleshooting page * addressed comments (cherry picked from commit a4afc75) * Sql CI/CD (#384) * Create CD.yml * Create CI.yml (cherry picked from commit fa1d4a0) * FIX: field function name letter case preserved in select with group by (#381) * FIX: Method field name * REF: parser logic * RMV: remove unused function * STY: unused imports * STY: unused import (cherry picked from commit ad8ad3a) * Fix broken LICENSE link in README.md (#394) * Fix broken License link * Reverting NOTICE name change (cherry picked from commit 62e36c9) * New SQL cluster settings endpoint (#400) * Add new _sql/settings endpoint, and logic to only affect opendistro.sql settings * Add integration tests * Change endpoint HTTP method to PUT * Update Settings doc (cherry picked from commit f1d538f) * Bug Fix, add support for strict_date_optional_time (#412) (cherry picked from commit 9ed430f) * Invalidate HTTP GET method (#414) * Removed http GET method; added integTest * Removed GET method in doc and doctest (cherry picked from commit 56464f0) * More docs in reference manual and add architecture doc (#417) * Initial commit for PartiQL and complex query docs * Initial commit for PartiQL and complex query docs * Initial commit for PartiQL and complex query docs * Ignore multi-query for now because of bug * Add test index mappings * Fix partiql doc * Bypass LEFT JOIN for now * Add doc for JOINs * Add doc for JOINs * Add doc for JOINs * Add more cases for PartiQL * Add more cases for PartiQL * Add multi-line support * Add multi-line support * Multi-line all complex queries * Multi-line all complex queries * Add doc for full text search * Add doc for full text search * Add doc for metadata query * Add doc for multi match query * Add doc for delete statement * Remove join explain * Add RDD * Add RDD * Print test data for PartiQL * Print test data for PartiQL * Change titles * Add architecture doc * Add docs for SQL functions * Update index * Update docs * Update docs * Update docs * Address PR comments (cherry picked from commit 6457b0b) * Bug fix, support subquery in from doesn't have alias (#418) (cherry picked from commit 5e5f485) * Anonymize sensitive data in queries exposed to RestSqlAction logs (#419) * remove sensitive data from queries for logging * Added rule to mask sensitive data from es logs * Applied API in SQLUtils to rebuild query string from AST; replace data masks with anonymous words * Inlined log message; added doc for new rule (cherry picked from commit 45fa29c) * Bug fix, ignore the term query rewrite if there is no index found (#425) * Bug fix, ignore the term query rewrite if there is no index found * move IsEqualIgnoreCaseAndWhiteSpace to MatcherUtils (cherry picked from commit 2bfe326) * Simple Query Cursor support (#390) * Add integration tests to be passed * Add cluster settings for cursor - enabled, fetch_size, keep_alive * Add fetch_size and cursor params. fetch_size validation * new SqlRequest constructor for cursor * Add logic to open scroll based on settings, fetch_size and limit values * Add cursor close endpoint * Remove date formatting changes * Fix unit and integ tests, Ignored date format tests for a while, synced previous cursor changes * Add cursor generation * Add test helper methods * Cursor close API * Remove commented code and add partial date formatting change * Add error metrics when not able to close cursor * Add indexname and fieldAliasMap to cursor context * Remove ignored test cases affected by date formatting changes * Remove unneeded interface, refactor CursorType enum * Remove logs, unneeded fields, comments, refactor * Disable cursor by default * Fix cursor for parameterized request, add integration test for same * LIMIT changes * Changes to handle different LIMIT cases * Add default cursor metrics * Add integration test on explain cursor * Update monitoring, settings and endpoint docs * Refactor cursor classes to separate package * Add Lombok for DefaultCursor * Add unit test for DefaultCursor * Update doc * Unit tests, bug fix , refactoring (cherry picked from commit bf4810d) * [BugFix] Enforce AVG to return double data type (#437) * Enforce AVG to return double. Update UT, add IT. (cherry picked from commit 5870bbf) * Bug fix, count(distinct field) should transalte to cardinality aggregation (#442) (cherry picked from commit b3df480) * Fix CSV injection issue (#447) * Sanitize to avoid CSV injection * Add IT (cherry picked from commit 78d4158) * [BugFix] mock LocalClusterState settings in QueryPlanner base class (#446) (cherry picked from commit 6525dca) * Escape comma for CSV header and all queries (#456) (cherry picked from commit ced1cd5) * Bug Fix, support using aggregation function in order by clause (#452) * Bug Fix, support using aggregation function in order by clause * address comments (cherry picked from commit 30b76ce) * Remove CI and change release workflow based on tags (#457) (cherry picked from commit 9758598) Co-authored-by: David Cui <53581635+davidcui-amzn@users.noreply.github.com> Co-authored-by: Chen Dai <46505291+dai-chen@users.noreply.github.com> Co-authored-by: Jordan Wilson <37088125+jordanw-bq@users.noreply.github.com> Co-authored-by: Abbas Hussain <abbas_10690@yahoo.com> Co-authored-by: Chloe <chloezh1102@gmail.com> Co-authored-by: Rishabh Singh <rishabhksingh@gmail.com> Co-authored-by: Qi Chen <chenqi0805@gmail.com> Co-authored-by: Zhongnan Su <szhongna@amazon.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
The JDBC output and Json formatted result have different behaviors when exceptions are thrown from the ES engine. Typically these exceptions are due to bad requests, cluster issues and so forth. Currently we have tested out a couple type of such exceptions, the are either indeed or their root causes are
ElasticsearchException
instances.It's unlikely to remodel JDBC driver to give the exactly same behavior with the ES raw response due to the limited JDBC types. This PR proposes to only change the behavior of the JDBC output and remain the Json output as it is looking like. The proposed behavior is looking like:
Query example:
SELECT CASE LOWER(OriginWeather) WHEN 'sunny' THEN '1' ELSE '2' END AS cases FROM kibana_sample_data_flights
Current behaviors look like:
Json format (thrown directly from the ES engine, which would be remained)
JDBC output:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.