Support trendline command in Calcite#3741
Conversation
Signed-off-by: Songkan Tang <songkant@amazon.com>
Signed-off-by: Songkan Tang <songkant@amazon.com>
|
CI failures CalcitePPLAppendcolTest > ... FAILED is not related to this fixing. |
LantaoJin
left a comment
There was a problem hiding this comment.
Will wma work in v2? Please add wma related documents in trendline.rst
docs/user/ppl/cmd/trendline.rst
Outdated
|
|
||
| Limitation | ||
| ========== | ||
| The ``trendline`` command will filter out all NULL values to make sure result correctness because it's meaningless to count NULL values. But this may reduce lines in result for further processing. No newline at end of file |
There was a problem hiding this comment.
Change to:
Starting with version 3.1.0, the trendline command requires all values in the specified field to be non-null. Any null values present in the calculation field will be automatically excluded from the command's output.
|
|
||
| trendlineType | ||
| : SMA | ||
| | WMA |
There was a problem hiding this comment.
update doc to explain new algorithm?
…lcite Signed-off-by: Songkan Tang <songkant@amazon.com>
|
@LantaoJin @penghuo Updated the doc with new wma algorithm and callout it's supported with Calcite enabled. |
| Syntax | ||
| ============ | ||
| `TRENDLINE [sort <[+|-] sort-field>] SMA(number-of-datapoints, field) [AS alias] [SMA(number-of-datapoints, field) [AS alias]]...` | ||
| `TRENDLINE [sort <[+|-] sort-field>] [SMA|WMA](number-of-datapoints, field) [AS alias] [[SMA|WMA](number-of-datapoints, field) [AS alias]]...` |
There was a problem hiding this comment.
Should this command later enforce sorting either on an implicit timestamp field?
There was a problem hiding this comment.
Should this command later enforce sorting either on an implicit timestamp field?
We could wait for more user feedbacks. Seems more than one command could be affected with an implicit timestamp field.
There was a problem hiding this comment.
Yeah, it could depend on customer feature request.
Signed-off-by: Songkan Tang <songkant@amazon.com>
|
@LantaoJin @dai-chen Fixed typo and make some doc phrasing clearer. |
| import org.json.JSONObject; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class CalcitePPLTrendlineIT extends CalcitePPLIntegTestCase { |
There was a problem hiding this comment.
missing CalcitePPLTrendlinePushdownIT if we add IT in standalone
There was a problem hiding this comment.
It's added now.
Signed-off-by: Songkan Tang <songkant@amazon.com>
Signed-off-by: Songkan Tang <songkant@amazon.com>
* Support trendline command in Calcite Signed-off-by: Songkan Tang <songkant@amazon.com> * Fix CalciteExplainIT for trendline command Signed-off-by: Songkan Tang <songkant@amazon.com> * Update trendline.rst doc to callout new wma algorithm supported by Calcite Signed-off-by: Songkan Tang <songkant@amazon.com> * Fix typo in the doctest and rephrase the doc and formula Signed-off-by: Songkan Tang <songkant@amazon.com> * Add missing trendline pushdown IT Signed-off-by: Songkan Tang <songkant@amazon.com> * Remove unexpected change Signed-off-by: Songkan Tang <songkant@amazon.com> --------- Signed-off-by: Songkan Tang <songkant@amazon.com> Signed-off-by: xinyual <xinyual@amazon.com>
Description
Support trendline command in Calcite
Related Issues
Resolves #3466
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.