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

Add post processing logic for aggregation query. #346

Merged

Conversation

penghuo
Copy link
Contributor

@penghuo penghuo commented Jan 14, 2020

Issue #, if available:

Description of changes:

Problem Statement

Support expression over aggregation function is an important feature has been request a lot by customer. The current SQL engine try to translate all the expression to painless script if possible which limit the implementation. This PR works on add the post processing logic into the current framework for aggregation query which is the first step to enable the expression over aggregation function support.

Change Notes

  1. Construct the QueryPlan for aggregation query if possible.
  2. Add the SQLExpr to PhysicalOperator convert.
  3. Add the ExprValue and Expression(SUM, SUB, LOG)
  4. Change the JDBC and CSF Formatter.

Integration Test changes

  1. CsvFormatResponseIT, fix the incorrect column assertion
  2. PrettyFormatResponseIT, fix the incorrect column assertion
  3. SQLFunctionsIT, fix the incorrect column assertion

Todo

The next PR will address the following pieces

  1. More Expression support
  2. Integration Test cases for expression over aggregation function
    By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Member

@dai-chen dai-chen 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 adding this important feature!

}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Does it work if the number of parameters in basic arithmetic expressions is indeterminate? eg. max(field_1) + max(field_2) + max(field_3) + ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will be compile as add(max(field_1),add(max(field_2),max(field_3)).

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!

@penghuo penghuo merged commit 239908b into opendistro-for-elasticsearch:master Jan 22, 2020
@penghuo penghuo deleted the improve-agg-expression branch April 22, 2020 03:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants