Skip to content

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

Open
jials wants to merge 4 commits intofeast-dev:masterfrom
jials:fix/empty-snowflake-query
Open

fix: filter empty SQL commands at execute_snowflake_statement call sites#6249
jials wants to merge 4 commits intofeast-dev:masterfrom
jials:fix/empty-snowflake-query

Conversation

@jials
Copy link
Copy Markdown

@jials jials commented Apr 9, 2026

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
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

✅ 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
Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@ntkathole ntkathole force-pushed the fix/empty-snowflake-query branch from aeff56c to f18f4ed Compare April 10, 2026 06:49
@jials
Copy link
Copy Markdown
Author

jials commented Apr 10, 2026

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



def execute_snowflake_statement(conn: SnowflakeConnection, query) -> SnowflakeCursor:
if not query.strip():
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the real fix should be in the caller, silently returning a cursor for an empty query in a shared utility function masks potential bugs.

something like:

for command in sql_cmds:
    query = command.replace("REGISTRY_PATH", f"{self.registry_path}")
    if query.strip():
        execute_snowflake_statement(conn, query)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated!

…ring in execute_snowflake_statement

Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@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 fix: filter empty SQL commands at execute_snowflake_statement call sites Apr 10, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
@jials jials requested a review from ntkathole April 10, 2026 09:40
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.

Snowflake Registry does not work

2 participants