xiangfu0 commented 5 years ago

Per issue: https://github.com/winedepot/pinot/issues/14. This PR will traverse all the PQL parsed AST nodes and update PinotQuery. Similar to updateBrokerRequest, this PR implements updatePinotQuery to populate all the required fields of pinotQuery then put pinotQuery into BrokerRequest.

Test cases cover existing PQLs conversion as well as the correctness of PinotQuery fields setup

codecov-io commented 5 years ago

Codecov Report

Merging #17 into develop will decrease coverage by 19.31%. The diff coverage is 69.13%.

@@              Coverage Diff               @@
##             develop      #17       +/-   ##
==============================================
- Coverage      64.04%   44.73%   -19.32%     
==============================================
  Files           1044     1044               
  Lines          53467    53689      +222     
  Branches        7600     7642       +42     
==============================================
- Hits           34245    24018    -10227     
- Misses         16884    27607    +10723     
+ Partials        2338     2064      -274

Impacted Files	Coverage Δ	Complexity Δ
...e/pinot/pql/parsers/pql2/ast/PredicateAstNode.java	`25% <ø> (-25%)`	`0 <0> (ø)`
...t/pql/parsers/pql2/ast/BooleanOperatorAstNode.java	`22.22% <0%> (-9.03%)`	`0 <0> (ø)`
...che/pinot/pql/parsers/pql2/ast/OptionsAstNode.java	`0% <0%> (-100%)`	`0 <0> (ø)`
...ers/pql2/ast/PredicateParenthesisGroupAstNode.java	`0% <0%> (-100%)`	`0 <0> (ø)`
...ache/pinot/pql/parsers/pql2/ast/HavingAstNode.java	`0% <0%> (-100%)`	`0 <0> (ø)`
...ache/pinot/pql/parsers/pql2/ast/OptionAstNode.java	`0% <0%> (-60%)`	`0 <0> (ø)`
...not/pql/parsers/pql2/ast/PredicateListAstNode.java	`72.09% <100%> (-17%)`	`0 <0> (ø)`
...che/pinot/pql/parsers/pql2/ast/GroupByAstNode.java	`100% <100%> (ø)`	`0 <0> (ø)`	:arrow_down:
...ot/pql/parsers/pql2/ast/StarColumnListAstNode.java	`100% <100%> (ø)`	`0 <0> (ø)`	:arrow_down:
...ache/pinot/pql/parsers/pql2/ast/SelectAstNode.java	`78.04% <100%> (-17.61%)`	`0 <0> (ø)`
... and 555 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 9751e3c...1b8fa75. Read the comment docs.

kishoreg commented 5 years ago

Add a description and link it to the issue.

kishoreg commented 5 years ago

Do you mean it's one literal with all values in the IN clause?

Either that or split the literal on ", " and set it as a list. Pinot handles both cases I think. But list is better since we dont have to pay the splitting cost for every segment.

winedepot / pinot

Adding AST parser to PinotQuery insider BrokerRequest #17

Codecov Report