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 support for arrays in snowflake #3758

Closed
wants to merge 5 commits into from

Conversation

JohnLemmonMedely
Copy link
Contributor

What this PR does / why we need it:
Snowflake doesn't support type-checked arrays but support for arrays can still be added if developers can guarantee the type consistency themselves through the external data pipelines.

Which issue(s) this PR fixes:

Fixes #2280

@JohnLemmonMedely
Copy link
Contributor Author

@sfc-gh-madkins Here's my draft PR (tests and documentation still pending). I've confirmed it unblocks my specific use-case to support a List[String] and materializing it to a Redis Online store but I'll need to setup the tests to confirm the other paths in the code work.

I'd appreciate a sanity check on some of the choices I had to make before I invest more time in this.

  1. To cast the data correctly from pandas I just apply a json.loads(x) on the columns for any array type.
  2. I only added support for Arrays to limit scope--but I think the same setup can work for objects.
  3. In the function snowflake_type_to_feast_value_type I force array types to cast to Feast's String type. It looks like this function is used when inferring column types but that can't be done perfectly in this direction. So casting to a String seemed like the safest option.
  4. Do you know any place in the code I'm missing changes?

JohnLemmonMedely and others added 2 commits September 15, 2023 13:41
Signed-off-by: john.lemmon <john.lemmon@medely.com>
Co-authored-by: Sam Rausser <srausser@gmail.com>
Signed-off-by: john.lemmon <john.lemmon@medely.com>
phil-park and others added 3 commits September 15, 2023 14:56
)

* fix: Remove unused parameter

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>

* feat: Apply cache to load proto registry for performance

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>

* fix: Fix update key when cache missed

Signed-off-by: phil-park <bakjeeone@hotmail.com>

---------

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>
Signed-off-by: phil-park <bakjeeone@hotmail.com>
…st-dev#3761)

* resolve feast-dev#3760

Signed-off-by: snowron <snowronark@gmail.com>

* format feature_server.py

Signed-off-by: snowron <snowronark@gmail.com>

---------

Signed-off-by: snowron <snowronark@gmail.com>
…offline store (feast-dev#3762)

* Add bigquery table create disposition to offline store

Signed-off-by: Nick Zeolla <nick.zeolla@starlingbank.com>

* linting

Signed-off-by: Nick Zeolla <nick.zeolla@starlingbank.com>

---------

Signed-off-by: Nick Zeolla <nick.zeolla@starlingbank.com>
@JohnLemmonMedely
Copy link
Contributor Author

Closing in favor of #3769

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.

Snowflake support does not include list features
6 participants