Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for handling simple attribute expression #106

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

sarthak77
Copy link
Member

Ref issue : #104

@codecov
Copy link

codecov bot commented Nov 25, 2021

Codecov Report

Merging #106 (8cece3d) into main (d7615f2) will decrease coverage by 0.25%.
The diff coverage is 57.14%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #106      +/-   ##
============================================
- Coverage     82.48%   82.22%   -0.26%     
- Complexity      421      428       +7     
============================================
  Files            37       37              
  Lines          1604     1615      +11     
  Branches        169      175       +6     
============================================
+ Hits           1323     1328       +5     
- Misses          207      208       +1     
- Partials         74       79       +5     
Flag Coverage Δ
integration 82.22% <57.14%> (-0.26%) ⬇️
unit 80.05% <57.14%> (-0.25%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...service/pinot/QueryRequestToPinotSQLConverter.java 86.11% <57.14%> (-1.86%) ⬇️

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 d7615f2...8cece3d. Read the comment docs.

@github-actions

This comment has been minimized.

@sarthak77 sarthak77 marked this pull request as ready for review November 25, 2021 04:49
@sarthak77 sarthak77 requested review from a team and aaron-steinfeld November 25, 2021 04:49
@github-actions

This comment has been minimized.

String logicalColumnName = expression.getColumnIdentifier().getColumnName();
String col = viewDefinition.getKeyColumnNameForMap(logicalColumnName);
if ((expression.getValueCase() == COLUMNIDENTIFIER)
|| (expression.getValueCase() == ATTRIBUTE_EXPRESSION && !isComplexAttribute(expression))) {

Choose a reason for hiding this comment

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

nit: (expression.getValueCase() == ATTRIBUTE_EXPRESSION && !isComplexAttribute(expression)) could be extracted into a method

Copy link
Member Author

Choose a reason for hiding this comment

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

resolved

@sarthak77 sarthak77 merged commit 0ec935f into main Nov 25, 2021
@sarthak77 sarthak77 deleted the feat_support_for_simple_attr branch November 25, 2021 06:37
@github-actions
Copy link

Unit Test Results

  23 files  +1    23 suites  +1   8s ⏱️ -1s
134 tests +8  134 ✔️ +8  0 💤 ±0  0 ❌ ±0 

Results for commit 0ec935f. ± Comparison against base commit d7615f2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants