Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion providers/exasol/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ PIP package Version required
``apache-airflow`` ``>=2.11.0``
``apache-airflow-providers-common-compat`` ``>=1.12.0``
``apache-airflow-providers-common-sql`` ``>=1.32.0``
``pyexasol`` ``>=0.26.0,<2``
``pyexasol`` ``>=2.0.0,<3``
``pandas`` ``>=2.1.2; python_version < "3.13"``
``pandas`` ``>=2.2.3; python_version >= "3.13" and python_version < "3.14"``
``pandas`` ``>=2.3.3; python_version >= "3.14"``
Expand Down
20 changes: 20 additions & 0 deletions providers/exasol/docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@
Changelog
---------

Breaking changes
~~~~~~~~~~~~~~~~

* ``Migrate exasol provider to pyexasol 2.x; remove the <2 cap (#69123)``

This provider now requires ``pyexasol>=2.0.0,<3``. pyexasol 2.0.0 ships a ``py.typed`` marker
(`pyexasol changelog <https://exasol.github.io/pyexasol/master/changes/changes_2.html>`__), which
surfaces several previously-hidden type mismatches in the Exasol hook:

* ``ExasolHook.get_records()`` and ``ExasolHook.get_first()`` now only accept a single SQL string,
not a list of statements (pyexasol's underlying ``execute()`` never actually supported a list; use
``ExasolHook.run()`` for executing multiple statements in sequence).
* ``parameters``/``query_params`` passed to ``get_records()``, ``get_first()``, ``run()``, and
``get_df()``/``get_pandas_df()`` must be a ``dict``/``Mapping``. Positional-style parameters
(lists/tuples) were never actually supported by pyexasol at runtime and will now raise a clear
``TypeError`` instead of failing deep inside pyexasol (or silently doing the wrong thing).
* If you pass ``export_params={"with_column_names": ...}`` to ``ExasolHook.export_to_file()`` or
``ExasolToS3Operator``, note that pyexasol 2.0.0 fixed this option to be interpreted as a real
boolean; previously any value (including ``False``) was treated as ``True``.
Comment on lines +30 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The list note stuff but nothing here strike as airflow breaking change is it?
Can you just remove the breaking changes title?

also, did you validate these statements? This feels AI generated. Please provide evidences that the points here are valid. Just referencing to the upstream docs/code should be enough

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are two things I can confirm with the changelog

  • with_column_names boolean fix: I can confirm that pyexasol's changelog has a dedicated bugfix (https://github.com/exasol/pyexasol/releases#release-2.2.1 - # 265)). So any value used to get forced to be True. Now it should require a real Boolean

  • py.typed marker: Confirmed. In the Same Changelog batch above (# 298) pyexasol added it when swapping the deprecated rsa dependency for cryptography. I think that this is the actual mechanism not a breaking change.

However,

  • get_records()/get_first() only accepting a single SQL string. I cannot confirm this one tho. No changelog says explicitly this changed in 2.x. However, it's consistent with every example in pyexasol's docs since they only show a single string. This means that's always the case.

  • Parameters should be dict/Mapping: Can't confirm either, but no changelog ref to point to.

So, two claims are true, two are unverified assumptions.
I suggest to drop the "Breaking changes" title since it is not a change to the provider's own interface and drop the two unverifiable bullets entirely. Keep only the two we can back:

This provider now requires ``pyexasol>=2.0.0,<3``. pyexasol 2.0.0 added a
``py.typed`` marker and fixed ``export_params={"with_column_names": ...}``
to require a real boolean (previously any value, including ``False``, was
treated as ``True`` — pyexasol changelog, bugfix #265).


4.10.4
......

Expand Down
2 changes: 1 addition & 1 deletion providers/exasol/docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ PIP package Version required
``apache-airflow`` ``>=2.11.0``
``apache-airflow-providers-common-compat`` ``>=1.12.0``
``apache-airflow-providers-common-sql`` ``>=1.32.0``
``pyexasol`` ``>=0.26.0,<2``
``pyexasol`` ``>=2.0.0,<3``
``pandas`` ``>=2.1.2; python_version < "3.13"``
``pandas`` ``>=2.2.3; python_version >= "3.13" and python_version < "3.14"``
``pandas`` ``>=2.3.3; python_version >= "3.14"``
Expand Down
6 changes: 3 additions & 3 deletions providers/exasol/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ dependencies = [
"apache-airflow>=2.11.0",
"apache-airflow-providers-common-compat>=1.12.0",
"apache-airflow-providers-common-sql>=1.32.0",
# Capped to 1.x: pyexasol 2.x ships stricter types that break the exasol hook.
# Remove the cap after migrating; tracked at https://github.com/apache/airflow/issues/69123
"pyexasol>=0.26.0,<2",
# Migrated to pyexasol 2.x (adds a py.typed marker; see hooks/exasol.py for the resulting fixes).
# Capped to 2.x since we haven't yet verified compatibility with a future 3.x major release.
"pyexasol>=2.0.0,<3",
'pandas>=2.1.2; python_version <"3.13"',
'pandas>=2.2.3; python_version >="3.13" and python_version <"3.14"',
'pandas>=2.3.3; python_version >="3.14"',
Expand Down
68 changes: 50 additions & 18 deletions providers/exasol/src/airflow/providers/exasol/hooks/exasol.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,36 @@ def __init__(self, *args, sqlalchemy_scheme: str | None = None, **kwargs) -> Non
self.schema = kwargs.pop("schema", None)
self._sqlalchemy_scheme = sqlalchemy_scheme

@staticmethod
def _validate_query_params(parameters: Iterable | Mapping[str, Any] | None) -> dict[str, Any] | None:
"""
Validate that query parameters are a dict, as required by pyexasol.

:param parameters: The parameters as passed to this hook's public methods.
:return: The same parameters, narrowed to ``dict[str, Any] | None`` for pyexasol/mypy.
"""
if parameters is not None and not isinstance(parameters, dict):
raise TypeError(
f"Exasol (pyexasol) only supports named/dict-style query parameters, got "
f"{type(parameters).__name__!r}. Pass a dict (e.g. {{'col1': 'value'}}) instead."
)
return parameters

def get_conn(self) -> ExaConnection:
conn = self.get_connection(self.get_conn_id())
airflow_conn = self.get_connection(self.get_conn_id())
conn_args = {
"dsn": f"{conn.host}:{conn.port}",
"user": conn.login,
"password": conn.password,
"schema": self.schema or conn.schema,
"dsn": f"{airflow_conn.host}:{airflow_conn.port}",
"user": airflow_conn.login,
"password": airflow_conn.password,
"schema": self.schema or airflow_conn.schema,
}
# check for parameters in conn.extra
for arg_name, arg_val in conn.extra_dejson.items():
for arg_name, arg_val in airflow_conn.extra_dejson.items():
if arg_name in ["compression", "encryption", "json_lib", "client_name"]:
conn_args[arg_name] = arg_val

conn = pyexasol.connect(**conn_args)
return conn
exa_conn = pyexasol.connect(**conn_args)
return exa_conn

@property
def sqlalchemy_scheme(self) -> str:
Expand Down Expand Up @@ -145,7 +160,7 @@ def _get_pandas_df(
``pyexasol.ExaConnection.export_to_pandas``.
"""
with closing(self.get_conn()) as conn:
df = conn.export_to_pandas(sql, query_params=parameters, **kwargs)
df = conn.export_to_pandas(sql, query_params=self._validate_query_params(parameters), **kwargs)
return df

@deprecated(
Expand Down Expand Up @@ -184,11 +199,19 @@ def get_records(
"""
Execute the SQL and return a set of records.

:param sql: the sql statement to be executed (str) or a list of
sql statements to execute
:param parameters: The parameters to render the SQL query with.
:param sql: the sql statement to be executed. Must be a single ``str``; pyexasol's ``execute()``
does not accept a list of statements. For executing multiple statements in sequence, use
:meth:`run` with ``handler=exasol_fetch_all_handler`` instead.
:param parameters: The parameters to render the SQL query with. Must be a ``dict``/``Mapping``;
pyexasol does not support positional (list/tuple) query parameters.
"""
with closing(self.get_conn()) as conn, closing(conn.execute(sql, parameters)) as cur:
if not isinstance(sql, str):
raise TypeError(
"ExasolHook.get_records() only supports a single SQL string, not a list of statements. "
"Use ExasolHook.run() with a handler for executing multiple statements."
)
query_params = self._validate_query_params(parameters)
with closing(self.get_conn()) as conn, closing(conn.execute(sql, query_params)) as cur:
send_sql_hook_lineage(
context=self,
sql=sql,
Expand All @@ -201,11 +224,19 @@ def get_first(self, sql: str | list[str], parameters: Iterable | Mapping[str, An
"""
Execute the SQL and return the first resulting row.

:param sql: the sql statement to be executed (str) or a list of
sql statements to execute
:param parameters: The parameters to render the SQL query with.
:param sql: the sql statement to be executed. Must be a single ``str``; pyexasol's ``execute()``
does not accept a list of statements. For executing multiple statements in sequence, use
:meth:`run` with ``handler=exasol_fetch_one_handler`` instead.
:param parameters: The parameters to render the SQL query with. Must be a ``dict``/``Mapping``;
pyexasol does not support positional (list/tuple) query parameters.
"""
with closing(self.get_conn()) as conn, closing(conn.execute(sql, parameters)) as cur:
if not isinstance(sql, str):
raise TypeError(
"ExasolHook.get_first() only supports a single SQL string, not a list of statements. "
"Use ExasolHook.run() with a handler for executing multiple statements."
)
query_params = self._validate_query_params(parameters)
with closing(self.get_conn()) as conn, closing(conn.execute(sql, query_params)) as cur:
send_sql_hook_lineage(
context=self,
sql=sql,
Expand Down Expand Up @@ -328,13 +359,14 @@ def run(
self.log.debug("Executing following statements against Exasol DB: %s", list(sql_list))
else:
raise ValueError("List of SQL statements is empty")
query_params = self._validate_query_params(parameters)
_last_result = None
with closing(self.get_conn()) as conn:
self.set_autocommit(conn, autocommit)
results = []
for sql_statement in sql_list:
self.log.info("Running statement: %s, parameters: %s", sql_statement, parameters)
with closing(conn.execute(sql_statement, parameters)) as exa_statement:
with closing(conn.execute(sql_statement, query_params)) as exa_statement:
if handler is not None:
result = self._make_common_data_structure(handler(exa_statement))

Expand Down
28 changes: 27 additions & 1 deletion providers/exasol/tests/unit/exasol/hooks/test_exasol.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,19 @@ def test_run_with_autocommit(self):

def test_run_with_parameters(self):
sql = "SQL"
parameters = ("param1", "param2")
parameters = {"param1": "value1", "param2": "value2"}
self.db_hook.run(sql, autocommit=True, parameters=parameters)
self.conn.set_autocommit.assert_called_once_with(True)
self.conn.execute.assert_called_once_with(sql, parameters)
self.conn.commit.assert_not_called()

def test_run_with_non_mapping_parameters_raises(self):
sql = "SQL"
parameters = ("param1", "param2")
with pytest.raises(TypeError, match="only supports named/dict-style query parameters"):
self.db_hook.run(sql, autocommit=True, parameters=parameters)
self.conn.execute.assert_not_called()

def test_run_multi_queries(self):
sql = ["SQL1", "SQL2"]
self.db_hook.run(sql, autocommit=True)
Expand Down Expand Up @@ -251,6 +258,20 @@ def test_get_first_hook_lineage(self, mock_send_lineage):
assert call_kw["sql_parameters"] is None
assert call_kw["cur"] is self.cur

@pytest.mark.parametrize("method_name", ["get_records", "get_first"])
def test_get_records_and_get_first_reject_list_of_statements(self, method_name):
method = getattr(self.db_hook, method_name)
with pytest.raises(TypeError, match="only supports a single SQL string"):
method(["SELECT 1", "SELECT 2"])
self.conn.execute.assert_not_called()

@pytest.mark.parametrize("method_name", ["get_records", "get_first"])
def test_get_records_and_get_first_reject_non_mapping_parameters(self, method_name):
method = getattr(self.db_hook, method_name)
with pytest.raises(TypeError, match="only supports named/dict-style query parameters"):
method("SELECT 1", parameters=("param1", "param2"))
self.conn.execute.assert_not_called()

@mock.patch("airflow.providers.common.sql.hooks.sql.send_sql_hook_lineage")
def test_get_df_hook_lineage(self, mock_send_lineage):
statement = "SQL"
Expand Down Expand Up @@ -334,3 +355,8 @@ def test_get_df_pandas(self):
def test_get_df_polars(self):
with pytest.raises(NotImplementedError):
self.db_hook.get_df("SQL", df_type="polars")

def test_get_df_pandas_rejects_non_mapping_parameters(self):
with pytest.raises(TypeError, match="only supports named/dict-style query parameters"):
self.db_hook.get_df("SQL", parameters=("param1", "param2"), df_type="pandas")
self.conn.export_to_pandas.assert_not_called()
Loading
Loading