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

Refine PPL head command syntax #1022

Merged
merged 5 commits into from Feb 4, 2021

Conversation

chloe-zh
Copy link
Member

@chloe-zh chloe-zh commented Jan 29, 2021

Issue #, if available:
#930

Description of changes:

  • Modified head command syntax to simply head [N] where the optional argument N is the number of results to return
  • Removed head operator and replaced the original use of head operator with limit operator
  • Updated user manual

Breaking Changes Note: We have made breaking changes in the syntax of head command from previous head [keeplast = (true | false)] [while "("<boolean-expression>")"] [N] to current head [N]. We are no longer supporting the keeplast and while options.

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

# Conflicts:
#	elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/protector/ElasticsearchExecutionProtector.java
@chloe-zh chloe-zh self-assigned this Jan 29, 2021
@chloe-zh chloe-zh added PPL Breaking Changes Breaking Changes that will impact clients labels Jan 29, 2021
@codecov
Copy link

codecov bot commented Jan 29, 2021

Codecov Report

Merging #1022 (1340e91) into develop (614c500) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1022      +/-   ##
=============================================
+ Coverage      99.87%   99.89%   +0.01%     
+ Complexity      2409     2402       -7     
=============================================
  Files            234      234              
  Lines           5527     5464      -63     
  Branches         358      326      -32     
=============================================
- Hits            5520     5458      -62     
  Misses             5        5              
+ Partials           2        1       -1     
Impacted Files Coverage Δ Complexity Δ
...endistroforelasticsearch/sql/executor/Explain.java 100.00% <ø> (ø) 27.00 <0.00> (-1.00)
...relasticsearch/sql/planner/DefaultImplementor.java 100.00% <ø> (ø) 15.00 <0.00> (-1.00)
...sticsearch/sql/planner/logical/LogicalPlanDSL.java 100.00% <ø> (ø) 15.00 <0.00> (-1.00)
...ch/sql/planner/logical/LogicalPlanNodeVisitor.java 100.00% <ø> (ø) 15.00 <0.00> (-1.00)
...icsearch/sql/planner/physical/PhysicalPlanDSL.java 100.00% <ø> (ø) 14.00 <0.00> (-1.00)
.../sql/planner/physical/PhysicalPlanNodeVisitor.java 100.00% <ø> (ø) 15.00 <0.00> (-1.00)
...tor/protector/ElasticsearchExecutionProtector.java 100.00% <ø> (ø) 19.00 <0.00> (-1.00)
...orelasticsearch/sql/ppl/utils/ArgumentFactory.java 100.00% <ø> (+1.61%) 24.00 <0.00> (-4.00) ⬆️
...ndistroforelasticsearch/sql/analysis/Analyzer.java 100.00% <100.00%> (ø) 43.00 <0.00> (-1.00)
...troforelasticsearch/sql/ppl/parser/AstBuilder.java 100.00% <100.00%> (ø) 33.00 <2.00> (ø)
... and 15 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 614c500...1340e91. Read the comment docs.

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.

Just a minor comment. Thanks for the refactoring!

docs/experiment/ppl/cmd/head.rst Show resolved Hide resolved
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!

@chloe-zh chloe-zh merged commit 2347a2a into opendistro-for-elasticsearch:develop Feb 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Changes Breaking Changes that will impact clients PPL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants