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

Support COUNT star and literal in new engine #802

Conversation

dai-chen
Copy link
Member

@dai-chen dai-chen commented Oct 27, 2020

Issue #, if available: N/A

Description of changes: Support COUNT(*) and COUNT(1) in the new SQL engine.

  1. Convert * to unresolved AllFields and then resolved literal "*". This is because COUNT(*) has same semantic as COUNT(any literal). So there is no need to add a new expression class (SELECT * has its own logic and don't need resolved AllFields expression neither)
  2. Follow our old approach in legacy code by generating Value Count aggregation DSL on metadata field _index for COUNT(*) and COUNT(1).

Example:

POST _opendistro/_sql/_explain
{
  "query": """
    SELECT
      COUNT(*), COUNT(FlightNum), COUNT(1)
    FROM kibana_sample_data_flights
  """
}
{
  "root": {
    "name": "ProjectOperator",
    "description": {
      "fields": "[COUNT(*), COUNT(FlightNum), COUNT(1)]"
    },
    "children": [
      {
        "name": "ElasticsearchIndexScan",
        "description": {
          "request": """ElasticsearchQueryRequest(indexName=kibana_sample_data_flights, sourceBuilder={"from":0,"size":0,"timeout":"1m","aggregations":{"COUNT(1)":{"value_count":{"field":"_index"}},"COUNT(*)":{"value_count":{"field":"_index"}},"COUNT(FlightNum)":{"value_count":{"field":"FlightNum"}}}}, searchDone=false)"""
        },
        "children": []
      }
    ]
  }
}

Documentation: https://github.com/dai-chen/sql/blob/support-count-star-and-literal/docs/user/dql/aggregations.rst#count-aggregations

Testing:

  1. Add new UT and comparison IT.
  2. Ignore one failed IT PrettyFormatResponseIT.aggregationFunctionInSelectCaseCheck(). Please see the reason on @Ignore message.

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 SQL label Oct 27, 2020
@dai-chen dai-chen self-assigned this Oct 27, 2020
@dai-chen dai-chen mentioned this pull request Oct 27, 2020
@dai-chen dai-chen force-pushed the support-count-star-and-literal branch from 1062bc3 to bfaf3c8 Compare October 27, 2020 23:00
@dai-chen dai-chen marked this pull request as ready for review October 28, 2020 00:19
@dai-chen dai-chen requested review from penghuo and chloe-zh October 28, 2020 00:19
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.

Thanks for the change!

@dai-chen dai-chen merged commit a718f58 into opendistro-for-elasticsearch:develop Oct 30, 2020
@dai-chen dai-chen deleted the support-count-star-and-literal branch October 30, 2020 00:39
penghuo pushed a commit that referenced this pull request Dec 15, 2020
* Support count star and literal

* Add UT

* Add UT

* Add UT

* Add comparison IT and doctest

* Ignore failed IT

* Change javadoc

* Add UT for in-memory count aggregator
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