Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Adding additional logging to ExprMissingValue exception #973

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

FreCap
Copy link
Contributor

@FreCap FreCap commented Jan 7, 2021

Issue #, if available:

In a simple GROUP BY queries I'm getting the following exception and need more info about the problem:

{
  "error": {
    "reason": "There was internal problem at backend",
    "details": "invalid value operation on MISSING_VALUE",
    "type": "IllegalStateException"
  },
  "status": 500
}

FAILS

POST _opendistro/_sql
{
  "query": "select ABC.Status, count(*) from my_index group by ABC.Status"
}

WORKS with ?format=json

POST _opendistro/_sql?format=json
{
  "query": "select ABC.Status, count(*) from my_index group by ABC.Status"
}

Description of changes:
Adding additional details to the error message

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

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #973 (0bbfc2d) into develop (4358ed2) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head 0bbfc2d differs from pull request most recent head ceded25. Consider uploading reports for the commit ceded25 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #973      +/-   ##
=============================================
- Coverage      99.89%   99.87%   -0.03%     
+ Complexity      2412     2350      -62     
=============================================
  Files            236      232       -4     
  Lines           5503     5413      -90     
  Branches         329      349      +20     
=============================================
- Hits            5497     5406      -91     
  Misses             5        5              
- Partials           1        2       +1     
Impacted Files Coverage Δ
...orelasticsearch/sql/ppl/utils/ArgumentFactory.java 98.38% <0.00%> (-1.62%) ⬇️
...opendistroforelasticsearch/sql/expression/DSL.java 100.00% <0.00%> (ø)
...endistroforelasticsearch/sql/executor/Explain.java 100.00% <0.00%> (ø)
...ndistroforelasticsearch/sql/analysis/Analyzer.java 100.00% <0.00%> (ø)
...stroforelasticsearch/sql/data/model/ExprValue.java 100.00% <0.00%> (ø)
...troforelasticsearch/sql/ppl/parser/AstBuilder.java 100.00% <0.00%> (ø)
...troforelasticsearch/sql/utils/ExpressionUtils.java 100.00% <0.00%> (ø)
...roforelasticsearch/sql/data/type/ExprCoreType.java 100.00% <0.00%> (ø)
...forelasticsearch/sql/data/model/ExprTimeValue.java 100.00% <0.00%> (ø)
...orelasticsearch/sql/sql/parser/AstSortBuilder.java 100.00% <0.00%> (ø)
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eeb805...ceded25. Read the comment docs.

@anirudha
Copy link
Contributor

anirudha commented Feb 4, 2021

@FreCap any update from your side, thanks

@chloe-zh chloe-zh requested review from penghuo and dai-chen February 5, 2021 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants