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

Fix json formatter for consistency with SQL #492

Conversation

dai-chen
Copy link
Member

Issue #, if available:

Description of changes: The JSON response format was slightly different from SQL's in the datarows field (with one more layer row). This PR is to fix the inconsistency so existing client such as SQL CLI can handle both response in the same way.

Example with this fix:

~ curl -H 'Content-Type: application/json' -XPOST "http://localhost:9200/_opendistro/_ppl" -d'
{
   "query": "source=accounts | fields id"
}'
{
  "schema": [{
    "name": "id",
    "type": "integer"
  }],
  "total": 2,
  "datarows": [
    [4],
    [5]
  ],
  "size": 2
}

Previously without fix:

~ curl -H 'Content-Type: application/json' -XPOST "http://localhost:9200/_opendistro/_ppl" -d'
{
   "query": "source=accounts | fields id"
}'
{
  "schema": [{
    "name": "id",
    "type": "integer"
  }],
  "total": 2,
  "datarows": [
    {"row": [4]},
    {"row": [5]}
  ],
  "size": 2
}

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

@dai-chen dai-chen added the PPL label May 26, 2020
@dai-chen dai-chen self-assigned this May 26, 2020
@dai-chen dai-chen marked this pull request as ready for review May 26, 2020 23:18
Copy link
Member

@chloe-zh chloe-zh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

LGTM

@dai-chen dai-chen merged commit 5ae2203 into opendistro-for-elasticsearch:feature/ppl May 27, 2020
@dai-chen dai-chen deleted the fix-json-formatter-issues branch May 27, 2020 01:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants