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

Snowflake add missing function definitions #433

Merged
merged 3 commits into from
Jun 7, 2024
Merged

Conversation

vil1
Copy link
Contributor

@vil1 vil1 commented Jun 5, 2024

Add entries in SnowflakeFunctionBuilder for functions that were used in the acceptance tests but not "defined" yet.

Note that the grammar around function_call has been reworked, this work is still in progress. Future work will include:

  • completing the definition of builtin_function (functions that have special syntax)
  • improving implementation around aggregate_function and ranking_windowed_function

@vil1 vil1 added the internal technical pr's not end user facing label Jun 5, 2024
@vil1 vil1 requested a review from a team as a code owner June 5, 2024 12:40
@vil1 vil1 requested a review from ravit-db June 5, 2024 12:40
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 97.35099% with 4 lines in your changes missing coverage. Please review.

Project coverage is 97.94%. Comparing base (60850b1) to head (2b3fc75).

Files Patch % Lines
...parsers/snowflake/SnowflakeExpressionBuilder.scala 70.00% 3 Missing ⚠️
...abricks/labs/remorph/parsers/FunctionBuilder.scala 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
- Coverage   97.97%   97.94%   -0.04%     
==========================================
  Files          61       61              
  Lines        3894     4031     +137     
  Branches      483      483              
==========================================
+ Hits         3815     3948     +133     
- Misses         45       49       +4     
  Partials       34       34              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Jun 5, 2024

Coverage tests results

354 tests  ±0   292 ✅ +41   4s ⏱️ -2s
  2 suites ±0     0 💤 ± 0 
  2 files   ±0    62 ❌  - 41 

For more details on these failures, see this check.

Results for commit 2b3fc75. ± Comparison against base commit 60850b1.

♻️ This comment has been updated with latest results.

@jimidle
Copy link
Contributor

jimidle commented Jun 5, 2024

I would like to merge #430 before we merge this so we don't start repeating the same edits. That PR finishes all functions except for the more exotic stuff like freeText. You can then use the TSQL grammar changes as a template without worrying I will change them half way through your own re-work on Snowflake.

@vil1
Copy link
Contributor Author

vil1 commented Jun 5, 2024

I would like to merge #430 before we merge this so we don't start repeating the same edits. That PR finishes all functions except for the more exotic stuff like freeText. You can then use the TSQL grammar changes as a template without worrying I will change them half way through your own re-work on Snowflake.

What edits do you refer to? the one in FunctionBuilder?

Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Lgtm

@nfx nfx merged commit ec3f0ba into main Jun 7, 2024
8 of 10 checks passed
@nfx nfx deleted the feature/snowflake-functions branch June 7, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal technical pr's not end user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants