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: Use ASOF JOIN in Snowflake offline store query #4850

Merged

Conversation

nanohanno
Copy link
Contributor

What this PR does / why we need it:

The query template for the Snowflake Offline Store is updated to use the recently added ASOF JOIN (https://www.snowflake.com/en/blog/time-series-analytics-asof-join-generally-available/). We have seen a very substantial improvement in feature queries using the new template. Especially, for large feature tables that have many historical entries for certain entity values the performance is even better than using the ttl parameter in the current query template.

Which issue(s) this PR fixes:

Fixes #4732

Misc

@bimtauer and @miianiemela have shown interest in the feature, are you able to take a look and test it in your setup?

@nanohanno nanohanno requested a review from a team as a code owner December 13, 2024 11:36
Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

Nice, thank you for this!

@franciscojavierarceo
Copy link
Member

Looks like integration tests are failing.

@nanohanno
Copy link
Contributor Author

Looks like integration tests are failing.

I did not realize that there are integation tests for Snowflake, that is great.
There was a bug in the template for features without entities that we did not check before. Hopefully the fix solves the failing test now.

@nanohanno nanohanno force-pushed the feature/snowflake_asof_join branch from 710d1de to 6b6c48e Compare December 19, 2024 14:13
@franciscojavierarceo
Copy link
Member

Looks like integration tests are failing.

I did not realize that there are integation tests for Snowflake, that is great. There was a bug in the template for features without entities that we did not check before. Hopefully the fix solves the failing test now.

Hmm. Looks like it's still failing fyi.

@nanohanno
Copy link
Contributor Author

Looks like integration tests are failing.

I did not realize that there are integation tests for Snowflake, that is great. There was a bug in the template for features without entities that we did not check before. Hopefully the fix solves the failing test now.

Hmm. Looks like it's still failing fyi.

Yeah, I did not consider setting up the Snowflake integration tests locally and debugging from the CI logs for the template query is a little painful. I have done some improvements today and am still positive that it is close to being there 🙏 . If not I will set up the Snowflake environment in the next days to see the errors in Snowsight.

Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@franciscojavierarceo franciscojavierarceo merged commit 8f591a2 into feast-dev:master Dec 23, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ASOF JOIN in Snowflake Source
2 participants