Skip to content

Commit bb67029

Browse files
committed
fix: Filter empty SQL commands at call sites instead of silently ignoring in execute_snowflake_statement
Signed-off-by: Jia Le <5955220+jials@users.noreply.github.com>
1 parent f18f4ed commit bb67029

File tree

3 files changed

+12
-22
lines changed

3 files changed

+12
-22
lines changed

sdk/python/feast/infra/registry/snowflake.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,9 @@ def __init__(
139139
with GetSnowflakeConnection(self.registry_config) as conn:
140140
sql_function_file = f"{os.path.dirname(feast.__file__)}/infra/utils/snowflake/registry/snowflake_table_creation.sql"
141141
with open(sql_function_file, "r") as file:
142-
sql_file = file.read()
143-
sql_cmds = sql_file.split(";")
142+
sql_cmds = [
143+
cmd.strip() for cmd in file.read().split(";") if cmd.strip()
144+
]
144145
for command in sql_cmds:
145146
query = command.replace("REGISTRY_PATH", f"{self.registry_path}")
146147
execute_snowflake_statement(conn, query)
@@ -224,9 +225,10 @@ def teardown(self):
224225
with GetSnowflakeConnection(self.registry_config) as conn:
225226
sql_function_file = f"{os.path.dirname(feast.__file__)}/infra/utils/snowflake/registry/snowflake_table_deletion.sql"
226227
with open(sql_function_file, "r") as file:
227-
sqlFile = file.read()
228-
sqlCommands = sqlFile.split(";")
229-
for command in sqlCommands:
228+
sql_cmds = [
229+
cmd.strip() for cmd in file.read().split(";") if cmd.strip()
230+
]
231+
for command in sql_cmds:
230232
query = command.replace("REGISTRY_PATH", f"{self.registry_path}")
231233
execute_snowflake_statement(conn, query)
232234

sdk/python/feast/infra/utils/snowflake/snowflake_utils.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ def assert_snowflake_feature_names(feature_view: FeatureView) -> None:
123123

124124

125125
def execute_snowflake_statement(conn: SnowflakeConnection, query) -> SnowflakeCursor:
126-
if not query.strip():
127-
return conn.cursor()
128126
cursor = conn.cursor().execute(query)
129127
if cursor is None:
130128
raise SnowflakeQueryUnknownError(query)

sdk/python/tests/unit/infra/utils/snowflake/test_snowflake_utils.py

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,27 +76,17 @@ def test_parse_private_key_path_key_path_encrypted(encrypted_private_key):
7676

7777

7878
class TestExecuteSnowflakeStatement:
79-
def test_empty_query_returns_cursor_without_executing(self):
79+
def test_empty_query_is_passed_through_to_execute(self):
8080
mock_conn = MagicMock()
8181
mock_cursor = MagicMock()
82+
mock_executed_cursor = MagicMock()
8283
mock_conn.cursor.return_value = mock_cursor
84+
mock_cursor.execute.return_value = mock_executed_cursor
8385

8486
result = execute_snowflake_statement(mock_conn, "")
8587

86-
assert result is mock_cursor
87-
mock_conn.cursor.assert_called_once()
88-
mock_cursor.execute.assert_not_called()
89-
90-
def test_whitespace_only_query_returns_cursor_without_executing(self):
91-
mock_conn = MagicMock()
92-
mock_cursor = MagicMock()
93-
mock_conn.cursor.return_value = mock_cursor
94-
95-
result = execute_snowflake_statement(mock_conn, " \t\n ")
96-
97-
assert result is mock_cursor
98-
mock_conn.cursor.assert_called_once()
99-
mock_cursor.execute.assert_not_called()
88+
assert result is mock_executed_cursor
89+
mock_cursor.execute.assert_called_once_with("")
10090

10191
def test_valid_query_executes_and_returns_cursor(self):
10292
mock_conn = MagicMock()

0 commit comments

Comments
 (0)