From 79eb9266d4292111e1adcb2e2763ee16beceba1a Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Jul 2025 15:06:25 +0100 Subject: [PATCH 01/10] feat: Server-side parametrised queries --- src/firebolt/async_db/__init__.py | 9 +- src/firebolt/async_db/connection.py | 35 +- src/firebolt/async_db/cursor.py | 82 +++- src/firebolt/common/constants.py | 2 + src/firebolt/common/cursor/base_cursor.py | 27 ++ src/firebolt/db/__init__.py | 9 +- src/firebolt/db/connection.py | 32 +- src/firebolt/db/cursor.py | 82 +++- .../dbapi/async/V2/test_queries_async.py | 100 ++++ .../integration/dbapi/sync/V2/test_queries.py | 101 ++++ tests/unit/async_db/test_cursor.py | 431 ++++++++++++++++++ tests/unit/db/test_cursor.py | 313 ++++++++++++- tests/unit/db_conftest.py | 178 ++++++++ 13 files changed, 1357 insertions(+), 44 deletions(-) diff --git a/src/firebolt/async_db/__init__.py b/src/firebolt/async_db/__init__.py index 284119b5cbb..0ee41340e11 100644 --- a/src/firebolt/async_db/__init__.py +++ b/src/firebolt/async_db/__init__.py @@ -18,6 +18,7 @@ Timestamp, TimestampFromTicks, ) +from firebolt.common.constants import PARAMSTYLE_DEFAULT from firebolt.utils.exception import ( DatabaseError, DataError, @@ -34,4 +35,10 @@ apilevel = "2.0" # threads may only share the module and connections, cursors should not be shared threadsafety = 2 -paramstyle = "qmark" +paramstyle = PARAMSTYLE_DEFAULT +""" +The parameter style for SQL queries. Supported values: +- 'qmark': Use ? as parameter placeholders (default, client-side substitution) +- 'native': Alias for 'qmark' +- 'fb_numeric': Use $1, $2, ... as placeholders (server-side, sent as query_parameters) +""" diff --git a/src/firebolt/async_db/connection.py b/src/firebolt/async_db/connection.py index 202aa7530ea..45c3d2aeba4 100644 --- a/src/firebolt/async_db/connection.py +++ b/src/firebolt/async_db/connection.py @@ -22,7 +22,10 @@ _parse_async_query_info_results, ) from firebolt.common.cache import _firebolt_system_engine_cache -from firebolt.common.constants import DEFAULT_TIMEOUT_SECONDS +from firebolt.common.constants import ( + DEFAULT_TIMEOUT_SECONDS, + PARAMSTYLE_DEFAULT, +) from firebolt.utils.exception import ( ConfigurationError, ConnectionClosedError, @@ -71,6 +74,7 @@ class Connection(BaseConnection): "_is_closed", "client_class", "cursor_type", + "paramstyle", ) def __init__( @@ -81,6 +85,7 @@ def __init__( cursor_type: Type[Cursor], api_endpoint: str, init_parameters: Optional[Dict[str, Any]] = None, + paramstyle: Optional[str] = PARAMSTYLE_DEFAULT, ): super().__init__(cursor_type) self.api_endpoint = api_endpoint @@ -90,12 +95,17 @@ def __init__( self.init_parameters = init_parameters or {} if database: self.init_parameters["database"] = database + self.paramstyle = paramstyle def cursor(self, **kwargs: Any) -> Cursor: if self.closed: raise ConnectionClosedError("Unable to create cursor: connection closed.") - c = self.cursor_type(client=self._client, connection=self, **kwargs) + if self.paramstyle is None: + self.paramstyle = PARAMSTYLE_DEFAULT + c = self.cursor_type( + client=self._client, connection=self, paramstyle=self.paramstyle, **kwargs + ) self._cursors.append(c) return c @@ -218,7 +228,10 @@ async def connect( disable_cache: bool = False, url: Optional[str] = None, additional_parameters: Dict[str, Any] = {}, + paramstyle: Optional[str] = None, ) -> Connection: + if paramstyle is None: + paramstyle = PARAMSTYLE_DEFAULT # auth parameter is optional in function signature # but is required to connect. # PEP 249 recommends making it kwargs. @@ -244,6 +257,7 @@ async def connect( user_agent_header=user_agent_header, database=database, connection_url=url, + paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V2: assert account_name is not None @@ -254,6 +268,7 @@ async def connect( database=database, engine_name=engine_name, api_endpoint=api_endpoint, + paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V1: return await connect_v1( @@ -264,6 +279,7 @@ async def connect( engine_name=engine_name, engine_url=engine_url, api_endpoint=api_endpoint, + paramstyle=paramstyle, ) else: raise ConfigurationError(f"Unsupported auth type: {type(auth)}") @@ -276,6 +292,7 @@ async def connect_v2( database: Optional[str] = None, engine_name: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, + paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt. @@ -322,6 +339,7 @@ async def connect_v2( CursorV2, api_endpoint, system_engine_params, + paramstyle=paramstyle, ) as system_engine_connection: cursor = system_engine_connection.cursor() @@ -338,6 +356,7 @@ async def connect_v2( CursorV2, api_endpoint, cursor.parameters, + paramstyle=paramstyle, ) @@ -349,6 +368,7 @@ async def connect_v1( engine_name: Optional[str] = None, engine_url: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, + paramstyle: Optional[str] = None, ) -> Connection: # These parameters are optional in function signature # but are required to connect. @@ -396,7 +416,14 @@ async def connect_v1( timeout=Timeout(DEFAULT_TIMEOUT_SECONDS, read=None), headers={"User-Agent": user_agent_header}, ) - return Connection(engine_url, database, client, CursorV1, api_endpoint) + return Connection( + engine_url, + database, + client, + CursorV1, + api_endpoint, + paramstyle=paramstyle or PARAMSTYLE_DEFAULT, + ) def connect_core( @@ -404,6 +431,7 @@ def connect_core( user_agent_header: str, database: Optional[str] = None, connection_url: Optional[str] = None, + paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt Core. @@ -441,4 +469,5 @@ def connect_core( client=client, cursor_type=CursorV2, api_endpoint=verified_url, + paramstyle=paramstyle, ) diff --git a/src/firebolt/async_db/cursor.py b/src/firebolt/async_db/cursor.py index bc4fe2f92ff..32277e1404b 100644 --- a/src/firebolt/async_db/cursor.py +++ b/src/firebolt/async_db/cursor.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging import time import warnings @@ -21,6 +22,8 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, + PARAMSTYLE_DEFAULT, + PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, UPDATE_PARAMETERS_HEADER, @@ -28,6 +31,7 @@ ) from firebolt.common.cursor.base_cursor import ( BaseCursor, + _convert_parameter_value, _parse_update_endpoint, _parse_update_parameters, _raise_if_internal_set_parameter, @@ -80,10 +84,12 @@ def __init__( self, *args: Any, client: AsyncClient, - connection: Connection, + connection: "Connection", + paramstyle: str = PARAMSTYLE_DEFAULT, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) + self.paramstyle = paramstyle self._client = client self.connection = connection self.engine_url = connection.engine_url @@ -216,27 +222,69 @@ async def _do_execute( ) -> None: await self._close_rowset_and_reset() self._row_set = StreamingAsyncRowSet() if streaming else InMemoryAsyncRowSet() - queries: List[Union[SetParameter, str]] = ( - [raw_query] - if skip_parsing - else self._formatter.split_format_sql(raw_query, parameters) - ) - timeout_controller = TimeoutController(timeout) - - if len(queries) > 1 and async_execution: - raise FireboltError( - "Server side async does not support multi-statement queries" - ) + paramstyle = self.paramstyle or PARAMSTYLE_DEFAULT + if paramstyle not in PARAMSTYLE_SUPPORTED: + raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: - for query in queries: - await self._execute_single_query( - query, timeout_controller, async_execution, streaming + if paramstyle == "fb_numeric": + await self._execute_fb_numeric( + raw_query, parameters, timeout, async_execution, streaming + ) + else: + queries: List[Union[SetParameter, str]] = ( + [raw_query] + if skip_parsing + else self._formatter.split_format_sql(raw_query, parameters) ) + timeout_controller = TimeoutController(timeout) + if len(queries) > 1 and async_execution: + raise FireboltError( + "Server side async does not support multi-statement queries" + ) + for query in queries: + await self._execute_single_query( + query, timeout_controller, async_execution, streaming + ) self._state = CursorState.DONE except Exception: self._state = CursorState.ERROR raise + async def _execute_fb_numeric( + self, + query: str, + parameters: Sequence[Sequence[ParameterType]], + timeout: Optional[float], + async_execution: bool, + streaming: bool, + ) -> None: + Cursor._log_query(query) + timeout_controller = TimeoutController(timeout) + timeout_controller.raise_if_timeout() + param_list = parameters[0] if parameters else [] + query_parameters = [ + {"name": f"${i+1}", "value": _convert_parameter_value(value)} + for i, value in enumerate(param_list) + ] + query_params: Dict[str, Any] = { + "output_format": self._get_output_format(streaming), + "query_parameters": json.dumps(query_parameters), + } + if async_execution: + query_params["async"] = True + resp = await self._api_request( + query, + query_params, + timeout=timeout_controller.remaining(), + ) + await self._raise_if_error(resp) + if async_execution: + await resp.aread() + self._parse_async_response(resp) + else: + await self._parse_response_headers(resp.headers) + await self._append_row_set_from_response(resp) + async def _execute_single_query( self, query: Union[SetParameter, str], @@ -522,7 +570,7 @@ def __init__( self, *args: Any, client: AsyncClientV2, - connection: Connection, + connection: "Connection", **kwargs: Any, ) -> None: assert isinstance(client, AsyncClientV2) @@ -605,7 +653,7 @@ def __init__( self, *args: Any, client: AsyncClientV1, - connection: Connection, + connection: "Connection", **kwargs: Any, ) -> None: assert isinstance(client, AsyncClientV1) diff --git a/src/firebolt/common/constants.py b/src/firebolt/common/constants.py index a48c8651b21..2e98d4370fe 100644 --- a/src/firebolt/common/constants.py +++ b/src/firebolt/common/constants.py @@ -28,3 +28,5 @@ class CursorState(Enum): UPDATE_ENDPOINT_HEADER = "Firebolt-Update-Endpoint" UPDATE_PARAMETERS_HEADER = "Firebolt-Update-Parameters" RESET_SESSION_HEADER = "Firebolt-Reset-Session" +PARAMSTYLE_DEFAULT = "qmark" +PARAMSTYLE_SUPPORTED = ["qmark", "native", "fb_numeric"] diff --git a/src/firebolt/common/cursor/base_cursor.py b/src/firebolt/common/cursor/base_cursor.py index b46b1de5395..d5eacd71739 100644 --- a/src/firebolt/common/cursor/base_cursor.py +++ b/src/firebolt/common/cursor/base_cursor.py @@ -2,6 +2,7 @@ import logging import re +from decimal import Decimal from types import TracebackType from typing import Any, Dict, List, Optional, Tuple, Type, Union @@ -26,6 +27,32 @@ logger = logging.getLogger(__name__) +def _convert_parameter_value(value: Any) -> Any: + """ + Convert parameter values for fb_numeric paramstyle to JSON-serializable format. + + This ensures consistent behavior between sync and async cursors. + Basic types (int, float, bool, None) are preserved as-is. + All other types are converted to strings for JSON serialization. + + Args: + value: The parameter value to convert + + Returns: + JSON-serializable value (int, float, bool, None, or string) + """ + if isinstance(value, (int, float, bool)) or value is None: + return value + + # Handle special cases for better string representation + if isinstance(value, Decimal): + return str(value) + elif isinstance(value, bytes): + return repr(value) # This gives us the b'...' format + else: + return str(value) + + def _parse_update_parameters(parameter_header: str) -> Dict[str, str]: """Parse update parameters and set them as attributes.""" # parse key1=value1,key2=value2 comma separated string into dict diff --git a/src/firebolt/db/__init__.py b/src/firebolt/db/__init__.py index e95c9a9f863..78a452c29cf 100644 --- a/src/firebolt/db/__init__.py +++ b/src/firebolt/db/__init__.py @@ -16,6 +16,7 @@ Timestamp, TimestampFromTicks, ) +from firebolt.common.constants import PARAMSTYLE_DEFAULT from firebolt.db.connection import Connection, connect from firebolt.db.cursor import Cursor from firebolt.utils.exception import ( @@ -34,4 +35,10 @@ apilevel = "2.0" # threads may only share the module and connections, cursors should not be shared threadsafety = 2 -paramstyle = "qmark" +paramstyle = PARAMSTYLE_DEFAULT +""" +The parameter style for SQL queries. Supported values: +- 'qmark': Use ? as parameter placeholders (default, client-side substitution) +- 'native': Alias for 'qmark' +- 'fb_numeric': Use $1, $2, ... as placeholders (server-side, sent as query_parameters) +""" diff --git a/src/firebolt/db/connection.py b/src/firebolt/db/connection.py index ef7b5ff62db..8c13e9fdc9c 100644 --- a/src/firebolt/db/connection.py +++ b/src/firebolt/db/connection.py @@ -21,7 +21,10 @@ _parse_async_query_info_results, ) from firebolt.common.cache import _firebolt_system_engine_cache -from firebolt.common.constants import DEFAULT_TIMEOUT_SECONDS +from firebolt.common.constants import ( + DEFAULT_TIMEOUT_SECONDS, + PARAMSTYLE_DEFAULT, +) from firebolt.db.cursor import Cursor, CursorV1, CursorV2 from firebolt.db.util import _get_system_engine_url_and_params from firebolt.utils.exception import ( @@ -50,6 +53,7 @@ def connect( disable_cache: bool = False, url: Optional[str] = None, additional_parameters: Dict[str, Any] = {}, + paramstyle: Optional[str] = None, ) -> Connection: # auth parameter is optional in function signature # but is required to connect. @@ -57,6 +61,9 @@ def connect( if not auth: raise ConfigurationError("auth is required to connect.") + if paramstyle is None: + paramstyle = PARAMSTYLE_DEFAULT + # Type checks assert auth is not None user_drivers = additional_parameters.get("user_drivers", []) @@ -77,6 +84,7 @@ def connect( user_agent_header=user_agent_header, database=database, connection_url=url, + paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V2: assert account_name is not None @@ -87,6 +95,7 @@ def connect( database=database, engine_name=engine_name, api_endpoint=api_endpoint, + paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V1: return connect_v1( @@ -97,6 +106,7 @@ def connect( engine_name=engine_name, engine_url=engine_url, api_endpoint=api_endpoint, + paramstyle=paramstyle, ) else: raise ConfigurationError(f"Unsupported auth type: {type(auth)}") @@ -109,6 +119,7 @@ def connect_v2( database: Optional[str] = None, engine_name: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, + paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt. @@ -155,6 +166,7 @@ def connect_v2( CursorV2, api_endpoint, system_engine_params, + paramstyle=paramstyle, ) as system_engine_connection: cursor = system_engine_connection.cursor() @@ -171,6 +183,7 @@ def connect_v2( CursorV2, api_endpoint, cursor.parameters, + paramstyle=paramstyle, ) @@ -201,6 +214,7 @@ class Connection(BaseConnection): "_is_closed", "client_class", "cursor_type", + "paramstyle", ) def __init__( @@ -211,6 +225,7 @@ def __init__( cursor_type: Type[Cursor], api_endpoint: str = DEFAULT_API_URL, init_parameters: Optional[Dict[str, Any]] = None, + paramstyle: Optional[str] = PARAMSTYLE_DEFAULT, ): super().__init__(cursor_type) self.api_endpoint = api_endpoint @@ -220,12 +235,16 @@ def __init__( self.init_parameters = init_parameters or {} if database: self.init_parameters["database"] = database + self.paramstyle = paramstyle def cursor(self, **kwargs: Any) -> Cursor: if self.closed: raise ConnectionClosedError("Unable to create cursor: connection closed.") - - c = self.cursor_type(client=self._client, connection=self, **kwargs) + if self.paramstyle is None: + self.paramstyle = PARAMSTYLE_DEFAULT + c = self.cursor_type( + client=self._client, connection=self, paramstyle=self.paramstyle, **kwargs + ) self._cursors.append(c) return c @@ -354,6 +373,7 @@ def connect_v1( engine_name: Optional[str] = None, engine_url: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, + paramstyle: Optional[str] = None, ) -> Connection: # These parameters are optional in function signature # but are required to connect. @@ -402,7 +422,9 @@ def connect_v1( timeout=Timeout(DEFAULT_TIMEOUT_SECONDS, read=None), headers={"User-Agent": user_agent_header}, ) - return Connection(engine_url, database, client, CursorV1, api_endpoint) + return Connection( + engine_url, database, client, CursorV1, api_endpoint, paramstyle=paramstyle + ) def connect_core( @@ -410,6 +432,7 @@ def connect_core( user_agent_header: str, database: Optional[str] = None, connection_url: Optional[str] = None, + paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt Core. @@ -448,4 +471,5 @@ def connect_core( client=client, cursor_type=CursorV2, api_endpoint=verified_url, + paramstyle=paramstyle, ) diff --git a/src/firebolt/db/cursor.py b/src/firebolt/db/cursor.py index 7af8cc562ea..85b37d0ea6f 100644 --- a/src/firebolt/db/cursor.py +++ b/src/firebolt/db/cursor.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging import time from abc import ABCMeta, abstractmethod @@ -29,6 +30,8 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, + PARAMSTYLE_DEFAULT, + PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, UPDATE_PARAMETERS_HEADER, @@ -36,6 +39,7 @@ ) from firebolt.common.cursor.base_cursor import ( BaseCursor, + _convert_parameter_value, _parse_update_endpoint, _parse_update_parameters, _raise_if_internal_set_parameter, @@ -86,10 +90,12 @@ def __init__( self, *args: Any, client: Client, - connection: Connection, + connection: "Connection", + paramstyle: str = PARAMSTYLE_DEFAULT, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) + self.paramstyle = paramstyle self._client = client self.connection = connection self.engine_url = connection.engine_url @@ -222,27 +228,69 @@ def _do_execute( ) -> None: self._close_rowset_and_reset() self._row_set = StreamingRowSet() if streaming else InMemoryRowSet() - queries: List[Union[SetParameter, str]] = ( - [raw_query] - if skip_parsing - else self._formatter.split_format_sql(raw_query, parameters) - ) - timeout_controller = TimeoutController(timeout) - - if len(queries) > 1 and async_execution: - raise FireboltError( - "Server side async does not support multi-statement queries" - ) + paramstyle = self.paramstyle or PARAMSTYLE_DEFAULT + if paramstyle not in PARAMSTYLE_SUPPORTED: + raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: - for query in queries: - self._execute_single_query( - query, timeout_controller, async_execution, streaming + if paramstyle == "fb_numeric": + self._execute_fb_numeric( + raw_query, parameters, timeout, async_execution, streaming + ) + else: + queries: List[Union[SetParameter, str]] = ( + [raw_query] + if skip_parsing + else self._formatter.split_format_sql(raw_query, parameters) ) + timeout_controller = TimeoutController(timeout) + if len(queries) > 1 and async_execution: + raise FireboltError( + "Server side async does not support multi-statement queries" + ) + for query in queries: + self._execute_single_query( + query, timeout_controller, async_execution, streaming + ) self._state = CursorState.DONE except Exception: self._state = CursorState.ERROR raise + def _execute_fb_numeric( + self, + query: str, + parameters: Sequence[Sequence[ParameterType]], + timeout: Optional[float], + async_execution: bool, + streaming: bool, + ) -> None: + Cursor._log_query(query) + timeout_controller = TimeoutController(timeout) + timeout_controller.raise_if_timeout() + param_list = parameters[0] if parameters else [] + query_parameters = [ + {"name": f"${i+1}", "value": _convert_parameter_value(value)} + for i, value in enumerate(param_list) + ] + query_params: Dict[str, Any] = { + "output_format": self._get_output_format(streaming), + "query_parameters": json.dumps(query_parameters), + } + if async_execution: + query_params["async"] = True + resp = self._api_request( + query, + query_params, + timeout=timeout_controller.remaining(), + ) + self._raise_if_error(resp) + if async_execution: + resp.read() + self._parse_async_response(resp) + else: + self._parse_response_headers(resp.headers) + self._append_row_set_from_response(resp) + def _execute_single_query( self, query: Union[SetParameter, str], @@ -482,7 +530,7 @@ def __enter__(self) -> Cursor: class CursorV2(Cursor): def __init__( - self, *args: Any, client: Client, connection: Connection, **kwargs: Any + self, *args: Any, client: Client, connection: "Connection", **kwargs: Any ) -> None: assert isinstance(client, ClientV2) # Type check super().__init__( @@ -562,7 +610,7 @@ def execute_async( class CursorV1(Cursor): def __init__( - self, *args: Any, client: ClientV1, connection: Connection, **kwargs: Any + self, *args: Any, client: ClientV1, connection: "Connection", **kwargs: Any ) -> None: assert isinstance(client, ClientV1) # Type check super().__init__( diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index c1f339a5592..ab821fd689b 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -479,3 +479,103 @@ async def test_select_struct( ) finally: await c.execute(cleanup_struct_query) + + +async def test_fb_numeric_paramstyle_all_types( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ] + await c.execute( + 'INSERT INTO "test_fb_numeric_all_types" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', + params, + ) + await c.execute( + 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] + ) + result = await c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal + assert result == [params] + + +async def test_fb_numeric_paramstyle_not_enough_params( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: not enough parameters supplied.""" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' + ) + # Only one param for two placeholders + try: + await c.execute( + 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() + ) + else: + assert False, "Expected error for not enough parameters" + + +async def test_fb_numeric_paramstyle_too_many_params( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + await c.execute( + 'INSERT INTO "test_fb_numeric_params2" VALUES ($1, $2)', [1, "foo", 123] + ) + await c.execute('SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1]) + result = await c.fetchall() + assert result == [[1, "foo"]] diff --git a/tests/integration/dbapi/sync/V2/test_queries.py b/tests/integration/dbapi/sync/V2/test_queries.py index 25e72c66719..c85bb86557c 100644 --- a/tests/integration/dbapi/sync/V2/test_queries.py +++ b/tests/integration/dbapi/sync/V2/test_queries.py @@ -561,3 +561,104 @@ def test_select_struct( ) finally: c.execute(cleanup_struct_query) + + +def test_fb_numeric_paramstyle_all_types( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ] + c.execute( + 'INSERT INTO "test_fb_numeric_all_types_sync" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', + params, + ) + c.execute( + 'SELECT * FROM "test_fb_numeric_all_types_sync" WHERE i = $1', [1] + ) + result = c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal + assert result == [params] + + +def test_fb_numeric_paramstyle_not_enough_params( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: not enough parameters supplied.""" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' + ) + # Only one param for two placeholders + try: + c.execute( + 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() + ) + else: + assert False, "Expected error for not enough parameters" + + +def test_fb_numeric_paramstyle_too_many_params( + engine_name, database_name, auth, account_name, api_endpoint +): + """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + paramstyle="fb_numeric", + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + c.execute( + 'INSERT INTO "test_fb_numeric_params2_sync" VALUES ($1, $2)', + [1, "foo", 123], + ) + c.execute('SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1]) + result = c.fetchall() + assert result == [[1, "foo"]] diff --git a/tests/unit/async_db/test_cursor.py b/tests/unit/async_db/test_cursor.py index c725b50f796..e1553d3e4da 100644 --- a/tests/unit/async_db/test_cursor.py +++ b/tests/unit/async_db/test_cursor.py @@ -1,4 +1,7 @@ +import re import time +from datetime import date, datetime +from decimal import Decimal from typing import Any, Callable, Dict, List from unittest.mock import patch @@ -953,3 +956,431 @@ def http_error(*args, **kwargs): # Error is raised during streaming with raises(FireboltStructuredError): await method() + + +@mark.parametrize( + "test_params,expected_query_params", + [ + # Basic types + ( + [42, "string", 3.14, True, False, None], + [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "string"}, + {"name": "$3", "value": 3.14}, + {"name": "$4", "value": True}, + {"name": "$5", "value": False}, + {"name": "$6", "value": None}, + ], + ), + # Edge cases for numeric types + ( + [0, -1, 0.0, -3.14], + [ + {"name": "$1", "value": 0}, + {"name": "$2", "value": -1}, + {"name": "$3", "value": 0.0}, + {"name": "$4", "value": -3.14}, + ], + ), + # String edge cases + ( + ["", "multi\nline", "special'chars\"test"], + [ + {"name": "$1", "value": ""}, + {"name": "$2", "value": "multi\nline"}, + {"name": "$3", "value": "special'chars\"test"}, + ], + ), + # Single parameter + ([42], [{"name": "$1", "value": 42}]), + # Empty parameters + ([], []), + ], +) +async def test_fb_numeric_parameter_formatting( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, + test_params: List[Any], + expected_query_params: List[Dict[str, Any]], +): + """Test that fb_numeric paramstyle formats parameters correctly for various types.""" + test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_complex_types_converted_to_strings_async( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle converts complex types to strings in async mode (same as sync).""" + test_params = [datetime(2023, 1, 1, 12, 30), date(2023, 6, 15)] + expected_query_params = [ + {"name": "$1", "value": "2023-01-01 12:30:00"}, + {"name": "$2", "value": "2023-06-15"}, + ] + + test_query = "SELECT * FROM test WHERE created_at = $1 AND birth_date = $2" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_no_client_side_substitution( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: URL, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle does not perform client-side parameter substitution.""" + test_query = "SELECT * FROM test WHERE id = $1 AND name = $2 AND value = $1" + test_params = [42, "test"] + expected_query_params = [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "test"}, + ] + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_executemany( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: URL, + fb_numeric_simple_callback: Callable, +): + """Test that fb_numeric paramstyle works with executemany method.""" + test_query = "INSERT INTO test (id, name) VALUES ($1, $2)" + + # For executemany, only the first parameter set is used with fb_numeric + test_params_seq = [ + [1, "first"], + [2, "second"], + [3, "third"], + ] + + async def validate_executemany_callback(request: Request, **kwargs): + assert request.method == "POST" + + # Should process multiple parameter sets sequentially + import json as json_mod + from urllib.parse import parse_qs + + qs = parse_qs(request.url.query) + query_params_raw = qs.get(b"query_parameters", []) + + if query_params_raw: + query_params_str = query_params_raw[0].decode() + actual_query_params = json_mod.loads(query_params_str) + # Should have parameters from one of the parameter sets + assert len(actual_query_params) == 2 + assert actual_query_params[0]["name"] == "$1" + assert actual_query_params[1]["name"] == "$2" + + return fb_numeric_simple_callback(request, **kwargs) + + httpx_mock.add_callback(validate_executemany_callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.executemany(test_query, test_params_seq) + + +async def test_fb_numeric_with_set_parameters( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: URL, + fb_numeric_simple_callback: Callable, +): + """Test that fb_numeric paramstyle works correctly when cursor has pre-existing set parameters.""" + cursor.paramstyle = "fb_numeric" + + # Manually set a parameter in the cursor (simulating what would happen + # if SET was called in a different paramstyle mode) + cursor._set_parameters = {"my_param": "test_value"} + + test_query = "SELECT * FROM test WHERE id = $1" + test_params = [42] + + async def validate_with_set_params_callback(request: Request, **kwargs): + assert request.method == "POST" + + # Should include both set parameters and query parameters + from urllib.parse import parse_qs + + qs = parse_qs(request.url.query) + + # Check for set parameter + assert b"my_param" in qs + assert qs[b"my_param"] == [b"test_value"] + + # Check for query parameters + query_params_raw = qs.get(b"query_parameters", []) + if query_params_raw: + import json as json_mod + + query_params_str = query_params_raw[0].decode() + actual_query_params = json_mod.loads(query_params_str) + expected = [{"name": "$1", "value": 42}] + assert actual_query_params == expected + + return fb_numeric_simple_callback(request, **kwargs) + + # Mock the SELECT query with set parameters + httpx_mock.add_callback(validate_with_set_params_callback, url=fb_numeric_query_url) + + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_execute_async( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: URL, + fb_numeric_async_callback: Callable, + async_token: str, +): + """Test that fb_numeric paramstyle works with execute_async method.""" + test_query = "SELECT * FROM test WHERE id = $1 AND name = $2" + test_params = [42, "async_test"] + + async def validate_async_callback(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + # Should include async=True parameter + from urllib.parse import parse_qs + + qs = parse_qs(request.url.query) + async_param = qs.get(b"async", []) + assert async_param == [b"true"], f"Expected async=true, got: {async_param}" + + # Should include query parameters + query_params_raw = qs.get(b"query_parameters", []) + if query_params_raw: + import json as json_mod + + query_params_str = query_params_raw[0].decode() + actual_query_params = json_mod.loads(query_params_str) + expected = [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "async_test"}, + ] + assert actual_query_params == expected + + return fb_numeric_async_callback(request, **kwargs) + + httpx_mock.add_callback(validate_async_callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + result = await cursor.execute_async(test_query, test_params) + + assert result == -1 # execute_async always returns -1 + assert cursor.async_query_token == async_token + + +async def test_fb_numeric_execute_stream( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: URL, + streaming_query_callback: Callable, +): + """Test that fb_numeric paramstyle works with execute_stream method.""" + test_query = "SELECT * FROM large_table WHERE id = $1" + test_params = [42] + + async def validate_streaming_callback(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + # Should include streaming output format + from urllib.parse import parse_qs + + qs = parse_qs(request.url.query) + output_format = qs.get(b"output_format", []) + assert output_format == [ + b"JSONLines_Compact" + ], f"Expected JSONLines_Compact output format, got: {output_format}" + + # Should include query parameters + query_params_raw = qs.get(b"query_parameters", []) + if query_params_raw: + import json as json_mod + + query_params_str = query_params_raw[0].decode() + actual_query_params = json_mod.loads(query_params_str) + expected = [{"name": "$1", "value": 42}] + assert actual_query_params == expected + + return streaming_query_callback(request, **kwargs) + + # Mock all fb_numeric URL patterns (the regex pattern covers all variations) + httpx_mock.add_callback(validate_streaming_callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute_stream(test_query, test_params) + + +@mark.parametrize( + "test_params,expected_query_params", + [ + # Decimal types - now converted to strings consistently + ( + [Decimal("123.45"), Decimal("0"), Decimal("-999.999")], + [ + {"name": "$1", "value": "123.45"}, + {"name": "$2", "value": "0"}, + {"name": "$3", "value": "-999.999"}, + ], + ), + # Bytes values - now converted to strings consistently + ( + [b"hello", b"\x00\x01\x02", b""], + [ + {"name": "$1", "value": "b'hello'"}, + {"name": "$2", "value": "b'\\x00\\x01\\x02'"}, + {"name": "$3", "value": "b''"}, + ], + ), + # List/Array values - now converted to strings consistently + ( + [[1, 2, 3], ["a", "b"], [], [None, True, False]], + [ + {"name": "$1", "value": "[1, 2, 3]"}, + {"name": "$2", "value": "['a', 'b']"}, + {"name": "$3", "value": "[]"}, + {"name": "$4", "value": "[None, True, False]"}, + ], + ), + # Mixed complex types - now converted to strings consistently + ( + [Decimal("42.0"), b"binary", [1, "mixed"], {"key": "value"}], + [ + {"name": "$1", "value": "42.0"}, + {"name": "$2", "value": "b'binary'"}, + {"name": "$3", "value": "[1, 'mixed']"}, + {"name": "$4", "value": "{'key': 'value'}"}, + ], + ), + ], +) +async def test_fb_numeric_additional_types_unified_behavior( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, + test_params: List[Any], + expected_query_params: List[Dict[str, Any]], +): + """Test that fb_numeric paramstyle handles additional types consistently in async mode (same as sync).""" + test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_mixed_basic_and_complex_types( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle works with mixed basic and complex types in async mode.""" + # Mix of basic types (preserved) and complex types (converted to strings) + test_params = [ + [1, 2, 3], # List -> converted to string + 42, # Basic int -> preserved + {"key": "value", "number": 42}, # Dict -> converted to string + None, # None -> preserved + ] + expected_query_params = [ + {"name": "$1", "value": "[1, 2, 3]"}, # Converted to string + {"name": "$2", "value": 42}, # Preserved as int + { + "name": "$3", + "value": "{'key': 'value', 'number': 42}", + }, # Converted to string + {"name": "$4", "value": None}, # Preserved as None + ] + + test_query = ( + "SELECT * FROM test WHERE data = $1 AND id = $2 AND meta = $3 AND null_val = $4" + ) + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_large_parameter_count_async( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle handles a large number of parameters in async mode.""" + # Test with 50 parameters + test_params = list(range(50)) + expected_query_params = [{"name": f"${i+1}", "value": i} for i in range(50)] + + placeholders = ", ".join(f"${i+1}" for i in range(50)) + test_query = f"SELECT * FROM test WHERE id IN ({placeholders})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_fb_numeric_special_float_values_async( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle handles special float values in async mode.""" + test_params = [ + float("inf"), + float("-inf"), + 2**63 - 1, # Large integer + -(2**63), # Very negative integer + ] + expected_query_params = [ + {"name": "$1", "value": float("inf")}, # JSON can handle Infinity + {"name": "$2", "value": float("-inf")}, # JSON can handle -Infinity + {"name": "$3", "value": 9223372036854775807}, + {"name": "$4", "value": -9223372036854775808}, + ] + + test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + + +async def test_unsupported_paramstyle_raises(cursor: Cursor) -> None: + """Test that unsupported paramstyles raise ProgrammingError.""" + cursor.paramstyle = "not_a_style" + with raises(ProgrammingError): + await cursor.execute("SELECT 1") diff --git a/tests/unit/db/test_cursor.py b/tests/unit/db/test_cursor.py index c9b1d7697b7..aed5800ba0d 100644 --- a/tests/unit/db/test_cursor.py +++ b/tests/unit/db/test_cursor.py @@ -1,6 +1,11 @@ +import json +import re import time +from datetime import date, datetime +from decimal import Decimal from typing import Any, Callable, Dict, List from unittest.mock import patch +from urllib.parse import parse_qs from httpx import URL, HTTPStatusError, Request, StreamError, codes from pytest import LogCaptureFixture, mark, raises @@ -9,7 +14,7 @@ from firebolt.common.constants import CursorState from firebolt.common.row_set.types import Column from firebolt.db import Cursor -from firebolt.db.cursor import ColType +from firebolt.db.cursor import ColType, ProgrammingError from firebolt.utils.exception import ( ConfigurationError, CursorClosedError, @@ -938,3 +943,309 @@ def http_error(*args, **kwargs): # Error is raised during streaming with raises(FireboltStructuredError): method() + + +@mark.parametrize( + "test_params,expected_query_params", + [ + # Basic types + ( + [42, "string", 3.14, True, False, None], + [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "string"}, + {"name": "$3", "value": 3.14}, + {"name": "$4", "value": True}, + {"name": "$5", "value": False}, + {"name": "$6", "value": None}, + ], + ), + # Edge cases for numeric types + ( + [0, -1, 0.0, -3.14], + [ + {"name": "$1", "value": 0}, + {"name": "$2", "value": -1}, + {"name": "$3", "value": 0.0}, + {"name": "$4", "value": -3.14}, + ], + ), + # String edge cases + ( + ["", "multi\nline", "special'chars\"test"], + [ + {"name": "$1", "value": ""}, + {"name": "$2", "value": "multi\nline"}, + {"name": "$3", "value": "special'chars\"test"}, + ], + ), + # Single parameter + ([42], [{"name": "$1", "value": 42}]), + # Empty parameters + ([], []), + ], +) +def test_fb_numeric_parameter_formatting( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, + test_params: List[Any], + expected_query_params: List[Dict[str, Any]], +): + """Test that fb_numeric paramstyle formats parameters correctly for various types.""" + test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_fb_numeric_complex_types_converted_to_strings( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle converts complex types to strings in sync mode.""" + test_params = [datetime(2023, 1, 1, 12, 30), date(2023, 6, 15)] + expected_query_params = [ + {"name": "$1", "value": "2023-01-01 12:30:00"}, + {"name": "$2", "value": "2023-06-15"}, + ] + + test_query = "SELECT * FROM test WHERE created_at = $1 AND birth_date = $2" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_fb_numeric_no_client_side_substitution( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle does not perform client-side parameter substitution.""" + test_query = "SELECT * FROM test WHERE id = $1 AND name = $2 AND value = $1" + test_params = [42, "test"] + expected_query_params = [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "test"}, + ] + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_fb_numeric_executemany( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_simple_callback: Callable, +): + """Test that fb_numeric paramstyle works with executemany method.""" + test_query = "INSERT INTO test (id, name) VALUES ($1, $2)" + + # For executemany, only the first parameter set is used with fb_numeric + test_params_seq = [ + [1, "first"], + [2, "second"], + [3, "third"], + ] + + def validate_executemany_callback(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + # Should process multiple parameter sets sequentially + qs = parse_qs(request.url.query) + query_params_raw = qs.get(b"query_parameters", []) + + if query_params_raw: + query_params_str = query_params_raw[0].decode() + actual_query_params = json.loads(query_params_str) + # Should have parameters from one of the parameter sets + assert len(actual_query_params) == 2 + assert actual_query_params[0]["name"] == "$1" + assert actual_query_params[1]["name"] == "$2" + + return fb_numeric_simple_callback(request, **kwargs) + + httpx_mock.add_callback(validate_executemany_callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.executemany(test_query, test_params_seq) + + +def test_fb_numeric_with_cursor_set_parameters( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_simple_callback: Callable, +): + """Test that fb_numeric paramstyle works correctly when cursor has pre-existing set parameters.""" + cursor.paramstyle = "fb_numeric" + + # Manually set a parameter in the cursor (simulating what would happen + # if SET was called in a different paramstyle mode) + cursor._set_parameters = {"my_param": "test_value"} + + test_query = "SELECT * FROM test WHERE id = $1" + test_params = [42] + + def validate_with_set_params_callback(request: Request, **kwargs): + assert request.method == "POST" + + # Should include both set parameters and query parameters + qs = parse_qs(request.url.query) + + # Check for set parameter + assert b"my_param" in qs + assert qs[b"my_param"] == [b"test_value"] + + # Check for query parameters + query_params_raw = qs.get(b"query_parameters", []) + if query_params_raw: + query_params_str = query_params_raw[0].decode() + actual_query_params = json.loads(query_params_str) + expected = [{"name": "$1", "value": 42}] + assert actual_query_params == expected + + return fb_numeric_simple_callback(request, **kwargs) + + # Mock the SELECT query with set parameters + httpx_mock.add_callback(validate_with_set_params_callback, url=fb_numeric_query_url) + + cursor.execute(test_query, test_params) + + +@mark.parametrize( + "test_params,expected_query_params", + [ + # Decimal types + ( + [Decimal("123.45"), Decimal("0"), Decimal("-999.999")], + [ + {"name": "$1", "value": "123.45"}, + {"name": "$2", "value": "0"}, + {"name": "$3", "value": "-999.999"}, + ], + ), + # Bytes values + ( + [b"hello", b"\x00\x01\x02", b""], + [ + {"name": "$1", "value": "b'hello'"}, + {"name": "$2", "value": "b'\\x00\\x01\\x02'"}, + {"name": "$3", "value": "b''"}, + ], + ), + # List/Array values + ( + [[1, 2, 3], ["a", "b"], [], [None, True, False]], + [ + {"name": "$1", "value": "[1, 2, 3]"}, + {"name": "$2", "value": "['a', 'b']"}, + {"name": "$3", "value": "[]"}, + {"name": "$4", "value": "[None, True, False]"}, + ], + ), + # Mixed complex types + ( + [Decimal("42.0"), b"binary", [1, "mixed"], {"key": "value"}], + [ + {"name": "$1", "value": "42.0"}, + {"name": "$2", "value": "b'binary'"}, + {"name": "$3", "value": "[1, 'mixed']"}, + {"name": "$4", "value": "{'key': 'value'}"}, + ], + ), + # Large numbers and edge cases + ( + [2**63 - 1, -(2**63), float("inf"), float("-inf")], + [ + {"name": "$1", "value": 9223372036854775807}, + {"name": "$2", "value": -9223372036854775808}, + {"name": "$3", "value": float("inf")}, + {"name": "$4", "value": float("-inf")}, + ], + ), + ], +) +def test_fb_numeric_additional_types( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, + test_params: List[Any], + expected_query_params: List[Dict[str, Any]], +): + """Test that fb_numeric paramstyle handles additional SDK-supported types correctly.""" + test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_fb_numeric_nested_complex_structures( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle handles deeply nested structures.""" + test_params = [ + {"nested": {"array": [1, 2, {"deep": True}]}}, + [{"mixed": Decimal("123.45")}, [1, [2, [3]]]], + ] + expected_query_params = [ + {"name": "$1", "value": "{'nested': {'array': [1, 2, {'deep': True}]}}"}, + {"name": "$2", "value": "[{'mixed': Decimal('123.45')}, [1, [2, [3]]]]"}, + ] + + test_query = "SELECT * FROM test WHERE data = $1 AND metadata = $2" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_fb_numeric_large_parameter_count( + cursor: Cursor, + httpx_mock: HTTPXMock, + fb_numeric_query_url: re.Pattern, + fb_numeric_callback_factory: Callable, +): + """Test that fb_numeric paramstyle handles a large number of parameters.""" + # Test with 50 parameters + test_params = list(range(50)) + expected_query_params = [{"name": f"${i+1}", "value": i} for i in range(50)] + + placeholders = ", ".join(f"${i+1}" for i in range(50)) + test_query = f"SELECT * FROM test WHERE id IN ({placeholders})" + + callback = fb_numeric_callback_factory(expected_query_params, test_query) + httpx_mock.add_callback(callback, url=fb_numeric_query_url) + + cursor.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + + +def test_unsupported_paramstyle_raises(cursor): + """Test that unsupported paramstyles raise ProgrammingError.""" + cursor.paramstyle = "not_a_style" + with raises(ProgrammingError): + cursor.execute("SELECT 1") diff --git a/tests/unit/db_conftest.py b/tests/unit/db_conftest.py index e19fa22023c..12905b0d293 100644 --- a/tests/unit/db_conftest.py +++ b/tests/unit/db_conftest.py @@ -1,9 +1,11 @@ import json +import re from dataclasses import asdict from datetime import date, datetime from decimal import Decimal from json import dumps as jdumps from typing import Any, Callable, Dict, List, Tuple +from urllib.parse import parse_qs from httpx import URL, Request, codes from pytest import fixture @@ -822,3 +824,179 @@ def do_query(request: Request, **kwargs) -> Response: return Response(status_code=codes.OK, content=streaming_error_query_response) return do_query + + +@fixture +def fb_numeric_test_parameters() -> List[List[Any]]: + """Test parameters covering all supported types for fb_numeric paramstyle.""" + return [ + # Basic types + [42, "string", 3.14, True, False, None], + # Edge cases + [0, "", 0.0, -1, -3.14], + # Complex types that should be converted to strings + [datetime(2023, 1, 1), date(2023, 1, 1)], + # Single parameter + [1], + # Empty parameters + [], + ] + + +@fixture +def fb_numeric_expected_query_params() -> List[List[Dict[str, Any]]]: + """Expected query_parameters JSON for corresponding test parameters.""" + return [ + # Basic types + [ + {"name": "$1", "value": 42}, + {"name": "$2", "value": "string"}, + {"name": "$3", "value": 3.14}, + {"name": "$4", "value": True}, + {"name": "$5", "value": False}, + {"name": "$6", "value": None}, + ], + # Edge cases + [ + {"name": "$1", "value": 0}, + {"name": "$2", "value": ""}, + {"name": "$3", "value": 0.0}, + {"name": "$4", "value": -1}, + {"name": "$5", "value": -3.14}, + ], + # Complex types (converted to strings in sync, kept as-is in async) + [ + {"name": "$1", "value": "2023-01-01 00:00:00"}, # sync behavior + {"name": "$2", "value": "2023-01-01"}, # sync behavior + ], + # Single parameter + [ + {"name": "$1", "value": 1}, + ], + # Empty parameters + [], + ] + + +@fixture +def fb_numeric_query_url(engine_url: str, db_name: str) -> re.Pattern: + """Regex pattern for fb_numeric queries that matches base URL regardless of query parameters.""" + base_url = f"https://{engine_url}" + return re.compile(rf"^{re.escape(base_url)}.*") + + +@fixture +def fb_numeric_query_url_exact(engine_url: str, db_name: str) -> URL: + """Exact URL for fb_numeric queries.""" + return URL(f"https://{engine_url}").copy_merge_params( + {"database": db_name, "output_format": "JSON_Compact"} + ) + + +@fixture +def fb_numeric_callback_factory( + query_description: List[Column], + query_data: List[List[ColType]], + query_statistics: Dict[str, Any], +) -> Callable: + """Factory for creating fb_numeric query callbacks that validate parameters.""" + + def create_callback( + expected_query_params: List[Dict[str, Any]], + expected_query: str = None, + is_async: bool = False, + ) -> Callable: + def do_query(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + # Validate query parameters in URL + qs = parse_qs(request.url.query) + query_params_raw = qs.get(b"query_parameters", []) + + if query_params_raw: + query_params_str = query_params_raw[0] + if isinstance(query_params_str, bytes): + query_params_str = query_params_str.decode() + actual_query_params = json.loads(query_params_str) + assert actual_query_params == expected_query_params, ( + f"Expected query_parameters: {expected_query_params}, " + f"got: {actual_query_params}" + ) + else: + assert ( + expected_query_params == [] + ), f"Expected empty query_parameters, but URL has: {dict(qs)}" + + # Validate query content if provided + if expected_query: + body = request.content.decode() if request.content else "" + assert expected_query in body or expected_query == body + + # Return appropriate response + if is_async: + return Response( + status_code=codes.OK, + json={ + "query_id": "test-async-token-123", + "statistics": query_statistics, + }, + ) + else: + query_response = { + "meta": [ + {"name": col.name, "type": col.type_code} + for col in query_description + ], + "data": query_data, + "rows": len(query_data), + "statistics": query_statistics, + } + return Response(status_code=codes.OK, json=query_response) + + return do_query + + return create_callback + + +@fixture +def fb_numeric_simple_callback( + query_statistics: Dict[str, Any], +) -> Callable: + """Simple callback for fb_numeric queries that just returns success.""" + + def do_query(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + query_response = { + "meta": [], + "data": [], + "rows": 0, + "statistics": query_statistics, + } + return Response(status_code=codes.OK, json=query_response) + + return do_query + + +@fixture +def fb_numeric_async_callback(async_token: str) -> Callable: + """Callback for fb_numeric async queries.""" + + def do_query(request: Request, **kwargs) -> Response: + assert request.method == "POST" + + # Validate async=True parameter + qs = parse_qs(request.url.query) + async_param = qs.get(b"async", []) + assert async_param == [b"true"], f"Expected async=true, got: {async_param}" + + return Response( + status_code=codes.OK, + json={ + "token": async_token, + "message": "Query submitted successfully", + "monitorSql": "SELECT 1", + }, + ) + + return do_query From dcfe55d45d0e1caca01ac191c47e3ce37a6d208e Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Jul 2025 17:13:59 +0100 Subject: [PATCH 02/10] improvements --- src/firebolt/common/cursor/base_cursor.py | 4 ++- .../dbapi/async/V2/test_queries_async.py | 15 +++++--- tests/unit/async_db/test_cursor.py | 36 ++++++++----------- tests/unit/db/test_cursor.py | 21 +++++------ tests/unit/db_conftest.py | 12 +++---- 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/firebolt/common/cursor/base_cursor.py b/src/firebolt/common/cursor/base_cursor.py index d5eacd71739..b6f9b4c630f 100644 --- a/src/firebolt/common/cursor/base_cursor.py +++ b/src/firebolt/common/cursor/base_cursor.py @@ -48,7 +48,9 @@ def _convert_parameter_value(value: Any) -> Any: if isinstance(value, Decimal): return str(value) elif isinstance(value, bytes): - return repr(value) # This gives us the b'...' format + return value.decode("utf-8") + elif isinstance(value, list): + return [_convert_parameter_value(item) for item in value] else: return str(value) diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index ab821fd689b..eaf10e5aa7b 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -497,7 +497,10 @@ async def test_fb_numeric_paramstyle_all_types( await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') await c.execute( 'CREATE FACT TABLE "test_fb_numeric_all_types" (' - "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, " + "a_int ARRAY(INT), dec DECIMAL(38, 3), " + "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " + "by BYTEA" ") PRIMARY INDEX i" ) params = [ @@ -508,18 +511,22 @@ async def test_fb_numeric_paramstyle_all_types( date(2022, 1, 1), # d DATE datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME True, # b BOOL - [1, 2, 3], # a ARRAY(INT) + [1, 2, 3], # a_int ARRAY(INT) Decimal("123.456"), # dec DECIMAL(38, 3) + ["hello", "world", "test"], # a_str ARRAY(STRING) + [[1, 2], [3, 4], [5]], # a_nested ARRAY(ARRAY(INT)) + Binary("test_bytea_data"), # by BYTEA ] await c.execute( - 'INSERT INTO "test_fb_numeric_all_types" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', + 'INSERT INTO "test_fb_numeric_all_types" VALUES ' + "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", params, ) await c.execute( 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] ) result = await c.fetchall() - # None is returned as None, arrays as lists, decimals as Decimal + # None is returned as None, arrays as lists, decimals as Decimal, bytea as bytes assert result == [params] diff --git a/tests/unit/async_db/test_cursor.py b/tests/unit/async_db/test_cursor.py index e1553d3e4da..7451096922b 100644 --- a/tests/unit/async_db/test_cursor.py +++ b/tests/unit/async_db/test_cursor.py @@ -1249,19 +1249,19 @@ async def validate_streaming_callback(request: Request, **kwargs) -> Response: ( [b"hello", b"\x00\x01\x02", b""], [ - {"name": "$1", "value": "b'hello'"}, - {"name": "$2", "value": "b'\\x00\\x01\\x02'"}, - {"name": "$3", "value": "b''"}, + {"name": "$1", "value": "hello"}, + {"name": "$2", "value": "\x00\x01\x02"}, + {"name": "$3", "value": ""}, ], ), # List/Array values - now converted to strings consistently ( [[1, 2, 3], ["a", "b"], [], [None, True, False]], [ - {"name": "$1", "value": "[1, 2, 3]"}, - {"name": "$2", "value": "['a', 'b']"}, - {"name": "$3", "value": "[]"}, - {"name": "$4", "value": "[None, True, False]"}, + {"name": "$1", "value": [1, 2, 3]}, + {"name": "$2", "value": ["a", "b"]}, + {"name": "$3", "value": []}, + {"name": "$4", "value": [None, True, False]}, ], ), # Mixed complex types - now converted to strings consistently @@ -1269,8 +1269,8 @@ async def validate_streaming_callback(request: Request, **kwargs) -> Response: [Decimal("42.0"), b"binary", [1, "mixed"], {"key": "value"}], [ {"name": "$1", "value": "42.0"}, - {"name": "$2", "value": "b'binary'"}, - {"name": "$3", "value": "[1, 'mixed']"}, + {"name": "$2", "value": "binary"}, + {"name": "$3", "value": [1, "mixed"]}, {"name": "$4", "value": "{'key': 'value'}"}, ], ), @@ -1302,20 +1302,12 @@ async def test_fb_numeric_mixed_basic_and_complex_types( ): """Test that fb_numeric paramstyle works with mixed basic and complex types in async mode.""" # Mix of basic types (preserved) and complex types (converted to strings) - test_params = [ - [1, 2, 3], # List -> converted to string - 42, # Basic int -> preserved - {"key": "value", "number": 42}, # Dict -> converted to string - None, # None -> preserved - ] + test_params = [[1, 2, 3], 42, {"key": "value", "number": 42}, None] expected_query_params = [ - {"name": "$1", "value": "[1, 2, 3]"}, # Converted to string - {"name": "$2", "value": 42}, # Preserved as int - { - "name": "$3", - "value": "{'key': 'value', 'number': 42}", - }, # Converted to string - {"name": "$4", "value": None}, # Preserved as None + {"name": "$1", "value": [1, 2, 3]}, + {"name": "$2", "value": 42}, + {"name": "$3", "value": "{'key': 'value', 'number': 42}"}, + {"name": "$4", "value": None}, ] test_query = ( diff --git a/tests/unit/db/test_cursor.py b/tests/unit/db/test_cursor.py index aed5800ba0d..ccb02ae5d8e 100644 --- a/tests/unit/db/test_cursor.py +++ b/tests/unit/db/test_cursor.py @@ -1143,19 +1143,19 @@ def validate_with_set_params_callback(request: Request, **kwargs): ( [b"hello", b"\x00\x01\x02", b""], [ - {"name": "$1", "value": "b'hello'"}, - {"name": "$2", "value": "b'\\x00\\x01\\x02'"}, - {"name": "$3", "value": "b''"}, + {"name": "$1", "value": "hello"}, + {"name": "$2", "value": "\x00\x01\x02"}, + {"name": "$3", "value": ""}, ], ), # List/Array values ( [[1, 2, 3], ["a", "b"], [], [None, True, False]], [ - {"name": "$1", "value": "[1, 2, 3]"}, - {"name": "$2", "value": "['a', 'b']"}, - {"name": "$3", "value": "[]"}, - {"name": "$4", "value": "[None, True, False]"}, + {"name": "$1", "value": [1, 2, 3]}, + {"name": "$2", "value": ["a", "b"]}, + {"name": "$3", "value": []}, + {"name": "$4", "value": [None, True, False]}, ], ), # Mixed complex types @@ -1163,8 +1163,8 @@ def validate_with_set_params_callback(request: Request, **kwargs): [Decimal("42.0"), b"binary", [1, "mixed"], {"key": "value"}], [ {"name": "$1", "value": "42.0"}, - {"name": "$2", "value": "b'binary'"}, - {"name": "$3", "value": "[1, 'mixed']"}, + {"name": "$2", "value": "binary"}, + {"name": "$3", "value": [1, "mixed"]}, {"name": "$4", "value": "{'key': 'value'}"}, ], ), @@ -1207,11 +1207,12 @@ def test_fb_numeric_nested_complex_structures( """Test that fb_numeric paramstyle handles deeply nested structures.""" test_params = [ {"nested": {"array": [1, 2, {"deep": True}]}}, + # Here decimal is a part of the JSON so it should be converted to string [{"mixed": Decimal("123.45")}, [1, [2, [3]]]], ] expected_query_params = [ {"name": "$1", "value": "{'nested': {'array': [1, 2, {'deep': True}]}}"}, - {"name": "$2", "value": "[{'mixed': Decimal('123.45')}, [1, [2, [3]]]]"}, + {"name": "$2", "value": ["{'mixed': Decimal('123.45')}", [1, [2, [3]]]]}, ] test_query = "SELECT * FROM test WHERE data = $1 AND metadata = $2" diff --git a/tests/unit/db_conftest.py b/tests/unit/db_conftest.py index 12905b0d293..03cfbb4f091 100644 --- a/tests/unit/db_conftest.py +++ b/tests/unit/db_conftest.py @@ -614,9 +614,9 @@ def async_multiple_query_data() -> List[List[ColType]]: "2025-01-23 14:08:06.087953+00", "2025-01-23 14:08:06.134208+00", "2025-01-23 14:08:06.410542+00", - "ENDED_SUCCESSFULLY", - "123e4567-e89b-12d3-a456-426614174000", - "f9520387-224c-48e9-9858-b2d05518ce94", + "RUNNING", + "", + "987e6543-e21b-34d3-b654-426614174111", "", "2", "2", @@ -628,9 +628,9 @@ def async_multiple_query_data() -> List[List[ColType]]: "2025-01-23 14:08:06.087953+00", "2025-01-23 14:08:06.134208+00", "2025-01-23 14:08:06.410542+00", - "RUNNING", - "", - "987e6543-e21b-34d3-b654-426614174111", + "ENDED_SUCCESSFULLY", + "123e4567-e89b-12d3-a456-426614174000", + "f9520387-224c-48e9-9858-b2d05518ce94", "", "2", "2", From bd04e1bfbcd607931de75a9db88142f8dc4f603e Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Jul 2025 17:28:38 +0100 Subject: [PATCH 03/10] refactor --- src/firebolt/async_db/cursor.py | 16 ++-------- src/firebolt/common/cursor/base_cursor.py | 37 +++++++++++++++++++++-- src/firebolt/db/cursor.py | 16 ++-------- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/firebolt/async_db/cursor.py b/src/firebolt/async_db/cursor.py index 32277e1404b..dc9563eff6b 100644 --- a/src/firebolt/async_db/cursor.py +++ b/src/firebolt/async_db/cursor.py @@ -1,6 +1,5 @@ from __future__ import annotations -import json import logging import time import warnings @@ -31,7 +30,6 @@ ) from firebolt.common.cursor.base_cursor import ( BaseCursor, - _convert_parameter_value, _parse_update_endpoint, _parse_update_parameters, _raise_if_internal_set_parameter, @@ -261,17 +259,9 @@ async def _execute_fb_numeric( Cursor._log_query(query) timeout_controller = TimeoutController(timeout) timeout_controller.raise_if_timeout() - param_list = parameters[0] if parameters else [] - query_parameters = [ - {"name": f"${i+1}", "value": _convert_parameter_value(value)} - for i, value in enumerate(param_list) - ] - query_params: Dict[str, Any] = { - "output_format": self._get_output_format(streaming), - "query_parameters": json.dumps(query_parameters), - } - if async_execution: - query_params["async"] = True + query_params = self._build_fb_numeric_query_params( + parameters, streaming, async_execution + ) resp = await self._api_request( query, query_params, diff --git a/src/firebolt/common/cursor/base_cursor.py b/src/firebolt/common/cursor/base_cursor.py index b6f9b4c630f..67ffc1d29db 100644 --- a/src/firebolt/common/cursor/base_cursor.py +++ b/src/firebolt/common/cursor/base_cursor.py @@ -1,14 +1,15 @@ from __future__ import annotations +import json import logging import re from decimal import Decimal from types import TracebackType -from typing import Any, Dict, List, Optional, Tuple, Type, Union +from typing import Any, Dict, List, Optional, Sequence, Tuple, Type, Union from httpx import URL, Response -from firebolt.common._types import RawColType, SetParameter +from firebolt.common._types import ParameterType, RawColType, SetParameter from firebolt.common.constants import ( DISALLOWED_PARAMETER_LIST, IMMUTABLE_PARAMETER_LIST, @@ -255,6 +256,38 @@ def _log_query(query: Union[str, SetParameter]) -> None: ): logger.debug(f"Running query: {query}") + def _build_fb_numeric_query_params( + self, + parameters: Sequence[Sequence[ParameterType]], + streaming: bool, + async_execution: bool, + ) -> Dict[str, Any]: + """ + Build query parameters dictionary for fb_numeric paramstyle. + + Args: + parameters: A sequence of parameter sequences. For fb_numeric, + only the first parameter sequence is used. + streaming: Whether streaming is enabled + async_execution: Whether async execution is enabled + + Returns: + Dictionary of query parameters to send with the request + """ + param_list = parameters[0] if parameters else [] + query_parameters = [ + {"name": f"${i+1}", "value": _convert_parameter_value(value)} + for i, value in enumerate(param_list) + ] + + query_params: Dict[str, Any] = { + "output_format": self._get_output_format(streaming), + "query_parameters": json.dumps(query_parameters), + } + if async_execution: + query_params["async"] = True + return query_params + @property def engine_name(self) -> str: """ diff --git a/src/firebolt/db/cursor.py b/src/firebolt/db/cursor.py index 85b37d0ea6f..01355f2b5e6 100644 --- a/src/firebolt/db/cursor.py +++ b/src/firebolt/db/cursor.py @@ -1,6 +1,5 @@ from __future__ import annotations -import json import logging import time from abc import ABCMeta, abstractmethod @@ -39,7 +38,6 @@ ) from firebolt.common.cursor.base_cursor import ( BaseCursor, - _convert_parameter_value, _parse_update_endpoint, _parse_update_parameters, _raise_if_internal_set_parameter, @@ -267,17 +265,9 @@ def _execute_fb_numeric( Cursor._log_query(query) timeout_controller = TimeoutController(timeout) timeout_controller.raise_if_timeout() - param_list = parameters[0] if parameters else [] - query_parameters = [ - {"name": f"${i+1}", "value": _convert_parameter_value(value)} - for i, value in enumerate(param_list) - ] - query_params: Dict[str, Any] = { - "output_format": self._get_output_format(streaming), - "query_parameters": json.dumps(query_parameters), - } - if async_execution: - query_params["async"] = True + query_params = self._build_fb_numeric_query_params( + parameters, streaming, async_execution + ) resp = self._api_request( query, query_params, From 9206c3436a0084fa6928b4222ee2dd150c674e11 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Tue, 22 Jul 2025 15:48:56 +0100 Subject: [PATCH 04/10] better approach --- src/firebolt/async_db/connection.py | 27 +-- src/firebolt/async_db/cursor.py | 7 +- src/firebolt/common/cursor/base_cursor.py | 3 +- src/firebolt/db/connection.py | 31 +-- src/firebolt/db/cursor.py | 7 +- .../dbapi/async/V2/test_queries_async.py | 195 ++++++++++-------- .../integration/dbapi/sync/V2/test_queries.py | 182 ++++++++-------- tests/unit/async_db/conftest.py | 10 + tests/unit/async_db/test_cursor.py | 42 ++-- tests/unit/db/conftest.py | 10 + tests/unit/db/test_cursor.py | 122 +++++++---- 11 files changed, 355 insertions(+), 281 deletions(-) diff --git a/src/firebolt/async_db/connection.py b/src/firebolt/async_db/connection.py index 45c3d2aeba4..140f2b88914 100644 --- a/src/firebolt/async_db/connection.py +++ b/src/firebolt/async_db/connection.py @@ -22,10 +22,7 @@ _parse_async_query_info_results, ) from firebolt.common.cache import _firebolt_system_engine_cache -from firebolt.common.constants import ( - DEFAULT_TIMEOUT_SECONDS, - PARAMSTYLE_DEFAULT, -) +from firebolt.common.constants import DEFAULT_TIMEOUT_SECONDS from firebolt.utils.exception import ( ConfigurationError, ConnectionClosedError, @@ -74,7 +71,6 @@ class Connection(BaseConnection): "_is_closed", "client_class", "cursor_type", - "paramstyle", ) def __init__( @@ -85,7 +81,6 @@ def __init__( cursor_type: Type[Cursor], api_endpoint: str, init_parameters: Optional[Dict[str, Any]] = None, - paramstyle: Optional[str] = PARAMSTYLE_DEFAULT, ): super().__init__(cursor_type) self.api_endpoint = api_endpoint @@ -95,17 +90,12 @@ def __init__( self.init_parameters = init_parameters or {} if database: self.init_parameters["database"] = database - self.paramstyle = paramstyle def cursor(self, **kwargs: Any) -> Cursor: if self.closed: raise ConnectionClosedError("Unable to create cursor: connection closed.") - if self.paramstyle is None: - self.paramstyle = PARAMSTYLE_DEFAULT - c = self.cursor_type( - client=self._client, connection=self, paramstyle=self.paramstyle, **kwargs - ) + c = self.cursor_type(client=self._client, connection=self, **kwargs) self._cursors.append(c) return c @@ -228,10 +218,7 @@ async def connect( disable_cache: bool = False, url: Optional[str] = None, additional_parameters: Dict[str, Any] = {}, - paramstyle: Optional[str] = None, ) -> Connection: - if paramstyle is None: - paramstyle = PARAMSTYLE_DEFAULT # auth parameter is optional in function signature # but is required to connect. # PEP 249 recommends making it kwargs. @@ -257,7 +244,6 @@ async def connect( user_agent_header=user_agent_header, database=database, connection_url=url, - paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V2: assert account_name is not None @@ -268,7 +254,6 @@ async def connect( database=database, engine_name=engine_name, api_endpoint=api_endpoint, - paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V1: return await connect_v1( @@ -279,7 +264,6 @@ async def connect( engine_name=engine_name, engine_url=engine_url, api_endpoint=api_endpoint, - paramstyle=paramstyle, ) else: raise ConfigurationError(f"Unsupported auth type: {type(auth)}") @@ -292,7 +276,6 @@ async def connect_v2( database: Optional[str] = None, engine_name: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, - paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt. @@ -339,7 +322,6 @@ async def connect_v2( CursorV2, api_endpoint, system_engine_params, - paramstyle=paramstyle, ) as system_engine_connection: cursor = system_engine_connection.cursor() @@ -356,7 +338,6 @@ async def connect_v2( CursorV2, api_endpoint, cursor.parameters, - paramstyle=paramstyle, ) @@ -368,7 +349,6 @@ async def connect_v1( engine_name: Optional[str] = None, engine_url: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, - paramstyle: Optional[str] = None, ) -> Connection: # These parameters are optional in function signature # but are required to connect. @@ -422,7 +402,6 @@ async def connect_v1( client, CursorV1, api_endpoint, - paramstyle=paramstyle or PARAMSTYLE_DEFAULT, ) @@ -431,7 +410,6 @@ def connect_core( user_agent_header: str, database: Optional[str] = None, connection_url: Optional[str] = None, - paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt Core. @@ -469,5 +447,4 @@ def connect_core( client=client, cursor_type=CursorV2, api_endpoint=verified_url, - paramstyle=paramstyle, ) diff --git a/src/firebolt/async_db/cursor.py b/src/firebolt/async_db/cursor.py index dc9563eff6b..430a13cd843 100644 --- a/src/firebolt/async_db/cursor.py +++ b/src/firebolt/async_db/cursor.py @@ -21,7 +21,6 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, - PARAMSTYLE_DEFAULT, PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, @@ -83,11 +82,9 @@ def __init__( *args: Any, client: AsyncClient, connection: "Connection", - paramstyle: str = PARAMSTYLE_DEFAULT, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) - self.paramstyle = paramstyle self._client = client self.connection = connection self.engine_url = connection.engine_url @@ -220,7 +217,9 @@ async def _do_execute( ) -> None: await self._close_rowset_and_reset() self._row_set = StreamingAsyncRowSet() if streaming else InMemoryAsyncRowSet() - paramstyle = self.paramstyle or PARAMSTYLE_DEFAULT + # Import paramstyle from module level + from firebolt.async_db import paramstyle + if paramstyle not in PARAMSTYLE_SUPPORTED: raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: diff --git a/src/firebolt/common/cursor/base_cursor.py b/src/firebolt/common/cursor/base_cursor.py index 67ffc1d29db..96f1edb7835 100644 --- a/src/firebolt/common/cursor/base_cursor.py +++ b/src/firebolt/common/cursor/base_cursor.py @@ -282,8 +282,9 @@ def _build_fb_numeric_query_params( query_params: Dict[str, Any] = { "output_format": self._get_output_format(streaming), - "query_parameters": json.dumps(query_parameters), } + if query_parameters: + query_params["query_parameters"] = json.dumps(query_parameters) if async_execution: query_params["async"] = True return query_params diff --git a/src/firebolt/db/connection.py b/src/firebolt/db/connection.py index 8c13e9fdc9c..d678cacd2fa 100644 --- a/src/firebolt/db/connection.py +++ b/src/firebolt/db/connection.py @@ -21,10 +21,7 @@ _parse_async_query_info_results, ) from firebolt.common.cache import _firebolt_system_engine_cache -from firebolt.common.constants import ( - DEFAULT_TIMEOUT_SECONDS, - PARAMSTYLE_DEFAULT, -) +from firebolt.common.constants import DEFAULT_TIMEOUT_SECONDS from firebolt.db.cursor import Cursor, CursorV1, CursorV2 from firebolt.db.util import _get_system_engine_url_and_params from firebolt.utils.exception import ( @@ -53,7 +50,6 @@ def connect( disable_cache: bool = False, url: Optional[str] = None, additional_parameters: Dict[str, Any] = {}, - paramstyle: Optional[str] = None, ) -> Connection: # auth parameter is optional in function signature # but is required to connect. @@ -61,9 +57,6 @@ def connect( if not auth: raise ConfigurationError("auth is required to connect.") - if paramstyle is None: - paramstyle = PARAMSTYLE_DEFAULT - # Type checks assert auth is not None user_drivers = additional_parameters.get("user_drivers", []) @@ -84,7 +77,6 @@ def connect( user_agent_header=user_agent_header, database=database, connection_url=url, - paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V2: assert account_name is not None @@ -95,7 +87,6 @@ def connect( database=database, engine_name=engine_name, api_endpoint=api_endpoint, - paramstyle=paramstyle, ) elif auth_version == FireboltAuthVersion.V1: return connect_v1( @@ -106,7 +97,6 @@ def connect( engine_name=engine_name, engine_url=engine_url, api_endpoint=api_endpoint, - paramstyle=paramstyle, ) else: raise ConfigurationError(f"Unsupported auth type: {type(auth)}") @@ -119,7 +109,6 @@ def connect_v2( database: Optional[str] = None, engine_name: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, - paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt. @@ -166,7 +155,6 @@ def connect_v2( CursorV2, api_endpoint, system_engine_params, - paramstyle=paramstyle, ) as system_engine_connection: cursor = system_engine_connection.cursor() @@ -183,7 +171,6 @@ def connect_v2( CursorV2, api_endpoint, cursor.parameters, - paramstyle=paramstyle, ) @@ -214,7 +201,6 @@ class Connection(BaseConnection): "_is_closed", "client_class", "cursor_type", - "paramstyle", ) def __init__( @@ -225,7 +211,6 @@ def __init__( cursor_type: Type[Cursor], api_endpoint: str = DEFAULT_API_URL, init_parameters: Optional[Dict[str, Any]] = None, - paramstyle: Optional[str] = PARAMSTYLE_DEFAULT, ): super().__init__(cursor_type) self.api_endpoint = api_endpoint @@ -235,16 +220,11 @@ def __init__( self.init_parameters = init_parameters or {} if database: self.init_parameters["database"] = database - self.paramstyle = paramstyle def cursor(self, **kwargs: Any) -> Cursor: if self.closed: raise ConnectionClosedError("Unable to create cursor: connection closed.") - if self.paramstyle is None: - self.paramstyle = PARAMSTYLE_DEFAULT - c = self.cursor_type( - client=self._client, connection=self, paramstyle=self.paramstyle, **kwargs - ) + c = self.cursor_type(client=self._client, connection=self, **kwargs) self._cursors.append(c) return c @@ -373,7 +353,6 @@ def connect_v1( engine_name: Optional[str] = None, engine_url: Optional[str] = None, api_endpoint: str = DEFAULT_API_URL, - paramstyle: Optional[str] = None, ) -> Connection: # These parameters are optional in function signature # but are required to connect. @@ -422,9 +401,7 @@ def connect_v1( timeout=Timeout(DEFAULT_TIMEOUT_SECONDS, read=None), headers={"User-Agent": user_agent_header}, ) - return Connection( - engine_url, database, client, CursorV1, api_endpoint, paramstyle=paramstyle - ) + return Connection(engine_url, database, client, CursorV1, api_endpoint) def connect_core( @@ -432,7 +409,6 @@ def connect_core( user_agent_header: str, database: Optional[str] = None, connection_url: Optional[str] = None, - paramstyle: Optional[str] = None, ) -> Connection: """Connect to Firebolt Core. @@ -471,5 +447,4 @@ def connect_core( client=client, cursor_type=CursorV2, api_endpoint=verified_url, - paramstyle=paramstyle, ) diff --git a/src/firebolt/db/cursor.py b/src/firebolt/db/cursor.py index 01355f2b5e6..7e7e4bd7ef0 100644 --- a/src/firebolt/db/cursor.py +++ b/src/firebolt/db/cursor.py @@ -29,7 +29,6 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, - PARAMSTYLE_DEFAULT, PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, @@ -89,11 +88,9 @@ def __init__( *args: Any, client: Client, connection: "Connection", - paramstyle: str = PARAMSTYLE_DEFAULT, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) - self.paramstyle = paramstyle self._client = client self.connection = connection self.engine_url = connection.engine_url @@ -226,7 +223,9 @@ def _do_execute( ) -> None: self._close_rowset_and_reset() self._row_set = StreamingRowSet() if streaming else InMemoryRowSet() - paramstyle = self.paramstyle or PARAMSTYLE_DEFAULT + # Import paramstyle from module level + from firebolt.db import paramstyle + if paramstyle not in PARAMSTYLE_SUPPORTED: raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index eaf10e5aa7b..1b5f95e5e4d 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -485,104 +485,125 @@ async def test_fb_numeric_paramstyle_all_types( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_all_types" (' - "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, " - "a_int ARRAY(INT), dec DECIMAL(38, 3), " - "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " - "by BYTEA" - ") PRIMARY INDEX i" - ) - params = [ - 1, # i INT - 1.123, # f FLOAT - "text", # s STRING - None, # sn STRING NULL - date(2022, 1, 1), # d DATE - datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME - True, # b BOOL - [1, 2, 3], # a_int ARRAY(INT) - Decimal("123.456"), # dec DECIMAL(38, 3) - ["hello", "world", "test"], # a_str ARRAY(STRING) - [[1, 2], [3, 4], [5]], # a_nested ARRAY(ARRAY(INT)) - Binary("test_bytea_data"), # by BYTEA - ] - await c.execute( - 'INSERT INTO "test_fb_numeric_all_types" VALUES ' - "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", - params, - ) - await c.execute( - 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] - ) - result = await c.fetchall() - # None is returned as None, arrays as lists, decimals as Decimal, bytea as bytes - assert result == [params] + import firebolt.async_db as async_db + + original_paramstyle = async_db.paramstyle + try: + async_db.paramstyle = "fb_numeric" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, " + "a_int ARRAY(INT), dec DECIMAL(38, 3), " + "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " + "by BYTEA" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a_int ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ["hello", "world", "test"], # a_str ARRAY(STRING) + [[1, 2], [3, 4], [5]], # a_nested ARRAY(ARRAY(INT)) + Binary("test_bytea_data"), # by BYTEA + ] + await c.execute( + 'INSERT INTO "test_fb_numeric_all_types" VALUES ' + "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", + params, + ) + await c.execute( + 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] + ) + result = await c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal, bytea as bytes + assert result == [params] + finally: + async_db.paramstyle = original_paramstyle async def test_fb_numeric_paramstyle_not_enough_params( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: not enough parameters supplied.""" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' - ) - # Only one param for two placeholders - try: + import firebolt.async_db as async_db + + original_paramstyle = async_db.paramstyle + try: + async_db.paramstyle = "fb_numeric" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') await c.execute( - 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() + 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' ) - else: - assert False, "Expected error for not enough parameters" + # Only one param for two placeholders + try: + await c.execute( + 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() + ) + else: + assert False, "Expected error for not enough parameters" + finally: + async_db.paramstyle = original_paramstyle async def test_fb_numeric_paramstyle_too_many_params( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' - ) - # Three params for two placeholders: should succeed, extra param ignored - await c.execute( - 'INSERT INTO "test_fb_numeric_params2" VALUES ($1, $2)', [1, "foo", 123] - ) - await c.execute('SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1]) - result = await c.fetchall() - assert result == [[1, "foo"]] + import firebolt.async_db as async_db + + original_paramstyle = async_db.paramstyle + try: + async_db.paramstyle = "fb_numeric" + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + await c.execute( + 'INSERT INTO "test_fb_numeric_params2" VALUES ($1, $2)', + [1, "foo", 123], + ) + await c.execute( + 'SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1] + ) + result = await c.fetchall() + assert result == [[1, "foo"]] + finally: + async_db.paramstyle = original_paramstyle diff --git a/tests/integration/dbapi/sync/V2/test_queries.py b/tests/integration/dbapi/sync/V2/test_queries.py index c85bb86557c..fe0a33b55a0 100644 --- a/tests/integration/dbapi/sync/V2/test_queries.py +++ b/tests/integration/dbapi/sync/V2/test_queries.py @@ -567,98 +567,118 @@ def test_fb_numeric_paramstyle_all_types( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' - "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" - ") PRIMARY INDEX i" - ) - params = [ - 1, # i INT - 1.123, # f FLOAT - "text", # s STRING - None, # sn STRING NULL - date(2022, 1, 1), # d DATE - datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME - True, # b BOOL - [1, 2, 3], # a ARRAY(INT) - Decimal("123.456"), # dec DECIMAL(38, 3) - ] - c.execute( - 'INSERT INTO "test_fb_numeric_all_types_sync" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', - params, - ) - c.execute( - 'SELECT * FROM "test_fb_numeric_all_types_sync" WHERE i = $1', [1] - ) - result = c.fetchall() - # None is returned as None, arrays as lists, decimals as Decimal - assert result == [params] + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ] + c.execute( + 'INSERT INTO "test_fb_numeric_all_types_sync" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', + params, + ) + c.execute( + 'SELECT * FROM "test_fb_numeric_all_types_sync" WHERE i = $1', [1] + ) + result = c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal + assert result == [params] + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_paramstyle_not_enough_params( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: not enough parameters supplied.""" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' - ) - # Only one param for two placeholders - try: + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') c.execute( - 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() + 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' ) - else: - assert False, "Expected error for not enough parameters" + # Only one param for two placeholders + try: + c.execute( + 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() + ) + else: + assert False, "Expected error for not enough parameters" + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_paramstyle_too_many_params( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - paramstyle="fb_numeric", - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' - ) - # Three params for two placeholders: should succeed, extra param ignored - c.execute( - 'INSERT INTO "test_fb_numeric_params2_sync" VALUES ($1, $2)', - [1, "foo", 123], - ) - c.execute('SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1]) - result = c.fetchall() - assert result == [[1, "foo"]] + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + c.execute( + 'INSERT INTO "test_fb_numeric_params2_sync" VALUES ($1, $2)', + [1, "foo", 123], + ) + c.execute( + 'SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1] + ) + result = c.fetchall() + assert result == [[1, "foo"]] + finally: + db.paramstyle = original_paramstyle diff --git a/tests/unit/async_db/conftest.py b/tests/unit/async_db/conftest.py index 8de506f35bf..0e70bde62b6 100644 --- a/tests/unit/async_db/conftest.py +++ b/tests/unit/async_db/conftest.py @@ -1,5 +1,6 @@ from pytest import fixture +import firebolt.async_db from firebolt.async_db import Connection, Cursor, connect from firebolt.client.auth import Auth from tests.unit.db_conftest import * # noqa @@ -32,3 +33,12 @@ async def connection( @fixture async def cursor(connection: Connection) -> Cursor: return connection.cursor() + + +@fixture +def fb_numeric_paramstyle(): + """Fixture that sets paramstyle to fb_numeric and resets it after the test.""" + original_paramstyle = firebolt.async_db.paramstyle + firebolt.async_db.paramstyle = "fb_numeric" + yield + firebolt.async_db.paramstyle = original_paramstyle diff --git a/tests/unit/async_db/test_cursor.py b/tests/unit/async_db/test_cursor.py index 7451096922b..01d6a14b07a 100644 --- a/tests/unit/async_db/test_cursor.py +++ b/tests/unit/async_db/test_cursor.py @@ -1005,6 +1005,7 @@ async def test_fb_numeric_parameter_formatting( fb_numeric_callback_factory: Callable, test_params: List[Any], expected_query_params: List[Dict[str, Any]], + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle formats parameters correctly for various types.""" test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" @@ -1012,7 +1013,6 @@ async def test_fb_numeric_parameter_formatting( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" await cursor.execute(test_query, test_params) @@ -1034,8 +1034,14 @@ async def test_fb_numeric_complex_types_converted_to_strings_async( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - await cursor.execute(test_query, test_params) + import firebolt.async_db as async_db + + original_paramstyle = async_db.paramstyle + try: + async_db.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + finally: + async_db.paramstyle = original_paramstyle async def test_fb_numeric_no_client_side_substitution( @@ -1055,8 +1061,14 @@ async def test_fb_numeric_no_client_side_substitution( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - await cursor.execute(test_query, test_params) + import firebolt.async_db as async_db + + original_paramstyle = async_db.paramstyle + try: + async_db.paramstyle = "fb_numeric" + await cursor.execute(test_query, test_params) + finally: + async_db.paramstyle = original_paramstyle async def test_fb_numeric_executemany( @@ -1283,6 +1295,7 @@ async def test_fb_numeric_additional_types_unified_behavior( fb_numeric_callback_factory: Callable, test_params: List[Any], expected_query_params: List[Dict[str, Any]], + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle handles additional types consistently in async mode (same as sync).""" test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" @@ -1290,7 +1303,6 @@ async def test_fb_numeric_additional_types_unified_behavior( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" await cursor.execute(test_query, test_params) @@ -1299,6 +1311,7 @@ async def test_fb_numeric_mixed_basic_and_complex_types( httpx_mock: HTTPXMock, fb_numeric_query_url: re.Pattern, fb_numeric_callback_factory: Callable, + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle works with mixed basic and complex types in async mode.""" # Mix of basic types (preserved) and complex types (converted to strings) @@ -1317,7 +1330,6 @@ async def test_fb_numeric_mixed_basic_and_complex_types( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" await cursor.execute(test_query, test_params) @@ -1326,6 +1338,7 @@ async def test_fb_numeric_large_parameter_count_async( httpx_mock: HTTPXMock, fb_numeric_query_url: re.Pattern, fb_numeric_callback_factory: Callable, + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle handles a large number of parameters in async mode.""" # Test with 50 parameters @@ -1338,7 +1351,6 @@ async def test_fb_numeric_large_parameter_count_async( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" await cursor.execute(test_query, test_params) @@ -1347,6 +1359,7 @@ async def test_fb_numeric_special_float_values_async( httpx_mock: HTTPXMock, fb_numeric_query_url: re.Pattern, fb_numeric_callback_factory: Callable, + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle handles special float values in async mode.""" test_params = [ @@ -1367,12 +1380,17 @@ async def test_fb_numeric_special_float_values_async( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" await cursor.execute(test_query, test_params) async def test_unsupported_paramstyle_raises(cursor: Cursor) -> None: """Test that unsupported paramstyles raise ProgrammingError.""" - cursor.paramstyle = "not_a_style" - with raises(ProgrammingError): - await cursor.execute("SELECT 1") + import firebolt.async_db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "not_a_style" + with raises(ProgrammingError): + await cursor.execute("SELECT 1") + finally: + db.paramstyle = original_paramstyle diff --git a/tests/unit/db/conftest.py b/tests/unit/db/conftest.py index 489533b0abe..d7385242f64 100644 --- a/tests/unit/db/conftest.py +++ b/tests/unit/db/conftest.py @@ -2,6 +2,7 @@ from pytest import fixture +import firebolt.db from firebolt.client.auth import Auth from firebolt.db import Connection, Cursor, connect @@ -49,3 +50,12 @@ def system_connection( @fixture def cursor(connection: Connection) -> Cursor: return connection.cursor() + + +@fixture +def fb_numeric_paramstyle(): + """Fixture that sets paramstyle to fb_numeric and resets it after the test.""" + original_paramstyle = firebolt.db.paramstyle + firebolt.db.paramstyle = "fb_numeric" + yield + firebolt.db.paramstyle = original_paramstyle diff --git a/tests/unit/db/test_cursor.py b/tests/unit/db/test_cursor.py index ccb02ae5d8e..889ae456b52 100644 --- a/tests/unit/db/test_cursor.py +++ b/tests/unit/db/test_cursor.py @@ -992,6 +992,7 @@ def test_fb_numeric_parameter_formatting( fb_numeric_callback_factory: Callable, test_params: List[Any], expected_query_params: List[Dict[str, Any]], + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle formats parameters correctly for various types.""" test_query = f"SELECT * FROM test WHERE col IN ({', '.join(f'${i+1}' for i in range(len(test_params)))})" @@ -999,7 +1000,6 @@ def test_fb_numeric_parameter_formatting( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" cursor.execute(test_query, test_params) @@ -1008,6 +1008,7 @@ def test_fb_numeric_complex_types_converted_to_strings( httpx_mock: HTTPXMock, fb_numeric_query_url: re.Pattern, fb_numeric_callback_factory: Callable, + fb_numeric_paramstyle, ): """Test that fb_numeric paramstyle converts complex types to strings in sync mode.""" test_params = [datetime(2023, 1, 1, 12, 30), date(2023, 6, 15)] @@ -1021,7 +1022,6 @@ def test_fb_numeric_complex_types_converted_to_strings( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" cursor.execute(test_query, test_params) @@ -1042,8 +1042,14 @@ def test_fb_numeric_no_client_side_substitution( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - cursor.execute(test_query, test_params) + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_executemany( @@ -1081,8 +1087,14 @@ def validate_executemany_callback(request: Request, **kwargs) -> Response: httpx_mock.add_callback(validate_executemany_callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - cursor.executemany(test_query, test_params_seq) + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + cursor.executemany(test_query, test_params_seq) + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_with_cursor_set_parameters( @@ -1092,39 +1104,47 @@ def test_fb_numeric_with_cursor_set_parameters( fb_numeric_simple_callback: Callable, ): """Test that fb_numeric paramstyle works correctly when cursor has pre-existing set parameters.""" - cursor.paramstyle = "fb_numeric" + import firebolt.db as db - # Manually set a parameter in the cursor (simulating what would happen - # if SET was called in a different paramstyle mode) - cursor._set_parameters = {"my_param": "test_value"} + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" - test_query = "SELECT * FROM test WHERE id = $1" - test_params = [42] + # Manually set a parameter in the cursor (simulating what would happen + # if SET was called in a different paramstyle mode) + cursor._set_parameters = {"my_param": "test_value"} - def validate_with_set_params_callback(request: Request, **kwargs): - assert request.method == "POST" + test_query = "SELECT * FROM test WHERE id = $1" + test_params = [42] - # Should include both set parameters and query parameters - qs = parse_qs(request.url.query) + def validate_with_set_params_callback(request: Request, **kwargs): + assert request.method == "POST" - # Check for set parameter - assert b"my_param" in qs - assert qs[b"my_param"] == [b"test_value"] + # Should include both set parameters and query parameters + qs = parse_qs(request.url.query) - # Check for query parameters - query_params_raw = qs.get(b"query_parameters", []) - if query_params_raw: - query_params_str = query_params_raw[0].decode() - actual_query_params = json.loads(query_params_str) - expected = [{"name": "$1", "value": 42}] - assert actual_query_params == expected + # Check for set parameter + assert b"my_param" in qs + assert qs[b"my_param"] == [b"test_value"] - return fb_numeric_simple_callback(request, **kwargs) + # Check for query parameters + query_params_raw = qs.get(b"query_parameters", []) + if query_params_raw: + query_params_str = query_params_raw[0].decode() + actual_query_params = json.loads(query_params_str) + expected = [{"name": "$1", "value": 42}] + assert actual_query_params == expected - # Mock the SELECT query with set parameters - httpx_mock.add_callback(validate_with_set_params_callback, url=fb_numeric_query_url) + return fb_numeric_simple_callback(request, **kwargs) - cursor.execute(test_query, test_params) + # Mock the SELECT query with set parameters + httpx_mock.add_callback( + validate_with_set_params_callback, url=fb_numeric_query_url + ) + + cursor.execute(test_query, test_params) + finally: + db.paramstyle = original_paramstyle @mark.parametrize( @@ -1194,8 +1214,14 @@ def test_fb_numeric_additional_types( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - cursor.execute(test_query, test_params) + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_nested_complex_structures( @@ -1220,8 +1246,14 @@ def test_fb_numeric_nested_complex_structures( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - cursor.execute(test_query, test_params) + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + finally: + db.paramstyle = original_paramstyle def test_fb_numeric_large_parameter_count( @@ -1241,12 +1273,24 @@ def test_fb_numeric_large_parameter_count( callback = fb_numeric_callback_factory(expected_query_params, test_query) httpx_mock.add_callback(callback, url=fb_numeric_query_url) - cursor.paramstyle = "fb_numeric" - cursor.execute(test_query, test_params) + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "fb_numeric" + cursor.execute(test_query, test_params) + finally: + db.paramstyle = original_paramstyle def test_unsupported_paramstyle_raises(cursor): """Test that unsupported paramstyles raise ProgrammingError.""" - cursor.paramstyle = "not_a_style" - with raises(ProgrammingError): - cursor.execute("SELECT 1") + import firebolt.db as db + + original_paramstyle = db.paramstyle + try: + db.paramstyle = "not_a_style" + with raises(ProgrammingError): + cursor.execute("SELECT 1") + finally: + db.paramstyle = original_paramstyle From 104243a0569eeea57657219def66dac9b777da9c Mon Sep 17 00:00:00 2001 From: ptiurin Date: Tue, 22 Jul 2025 16:27:24 +0100 Subject: [PATCH 05/10] refactor integration tests --- tests/integration/dbapi/async/V2/conftest.py | 10 + .../dbapi/async/V2/test_queries_async.py | 199 ++++++++---------- tests/integration/dbapi/sync/V2/conftest.py | 10 + .../integration/dbapi/sync/V2/test_queries.py | 183 +++++++--------- 4 files changed, 188 insertions(+), 214 deletions(-) diff --git a/tests/integration/dbapi/async/V2/conftest.py b/tests/integration/dbapi/async/V2/conftest.py index 7265dc1596f..81ac291233a 100644 --- a/tests/integration/dbapi/async/V2/conftest.py +++ b/tests/integration/dbapi/async/V2/conftest.py @@ -4,6 +4,7 @@ from pytest import fixture +import firebolt.async_db from firebolt.async_db import Connection, connect from firebolt.client.auth.base import Auth from firebolt.client.auth.client_credentials import ClientCredentials @@ -164,3 +165,12 @@ async def mixed_case_db_and_engine( await system_cursor.execute(f'DROP DATABASE "{test_db_name}"') await system_cursor.execute(f'STOP ENGINE "{test_engine_name}"') await system_cursor.execute(f'DROP ENGINE "{test_engine_name}"') + + +@fixture +def fb_numeric_paramstyle(): + """Fixture that sets paramstyle to fb_numeric and resets it after the test.""" + original_paramstyle = firebolt.async_db.paramstyle + firebolt.async_db.paramstyle = "fb_numeric" + yield + firebolt.async_db.paramstyle = original_paramstyle diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index 1b5f95e5e4d..5e54781d016 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -482,128 +482,105 @@ async def test_select_struct( async def test_fb_numeric_paramstyle_all_types( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" - import firebolt.async_db as async_db - - original_paramstyle = async_db.paramstyle - try: - async_db.paramstyle = "fb_numeric" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_all_types" (' - "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, " - "a_int ARRAY(INT), dec DECIMAL(38, 3), " - "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " - "by BYTEA" - ") PRIMARY INDEX i" - ) - params = [ - 1, # i INT - 1.123, # f FLOAT - "text", # s STRING - None, # sn STRING NULL - date(2022, 1, 1), # d DATE - datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME - True, # b BOOL - [1, 2, 3], # a_int ARRAY(INT) - Decimal("123.456"), # dec DECIMAL(38, 3) - ["hello", "world", "test"], # a_str ARRAY(STRING) - [[1, 2], [3, 4], [5]], # a_nested ARRAY(ARRAY(INT)) - Binary("test_bytea_data"), # by BYTEA - ] - await c.execute( - 'INSERT INTO "test_fb_numeric_all_types" VALUES ' - "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", - params, - ) - await c.execute( - 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] - ) - result = await c.fetchall() - # None is returned as None, arrays as lists, decimals as Decimal, bytea as bytes - assert result == [params] - finally: - async_db.paramstyle = original_paramstyle + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, " + "a_int ARRAY(INT), dec DECIMAL(38, 3), " + "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " + "by BYTEA" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a_int ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ["hello", "world", "test"], # a_str ARRAY(STRING) + [[1, 2], [3, 4], [5]], # a_nested ARRAY(ARRAY(INT)) + Binary("test_bytea_data"), # by BYTEA + ] + await c.execute( + 'INSERT INTO "test_fb_numeric_all_types" VALUES ' + "($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", + params, + ) + await c.execute( + 'SELECT * FROM "test_fb_numeric_all_types" WHERE i = $1', [1] + ) + result = await c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal, bytea as bytes + assert result == [params] async def test_fb_numeric_paramstyle_not_enough_params( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: not enough parameters supplied.""" - import firebolt.async_db as async_db - - original_paramstyle = async_db.paramstyle - try: - async_db.paramstyle = "fb_numeric" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' + ) + # Only one param for two placeholders + try: await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' + 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() ) - # Only one param for two placeholders - try: - await c.execute( - 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() - ) - else: - assert False, "Expected error for not enough parameters" - finally: - async_db.paramstyle = original_paramstyle + else: + assert False, "Expected error for not enough parameters" async def test_fb_numeric_paramstyle_too_many_params( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" - import firebolt.async_db as async_db - - original_paramstyle = async_db.paramstyle - try: - async_db.paramstyle = "fb_numeric" - async with await connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' - ) - # Three params for two placeholders: should succeed, extra param ignored - await c.execute( - 'INSERT INTO "test_fb_numeric_params2" VALUES ($1, $2)', - [1, "foo", 123], - ) - await c.execute( - 'SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1] - ) - result = await c.fetchall() - assert result == [[1, "foo"]] - finally: - async_db.paramstyle = original_paramstyle + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + async with connection.cursor() as c: + await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') + await c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + await c.execute( + 'INSERT INTO "test_fb_numeric_params2" VALUES ($1, $2)', + [1, "foo", 123], + ) + await c.execute('SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1]) + result = await c.fetchall() + assert result == [[1, "foo"]] diff --git a/tests/integration/dbapi/sync/V2/conftest.py b/tests/integration/dbapi/sync/V2/conftest.py index c6b70f66de0..63ec51d5e9d 100644 --- a/tests/integration/dbapi/sync/V2/conftest.py +++ b/tests/integration/dbapi/sync/V2/conftest.py @@ -4,6 +4,7 @@ from pytest import fixture +import firebolt.db from firebolt.client.auth.base import Auth from firebolt.client.auth.client_credentials import ClientCredentials from firebolt.db import Connection, connect @@ -164,3 +165,12 @@ def mixed_case_db_and_engine( system_cursor.execute(f'DROP DATABASE "{test_db_name}"') system_cursor.execute(f'STOP ENGINE "{test_engine_name}"') system_cursor.execute(f'DROP ENGINE "{test_engine_name}"') + + +@fixture +def fb_numeric_paramstyle(): + """Fixture that sets paramstyle to fb_numeric and resets it after the test.""" + original_paramstyle = firebolt.db.paramstyle + firebolt.db.paramstyle = "fb_numeric" + yield + firebolt.db.paramstyle = original_paramstyle diff --git a/tests/integration/dbapi/sync/V2/test_queries.py b/tests/integration/dbapi/sync/V2/test_queries.py index fe0a33b55a0..e79fe5dcde1 100644 --- a/tests/integration/dbapi/sync/V2/test_queries.py +++ b/tests/integration/dbapi/sync/V2/test_queries.py @@ -564,121 +564,98 @@ def test_select_struct( def test_fb_numeric_paramstyle_all_types( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: insert/select all supported types, and parameter count errors.""" - import firebolt.db as db - - original_paramstyle = db.paramstyle - try: - db.paramstyle = "fb_numeric" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' - "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" - ") PRIMARY INDEX i" - ) - params = [ - 1, # i INT - 1.123, # f FLOAT - "text", # s STRING - None, # sn STRING NULL - date(2022, 1, 1), # d DATE - datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME - True, # b BOOL - [1, 2, 3], # a ARRAY(INT) - Decimal("123.456"), # dec DECIMAL(38, 3) - ] - c.execute( - 'INSERT INTO "test_fb_numeric_all_types_sync" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', - params, - ) - c.execute( - 'SELECT * FROM "test_fb_numeric_all_types_sync" WHERE i = $1', [1] - ) - result = c.fetchall() - # None is returned as None, arrays as lists, decimals as Decimal - assert result == [params] - finally: - db.paramstyle = original_paramstyle + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_all_types_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' + "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" + ") PRIMARY INDEX i" + ) + params = [ + 1, # i INT + 1.123, # f FLOAT + "text", # s STRING + None, # sn STRING NULL + date(2022, 1, 1), # d DATE + datetime(2022, 1, 1, 1, 1, 1), # dt DATETIME + True, # b BOOL + [1, 2, 3], # a ARRAY(INT) + Decimal("123.456"), # dec DECIMAL(38, 3) + ] + c.execute( + 'INSERT INTO "test_fb_numeric_all_types_sync" VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)', + params, + ) + c.execute( + 'SELECT * FROM "test_fb_numeric_all_types_sync" WHERE i = $1', [1] + ) + result = c.fetchall() + # None is returned as None, arrays as lists, decimals as Decimal + assert result == [params] def test_fb_numeric_paramstyle_not_enough_params( engine_name, database_name, auth, account_name, api_endpoint ): """Test fb_numeric paramstyle: not enough parameters supplied.""" - import firebolt.db as db - - original_paramstyle = db.paramstyle - try: - db.paramstyle = "fb_numeric" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' + ) + # Only one param for two placeholders + try: c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' + 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] + ) + except Exception as e: + assert ( + "parameter" in str(e).lower() + or "argument" in str(e).lower() + or "missing" in str(e).lower() ) - # Only one param for two placeholders - try: - c.execute( - 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() - ) - else: - assert False, "Expected error for not enough parameters" - finally: - db.paramstyle = original_paramstyle + else: + assert False, "Expected error for not enough parameters" def test_fb_numeric_paramstyle_too_many_params( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: too many parameters supplied (should succeed).""" - import firebolt.db as db - - original_paramstyle = db.paramstyle - try: - db.paramstyle = "fb_numeric" - with connect( - engine_name=engine_name, - database=database_name, - auth=auth, - account_name=account_name, - api_endpoint=api_endpoint, - ) as connection: - with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' - ) - # Three params for two placeholders: should succeed, extra param ignored - c.execute( - 'INSERT INTO "test_fb_numeric_params2_sync" VALUES ($1, $2)', - [1, "foo", 123], - ) - c.execute( - 'SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1] - ) - result = c.fetchall() - assert result == [[1, "foo"]] - finally: - db.paramstyle = original_paramstyle + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + with connection.cursor() as c: + c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') + c.execute( + 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' + ) + # Three params for two placeholders: should succeed, extra param ignored + c.execute( + 'INSERT INTO "test_fb_numeric_params2_sync" VALUES ($1, $2)', + [1, "foo", 123], + ) + c.execute('SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1]) + result = c.fetchall() + assert result == [[1, "foo"]] From 8ab1417714aa08ad52c2303644e0352e1c72ef5b Mon Sep 17 00:00:00 2001 From: ptiurin Date: Tue, 22 Jul 2025 16:56:50 +0100 Subject: [PATCH 06/10] improve tests --- .../dbapi/async/V2/test_queries_async.py | 59 ++++++++++-------- .../integration/dbapi/sync/V2/test_queries.py | 62 ++++++++++--------- 2 files changed, 67 insertions(+), 54 deletions(-) diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index 5e54781d016..628b5c6d108 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -134,7 +134,7 @@ async def test_query(c: Cursor, query: str) -> None: await test_query( c, 'CREATE FACT TABLE "test_drop_create_async"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int)) primary index id", + "d date, dt datetime, b bool, a array(int))d", ) # Dimension table @@ -177,7 +177,7 @@ async def test_empty_query(c: Cursor, query: str) -> None: await c.execute('DROP TABLE IF EXISTS "test_insert_async_tb"') await c.execute( 'CREATE FACT TABLE "test_insert_async_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int)) primary index id" + "d date, dt datetime, b bool, a array(int))d" ) await test_empty_query( @@ -226,7 +226,7 @@ async def test_empty_query(c: Cursor, query: str, params: tuple) -> None: await c.execute( 'CREATE FACT TABLE "test_tb_async_parameterized"(i int, f float, s string, sn' " string null, d date, dt datetime, b bool, a array(int), " - "dec decimal(38, 3), ss string) primary index i", + "dec decimal(38, 3), ss string)", ) params = [ @@ -284,8 +284,7 @@ async def test_multi_statement_query(connection: Connection) -> None: async with connection.cursor() as c: await c.execute('DROP TABLE IF EXISTS "test_tb_async_multi_statement"') await c.execute( - 'CREATE FACT TABLE "test_tb_async_multi_statement"(i int, s string)' - " primary index i" + 'CREATE FACT TABLE "test_tb_async_multi_statement"(i int, s string)' "" ) await c.execute( @@ -352,9 +351,7 @@ async def test_bytea_roundtrip( """Inserted and than selected bytea value doesn't get corrupted.""" async with connection.cursor() as c: await c.execute('DROP TABLE IF EXISTS "test_bytea_roundtrip_2"') - await c.execute( - 'CREATE FACT TABLE "test_bytea_roundtrip_2"(id int, b bytea) primary index id' - ) + await c.execute('CREATE FACT TABLE "test_bytea_roundtrip_2"(id int, b bytea)d') data = "bytea_123\n\tヽ༼ຈل͜ຈ༽ﾉ" @@ -500,7 +497,7 @@ async def test_fb_numeric_paramstyle_all_types( "a_int ARRAY(INT), dec DECIMAL(38, 3), " "a_str ARRAY(STRING), a_nested ARRAY(ARRAY(INT)), " "by BYTEA" - ") PRIMARY INDEX i" + ")" ) params = [ 1, # i INT @@ -541,23 +538,12 @@ async def test_fb_numeric_paramstyle_not_enough_params( api_endpoint=api_endpoint, ) as connection: async with connection.cursor() as c: - await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params"') - await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params" (i INT, s STRING) PRIMARY INDEX i' + with raises(FireboltStructuredError) as exc_info: + await c.execute("SELECT $1, $2", [1]) + assert ( + "query referenced positional parameter $2, but it was not set" + in str(exc_info.value).lower() ) - # Only one param for two placeholders - try: - await c.execute( - 'INSERT INTO "test_fb_numeric_params" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() - ) - else: - assert False, "Expected error for not enough parameters" async def test_fb_numeric_paramstyle_too_many_params( @@ -574,7 +560,7 @@ async def test_fb_numeric_paramstyle_too_many_params( async with connection.cursor() as c: await c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2"') await c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING) PRIMARY INDEX i' + 'CREATE FACT TABLE "test_fb_numeric_params2" (i INT, s STRING)' ) # Three params for two placeholders: should succeed, extra param ignored await c.execute( @@ -584,3 +570,24 @@ async def test_fb_numeric_paramstyle_too_many_params( await c.execute('SELECT * FROM "test_fb_numeric_params2" WHERE i = $1', [1]) result = await c.fetchall() assert result == [[1, "foo"]] + + +async def test_fb_numeric_paramstyle_incorrect_params( + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle +): + async with await connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + c = connection.cursor() + with raises(FireboltStructuredError) as exc_info: + await c.execute( + "SELECT $34, $72", + [1, "foo"], + ) + assert "Query referenced positional parameter $34, but it was not set" in str( + exc_info.value + ) diff --git a/tests/integration/dbapi/sync/V2/test_queries.py b/tests/integration/dbapi/sync/V2/test_queries.py index e79fe5dcde1..23d3304a395 100644 --- a/tests/integration/dbapi/sync/V2/test_queries.py +++ b/tests/integration/dbapi/sync/V2/test_queries.py @@ -138,7 +138,7 @@ def test_query(c: Cursor, query: str) -> None: test_query( c, 'CREATE FACT TABLE "test_drop_create_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int)) primary index id", + "d date, dt datetime, b bool, a array(int))d", ) # Dimension table @@ -181,7 +181,7 @@ def test_empty_query(c: Cursor, query: str) -> None: c.execute('DROP TABLE IF EXISTS "test_insert_tb"') c.execute( 'CREATE FACT TABLE "test_insert_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int)) primary index id" + "d date, dt datetime, b bool, a array(int))d" ) test_empty_query( @@ -243,7 +243,7 @@ def test_empty_query(c: Cursor, query: str, params: tuple) -> None: c.execute( 'CREATE FACT TABLE "test_tb_parameterized"(i int, f float, s string, sn' " string null, d date, dt datetime, b bool, a array(int), " - "dec decimal(38, 3), ss string) primary index i", + "dec decimal(38, 3), ss string)", ) params = [ @@ -284,9 +284,7 @@ def test_multi_statement_query(connection: Connection) -> None: with connection.cursor() as c: c.execute('DROP TABLE IF EXISTS "test_tb_multi_statement"') - c.execute( - 'CREATE FACT TABLE "test_tb_multi_statement"(i int, s string) primary index i' - ) + c.execute('CREATE FACT TABLE "test_tb_multi_statement"(i int, s string)') c.execute( "INSERT INTO \"test_tb_multi_statement\" values (1, 'a'), (2, 'b');" @@ -439,9 +437,7 @@ def test_bytea_roundtrip( """Inserted and than selected bytea value doesn't get corrupted.""" with connection.cursor() as c: c.execute('DROP TABLE IF EXISTS "test_bytea_roundtrip"') - c.execute( - 'CREATE FACT TABLE "test_bytea_roundtrip"(id int, b bytea) primary index id' - ) + c.execute('CREATE FACT TABLE "test_bytea_roundtrip"(id int, b bytea)d') data = "bytea_123\n\tヽ༼ຈل͜ຈ༽ﾉ" @@ -579,7 +575,7 @@ def test_fb_numeric_paramstyle_all_types( c.execute( 'CREATE FACT TABLE "test_fb_numeric_all_types_sync" (' "i INT, f FLOAT, s STRING, sn STRING NULL, d DATE, dt DATETIME, b BOOL, a ARRAY(INT), dec DECIMAL(38, 3)" - ") PRIMARY INDEX i" + ")" ) params = [ 1, # i INT @@ -605,7 +601,7 @@ def test_fb_numeric_paramstyle_all_types( def test_fb_numeric_paramstyle_not_enough_params( - engine_name, database_name, auth, account_name, api_endpoint + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle ): """Test fb_numeric paramstyle: not enough parameters supplied.""" with connect( @@ -616,23 +612,12 @@ def test_fb_numeric_paramstyle_not_enough_params( api_endpoint=api_endpoint, ) as connection: with connection.cursor() as c: - c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params_sync"') - c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params_sync" (i INT, s STRING) PRIMARY INDEX i' + with raises(FireboltStructuredError) as exc_info: + c.execute("SELECT $1, $2", [1]) + assert ( + "query referenced positional parameter $2, but it was not set" + in str(exc_info.value).lower() ) - # Only one param for two placeholders - try: - c.execute( - 'INSERT INTO "test_fb_numeric_params_sync" VALUES ($1, $2)', [1] - ) - except Exception as e: - assert ( - "parameter" in str(e).lower() - or "argument" in str(e).lower() - or "missing" in str(e).lower() - ) - else: - assert False, "Expected error for not enough parameters" def test_fb_numeric_paramstyle_too_many_params( @@ -649,7 +634,7 @@ def test_fb_numeric_paramstyle_too_many_params( with connection.cursor() as c: c.execute('DROP TABLE IF EXISTS "test_fb_numeric_params2_sync"') c.execute( - 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING) PRIMARY INDEX i' + 'CREATE FACT TABLE "test_fb_numeric_params2_sync" (i INT, s STRING)' ) # Three params for two placeholders: should succeed, extra param ignored c.execute( @@ -659,3 +644,24 @@ def test_fb_numeric_paramstyle_too_many_params( c.execute('SELECT * FROM "test_fb_numeric_params2_sync" WHERE i = $1', [1]) result = c.fetchall() assert result == [[1, "foo"]] + + +def test_fb_numeric_paramstyle_incorrect_params( + engine_name, database_name, auth, account_name, api_endpoint, fb_numeric_paramstyle +): + with connect( + engine_name=engine_name, + database=database_name, + auth=auth, + account_name=account_name, + api_endpoint=api_endpoint, + ) as connection: + c = connection.cursor() + with raises(FireboltStructuredError) as exc_info: + c.execute( + "SELECT $34, $72", + [1, "foo"], + ) + assert "Query referenced positional parameter $34, but it was not set" in str( + exc_info.value + ) From 8a98b26c96d360572220c88822149fdea4e7aa71 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Tue, 22 Jul 2025 17:36:35 +0100 Subject: [PATCH 07/10] fixup find-replace --- src/firebolt/async_db/cursor.py | 6 +++--- src/firebolt/db/cursor.py | 6 +++--- .../dbapi/async/V2/test_queries_async.py | 14 +++++++++----- tests/integration/dbapi/sync/V2/test_queries.py | 14 +++++++++----- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/firebolt/async_db/cursor.py b/src/firebolt/async_db/cursor.py index 430a13cd843..9c1ac5df4e2 100644 --- a/src/firebolt/async_db/cursor.py +++ b/src/firebolt/async_db/cursor.py @@ -81,7 +81,7 @@ def __init__( self, *args: Any, client: AsyncClient, - connection: "Connection", + connection: Connection, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) @@ -559,7 +559,7 @@ def __init__( self, *args: Any, client: AsyncClientV2, - connection: "Connection", + connection: Connection, **kwargs: Any, ) -> None: assert isinstance(client, AsyncClientV2) @@ -642,7 +642,7 @@ def __init__( self, *args: Any, client: AsyncClientV1, - connection: "Connection", + connection: Connection, **kwargs: Any, ) -> None: assert isinstance(client, AsyncClientV1) diff --git a/src/firebolt/db/cursor.py b/src/firebolt/db/cursor.py index 7e7e4bd7ef0..74771089e44 100644 --- a/src/firebolt/db/cursor.py +++ b/src/firebolt/db/cursor.py @@ -87,7 +87,7 @@ def __init__( self, *args: Any, client: Client, - connection: "Connection", + connection: Connection, **kwargs: Any, ) -> None: super().__init__(*args, **kwargs) @@ -519,7 +519,7 @@ def __enter__(self) -> Cursor: class CursorV2(Cursor): def __init__( - self, *args: Any, client: Client, connection: "Connection", **kwargs: Any + self, *args: Any, client: Client, connection: Connection, **kwargs: Any ) -> None: assert isinstance(client, ClientV2) # Type check super().__init__( @@ -599,7 +599,7 @@ def execute_async( class CursorV1(Cursor): def __init__( - self, *args: Any, client: ClientV1, connection: "Connection", **kwargs: Any + self, *args: Any, client: ClientV1, connection: Connection, **kwargs: Any ) -> None: assert isinstance(client, ClientV1) # Type check super().__init__( diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index 628b5c6d108..ef6c149b87f 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -134,7 +134,7 @@ async def test_query(c: Cursor, query: str) -> None: await test_query( c, 'CREATE FACT TABLE "test_drop_create_async"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int))d", + "d date, dt datetime, b bool, a array(int))", ) # Dimension table @@ -177,7 +177,7 @@ async def test_empty_query(c: Cursor, query: str) -> None: await c.execute('DROP TABLE IF EXISTS "test_insert_async_tb"') await c.execute( 'CREATE FACT TABLE "test_insert_async_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int))d" + "d date, dt datetime, b bool, a array(int)) primary index id" ) await test_empty_query( @@ -226,7 +226,7 @@ async def test_empty_query(c: Cursor, query: str, params: tuple) -> None: await c.execute( 'CREATE FACT TABLE "test_tb_async_parameterized"(i int, f float, s string, sn' " string null, d date, dt datetime, b bool, a array(int), " - "dec decimal(38, 3), ss string)", + "dec decimal(38, 3), ss string) primary index i", ) params = [ @@ -284,7 +284,9 @@ async def test_multi_statement_query(connection: Connection) -> None: async with connection.cursor() as c: await c.execute('DROP TABLE IF EXISTS "test_tb_async_multi_statement"') await c.execute( - 'CREATE FACT TABLE "test_tb_async_multi_statement"(i int, s string)' "" + 'CREATE FACT TABLE "test_tb_async_multi_statement"(i int, s string)' + "" + " primary index i" ) await c.execute( @@ -351,7 +353,9 @@ async def test_bytea_roundtrip( """Inserted and than selected bytea value doesn't get corrupted.""" async with connection.cursor() as c: await c.execute('DROP TABLE IF EXISTS "test_bytea_roundtrip_2"') - await c.execute('CREATE FACT TABLE "test_bytea_roundtrip_2"(id int, b bytea)d') + await c.execute( + 'CREATE FACT TABLE "test_bytea_roundtrip_2"(id int, b bytea) primary index id' + ) data = "bytea_123\n\tヽ༼ຈل͜ຈ༽ﾉ" diff --git a/tests/integration/dbapi/sync/V2/test_queries.py b/tests/integration/dbapi/sync/V2/test_queries.py index 23d3304a395..ff07b49012d 100644 --- a/tests/integration/dbapi/sync/V2/test_queries.py +++ b/tests/integration/dbapi/sync/V2/test_queries.py @@ -138,7 +138,7 @@ def test_query(c: Cursor, query: str) -> None: test_query( c, 'CREATE FACT TABLE "test_drop_create_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int))d", + "d date, dt datetime, b bool, a array(int)) primary index id", ) # Dimension table @@ -181,7 +181,7 @@ def test_empty_query(c: Cursor, query: str) -> None: c.execute('DROP TABLE IF EXISTS "test_insert_tb"') c.execute( 'CREATE FACT TABLE "test_insert_tb"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int))d" + "d date, dt datetime, b bool, a array(int)) primary index id" ) test_empty_query( @@ -243,7 +243,7 @@ def test_empty_query(c: Cursor, query: str, params: tuple) -> None: c.execute( 'CREATE FACT TABLE "test_tb_parameterized"(i int, f float, s string, sn' " string null, d date, dt datetime, b bool, a array(int), " - "dec decimal(38, 3), ss string)", + "dec decimal(38, 3), ss string) primary index i", ) params = [ @@ -284,7 +284,9 @@ def test_multi_statement_query(connection: Connection) -> None: with connection.cursor() as c: c.execute('DROP TABLE IF EXISTS "test_tb_multi_statement"') - c.execute('CREATE FACT TABLE "test_tb_multi_statement"(i int, s string)') + c.execute( + 'CREATE FACT TABLE "test_tb_multi_statement"(i int, s string) primary index i' + ) c.execute( "INSERT INTO \"test_tb_multi_statement\" values (1, 'a'), (2, 'b');" @@ -437,7 +439,9 @@ def test_bytea_roundtrip( """Inserted and than selected bytea value doesn't get corrupted.""" with connection.cursor() as c: c.execute('DROP TABLE IF EXISTS "test_bytea_roundtrip"') - c.execute('CREATE FACT TABLE "test_bytea_roundtrip"(id int, b bytea)d') + c.execute( + 'CREATE FACT TABLE "test_bytea_roundtrip"(id int, b bytea) primary index id' + ) data = "bytea_123\n\tヽ༼ຈل͜ຈ༽ﾉ" From a43b97b9a69a54c2554dc7549b700536489daaad Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 23 Jul 2025 09:18:44 +0100 Subject: [PATCH 08/10] remove unintended changes --- tests/integration/dbapi/async/V2/test_queries_async.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/dbapi/async/V2/test_queries_async.py b/tests/integration/dbapi/async/V2/test_queries_async.py index ef6c149b87f..e6e16a280a4 100644 --- a/tests/integration/dbapi/async/V2/test_queries_async.py +++ b/tests/integration/dbapi/async/V2/test_queries_async.py @@ -134,7 +134,7 @@ async def test_query(c: Cursor, query: str) -> None: await test_query( c, 'CREATE FACT TABLE "test_drop_create_async"(id int, sn string null, f float,' - "d date, dt datetime, b bool, a array(int))", + "d date, dt datetime, b bool, a array(int)) primary index id", ) # Dimension table @@ -285,7 +285,6 @@ async def test_multi_statement_query(connection: Connection) -> None: await c.execute('DROP TABLE IF EXISTS "test_tb_async_multi_statement"') await c.execute( 'CREATE FACT TABLE "test_tb_async_multi_statement"(i int, s string)' - "" " primary index i" ) From b1c54bdbeedbd708b0649c8d9ef7ee8ad0db3ce5 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 23 Jul 2025 17:41:33 +0100 Subject: [PATCH 09/10] enum --- src/firebolt/async_db/__init__.py | 4 ++-- src/firebolt/async_db/cursor.py | 8 +++++--- src/firebolt/common/constants.py | 8 ++++++-- src/firebolt/db/__init__.py | 4 ++-- src/firebolt/db/cursor.py | 8 +++++--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/firebolt/async_db/__init__.py b/src/firebolt/async_db/__init__.py index 0ee41340e11..b88f10e56b1 100644 --- a/src/firebolt/async_db/__init__.py +++ b/src/firebolt/async_db/__init__.py @@ -18,7 +18,7 @@ Timestamp, TimestampFromTicks, ) -from firebolt.common.constants import PARAMSTYLE_DEFAULT +from firebolt.common.constants import ParameterStyle from firebolt.utils.exception import ( DatabaseError, DataError, @@ -35,7 +35,7 @@ apilevel = "2.0" # threads may only share the module and connections, cursors should not be shared threadsafety = 2 -paramstyle = PARAMSTYLE_DEFAULT +paramstyle = ParameterStyle.QMARK.value """ The parameter style for SQL queries. Supported values: - 'qmark': Use ? as parameter placeholders (default, client-side substitution) diff --git a/src/firebolt/async_db/cursor.py b/src/firebolt/async_db/cursor.py index 9c1ac5df4e2..20fa2c1a7e8 100644 --- a/src/firebolt/async_db/cursor.py +++ b/src/firebolt/async_db/cursor.py @@ -21,11 +21,11 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, - PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, UPDATE_PARAMETERS_HEADER, CursorState, + ParameterStyle, ) from firebolt.common.cursor.base_cursor import ( BaseCursor, @@ -220,10 +220,12 @@ async def _do_execute( # Import paramstyle from module level from firebolt.async_db import paramstyle - if paramstyle not in PARAMSTYLE_SUPPORTED: + try: + parameter_style = ParameterStyle(paramstyle) + except ValueError: raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: - if paramstyle == "fb_numeric": + if parameter_style == ParameterStyle.FB_NUMERIC: await self._execute_fb_numeric( raw_query, parameters, timeout, async_execution, streaming ) diff --git a/src/firebolt/common/constants.py b/src/firebolt/common/constants.py index 2e98d4370fe..9e1a94cf764 100644 --- a/src/firebolt/common/constants.py +++ b/src/firebolt/common/constants.py @@ -19,6 +19,12 @@ class CursorState(Enum): CLOSED = 4 +class ParameterStyle(Enum): + QMARK = "qmark" # ? as parameter placeholders (default, client-side) + NATIVE = "native" # Alias for 'qmark' + FB_NUMERIC = "fb_numeric" # $1, $2, ... as placeholders (server-side) + + # Parameters that should be set using USE instead of SET USE_PARAMETER_LIST = ["database", "engine"] # parameters that can only be set by the backend @@ -28,5 +34,3 @@ class CursorState(Enum): UPDATE_ENDPOINT_HEADER = "Firebolt-Update-Endpoint" UPDATE_PARAMETERS_HEADER = "Firebolt-Update-Parameters" RESET_SESSION_HEADER = "Firebolt-Reset-Session" -PARAMSTYLE_DEFAULT = "qmark" -PARAMSTYLE_SUPPORTED = ["qmark", "native", "fb_numeric"] diff --git a/src/firebolt/db/__init__.py b/src/firebolt/db/__init__.py index 78a452c29cf..07bfb01641d 100644 --- a/src/firebolt/db/__init__.py +++ b/src/firebolt/db/__init__.py @@ -16,7 +16,7 @@ Timestamp, TimestampFromTicks, ) -from firebolt.common.constants import PARAMSTYLE_DEFAULT +from firebolt.common.constants import ParameterStyle from firebolt.db.connection import Connection, connect from firebolt.db.cursor import Cursor from firebolt.utils.exception import ( @@ -35,7 +35,7 @@ apilevel = "2.0" # threads may only share the module and connections, cursors should not be shared threadsafety = 2 -paramstyle = PARAMSTYLE_DEFAULT +paramstyle = ParameterStyle.QMARK.value """ The parameter style for SQL queries. Supported values: - 'qmark': Use ? as parameter placeholders (default, client-side substitution) diff --git a/src/firebolt/db/cursor.py b/src/firebolt/db/cursor.py index 74771089e44..d8d913894e7 100644 --- a/src/firebolt/db/cursor.py +++ b/src/firebolt/db/cursor.py @@ -29,11 +29,11 @@ from firebolt.common._types import ColType, ParameterType, SetParameter from firebolt.common.constants import ( JSON_OUTPUT_FORMAT, - PARAMSTYLE_SUPPORTED, RESET_SESSION_HEADER, UPDATE_ENDPOINT_HEADER, UPDATE_PARAMETERS_HEADER, CursorState, + ParameterStyle, ) from firebolt.common.cursor.base_cursor import ( BaseCursor, @@ -226,10 +226,12 @@ def _do_execute( # Import paramstyle from module level from firebolt.db import paramstyle - if paramstyle not in PARAMSTYLE_SUPPORTED: + try: + parameter_style = ParameterStyle(paramstyle) + except ValueError: raise ProgrammingError(f"Unsupported paramstyle: {paramstyle}") try: - if paramstyle == "fb_numeric": + if parameter_style == ParameterStyle.FB_NUMERIC: self._execute_fb_numeric( raw_query, parameters, timeout, async_execution, streaming ) From c2a03d2887fe7df2b34984910bdb8ba0d0ec5f20 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 23 Jul 2025 17:53:53 +0100 Subject: [PATCH 10/10] refcator to formatter --- src/firebolt/common/cursor/base_cursor.py | 34 +++------------------- src/firebolt/common/statement_formatter.py | 28 ++++++++++++++++++ 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/firebolt/common/cursor/base_cursor.py b/src/firebolt/common/cursor/base_cursor.py index 96f1edb7835..da89fc0f404 100644 --- a/src/firebolt/common/cursor/base_cursor.py +++ b/src/firebolt/common/cursor/base_cursor.py @@ -3,7 +3,6 @@ import json import logging import re -from decimal import Decimal from types import TracebackType from typing import Any, Dict, List, Optional, Sequence, Tuple, Type, Union @@ -28,34 +27,6 @@ logger = logging.getLogger(__name__) -def _convert_parameter_value(value: Any) -> Any: - """ - Convert parameter values for fb_numeric paramstyle to JSON-serializable format. - - This ensures consistent behavior between sync and async cursors. - Basic types (int, float, bool, None) are preserved as-is. - All other types are converted to strings for JSON serialization. - - Args: - value: The parameter value to convert - - Returns: - JSON-serializable value (int, float, bool, None, or string) - """ - if isinstance(value, (int, float, bool)) or value is None: - return value - - # Handle special cases for better string representation - if isinstance(value, Decimal): - return str(value) - elif isinstance(value, bytes): - return value.decode("utf-8") - elif isinstance(value, list): - return [_convert_parameter_value(item) for item in value] - else: - return str(value) - - def _parse_update_parameters(parameter_header: str) -> Dict[str, str]: """Parse update parameters and set them as attributes.""" # parse key1=value1,key2=value2 comma separated string into dict @@ -276,7 +247,10 @@ def _build_fb_numeric_query_params( """ param_list = parameters[0] if parameters else [] query_parameters = [ - {"name": f"${i+1}", "value": _convert_parameter_value(value)} + { + "name": f"${i+1}", + "value": self._formatter.convert_parameter_for_serialization(value), + } for i, value in enumerate(param_list) ] diff --git a/src/firebolt/common/statement_formatter.py b/src/firebolt/common/statement_formatter.py index 1f5e55abaac..27a79a031c4 100644 --- a/src/firebolt/common/statement_formatter.py +++ b/src/firebolt/common/statement_formatter.py @@ -62,6 +62,34 @@ def format_value(self, value: ParameterType) -> str: raise DataError(f"unsupported parameter type {type(value)}") + def convert_parameter_for_serialization( + self, value: ParameterType + ) -> Union[int, float, bool, None, str, List]: + """ + Convert parameter values for fb_numeric paramstyle to JSON-serializable + format. This is used for server-side parameter substitution. + + Basic types (int, float, bool, None) are preserved as-is. + All other types are converted to strings for JSON serialization. + + Args: + value: The parameter value to convert + + Returns: + JSON-serializable value (int, float, bool, None, or string) + """ + if isinstance(value, (int, float, bool)) or value is None: + return value + + if isinstance(value, Decimal): + return str(value) + elif isinstance(value, bytes): + return value.decode("utf-8") + elif isinstance(value, list): + return [self.convert_parameter_for_serialization(item) for item in value] + else: + return str(value) + def format_statement( self, statement: Statement, parameters: Sequence[ParameterType] ) -> str: