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

feat: add scalar functions in postgres #165

Merged
merged 2 commits into from
Aug 11, 2022

Conversation

saxenakshitiz
Copy link
Contributor

No description provided.

@saxenakshitiz saxenakshitiz requested a review from a team August 11, 2022 15:21
@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #165 (762a89a) into main (4bf632a) will increase coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #165      +/-   ##
============================================
+ Coverage     78.02%   78.11%   +0.08%     
  Complexity      758      758              
============================================
  Files            79       79              
  Lines          3318     3317       -1     
  Branches        371      371              
============================================
+ Hits           2589     2591       +2     
+ Misses          568      565       -3     
  Partials        161      161              
Flag Coverage Δ
integration 78.11% <ø> (+0.08%) ⬆️
unit 70.09% <ø> (+0.08%) ⬆️

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

Impacted Files Coverage Δ
...postgres/converters/PostgresFunctionConverter.java 64.51% <ø> (-0.29%) ⬇️
...gres/converters/DefaultColumnRequestConverter.java 81.70% <0.00%> (+1.21%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

CREATE OR REPLACE FUNCTION dateTimeConvert (bigint, bigint) RETURNS bigint AS $$ select ((($1 + $2 - 1)/$2)*$2) $$ LANGUAGE SQL;

CREATE OR REPLACE FUNCTION conditional (condition text, first text, second text)
Copy link
Contributor

Choose a reason for hiding this comment

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

should condition be text or boolean? similarly, is there any kind of "any" type for first and second?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what we discussed yesterday right that condition can be null as well. That is reason I have used text, and converting to boolean only if it is not null.

All arguments must be of specific type in postgres functions.

Copy link
Contributor

@aaron-steinfeld aaron-steinfeld left a comment

Choose a reason for hiding this comment

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

HTPinotQueriesTest > testServicesQueriesForAvgRateWithTimeAggregation() FAILED
    org.opentest4j.AssertionFailedError: expected: <4> but was: <5>
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:527)
        at app//org.hypertrace.core.query.service.htqueries.HTPinotQueriesTest.testServicesQueriesForAvgRateWithTimeAggregation(HTPinotQueriesTest.java:396)

@saxenakshitiz
Copy link
Contributor Author

HTPinotQueriesTest > testServicesQueriesForAvgRateWithTimeAggregation() FAILED
    org.opentest4j.AssertionFailedError: expected: <4> but was: <5>
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:527)
        at app//org.hypertrace.core.query.service.htqueries.HTPinotQueriesTest.testServicesQueriesForAvgRateWithTimeAggregation(HTPinotQueriesTest.java:396)

None of the changes in PR impact pinot implementation in any way. Let me check this failure by running locally.

@github-actions
Copy link

Unit Test Results

  38 files  ±0    38 suites  ±0   9s ⏱️ ±0s
247 tests ±0  247 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 762a89a. ± Comparison against base commit 4bf632a.

@saxenakshitiz saxenakshitiz merged commit 7f4c943 into main Aug 11, 2022
@saxenakshitiz saxenakshitiz deleted the add_scalar_functions_postgres branch August 11, 2022 18:14
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.

2 participants