◐ Shell
reader mode source ↗
Skip to content

fix: Filter empty SQL commands at execute_snowflake_statement call sites#6249

Merged
franciscojavierarceo merged 4 commits into
feast-dev:masterfrom
jials:fix/empty-snowflake-query
Apr 21, 2026
Merged

fix: Filter empty SQL commands at execute_snowflake_statement call sites#6249
franciscojavierarceo merged 4 commits into
feast-dev:masterfrom
jials:fix/empty-snowflake-query

Conversation

@jials

@jials jials commented Apr 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

When SnowflakeRegistry splits .sql files by ;, trailing semicolons produce empty strings that get passed to execute_snowflake_statement. The PR moves that filtering to the call sites, stripping and discarding empty commands before execution.

Which issue(s) this PR fixes:

Fixes #5204 (partially)

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Misc

Open with Devin

@jials jials requested a review from a team as a code owner April 9, 2026 02:56
devin-ai-integration[bot]
devin-ai-integration Bot reviewed Apr 9, 2026
View reviewed changes

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@jials jials force-pushed the fix/empty-snowflake-query branch 2 times, most recently from 607c1da to aeff56c Compare April 10, 2026 00:40
@ntkathole ntkathole force-pushed the fix/empty-snowflake-query branch from aeff56c to f18f4ed Compare April 10, 2026 06:49
@jials

jials commented Apr 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

There are still other issues with the existing Snowflake Registry codebase. Will raise a followup PR to fix the rest of the issues.

@jials jials force-pushed the fix/empty-snowflake-query branch from a96b783 to bb67029 Compare April 10, 2026 07:36
@jials jials changed the title fix: Handle empty query in execute_snowflake_statement Apr 10, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

devin-ai-integration[bot]

This comment was marked as resolved.

Sign in to view

@jials jials requested a review from ntkathole April 10, 2026 09:40
@jials jials changed the title fix: filter empty SQL commands at execute_snowflake_statement call sites Apr 12, 2026
@jials

jials commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole can we get this reviewed and merged please? Thanks

@shuchu

shuchu commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator

please rebase with the Master branch.

jials added 4 commits April 20, 2026 20:41
@jials
fix: Handle empty query in execute_snowflake_statement …
0c004cf 
Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@jials
fix: Filter empty SQL commands at call sites instead of silently igno… …
4b14cbe 
…ring in execute_snowflake_statement

Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@jials
fix: Add missing semicolons in snowflake_table_deletion.sql …
dee3734 
Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@jials
fix: Add missing PROJECTS table drop to snowflake_table_deletion.sql …
7796a3b 
Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@jials jials force-pushed the fix/empty-snowflake-query branch from a0f3d21 to 7796a3b Compare April 20, 2026 12:41
@jials

jials commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

I don't think the failures are related to the changes in this PR. Can we re-trigger the failed actions please?

Hide details View details @franciscojavierarceo franciscojavierarceo merged commit 92ffbb9 into feast-dev:master Apr 21, 2026
41 of 45 checks passed
@jials jials deleted the fix/empty-snowflake-query branch April 22, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments

@franciscojavierarceo franciscojavierarceo franciscojavierarceo approved these changes

@ntkathole ntkathole Awaiting requested review from ntkathole

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Snowflake Registry does not work

4 participants

@jials @shuchu @franciscojavierarceo @ntkathole