Copilot commented on code in PR #64610:
URL: https://github.com/apache/airflow/pull/64610#discussion_r3027588771
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -267,6 +268,74 @@ def depends_search(
return depends_search
+class _RegexParam(BaseParam[str]):
+ """Filter using database-level regex matching (regexp_match).
+
+ On SQLite, falls back to exact-match (``==``) because SQLite does not
+ natively support ``regexp_match``.
+ """
+
+ def __init__(self, attribute: ColumnElement, skip_none: bool = True) ->
None:
+ super().__init__(skip_none=skip_none)
+ self.attribute: ColumnElement = attribute
+
+ def to_orm(self, select: Select) -> Select:
+ if self.value is None and self.skip_none:
+ return select
+ from airflow.settings import engine
+
+ if engine and engine.dialect.name == "sqlite":
+ return select.where(self.attribute == self.value)
+ return select.where(self.attribute.regexp_match(self.value))
+
+ @classmethod
+ def depends(cls, *args: Any, **kwargs: Any) -> Self:
+ raise NotImplementedError("Use regex_param_factory instead, depends is
not implemented.")
+
+
+def _validate_regex_pattern(value: str | None) -> str | None:
+ """Validate that the regex pattern is syntactically correct.
+
+ On SQLite the value is used as an exact-match string, so no regex
+ validation is performed.
+ """
+ if value is None:
+ return value
+ import re
+
+ from airflow.settings import engine
+
+ if engine and engine.dialect.name == "sqlite":
+ return value
Review Comment:
New code introduces local imports inside functions (`from airflow.settings
import engine` in `to_orm()` and `import re`/`from airflow.settings import
engine` in `_validate_regex_pattern`). Airflow guidelines prefer module-level
imports unless there is a clear need (circular import/lazy-load); please move
these imports to the top of the module or add justification for keeping them
local.
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -685,6 +686,8 @@ def get(
common_params["ascending"] = ascending
if limit:
common_params["limit"] = limit
+ if partition_key is not None:
+ common_params["partition_key_pattern"] = partition_key
if name or uri:
Review Comment:
SDK client parameter name `partition_key` is forwarded as the HTTP query
param `partition_key_pattern` (regex). The current name is ambiguous (suggests
exact match) and differs from the server/API naming. Consider renaming the SDK
argument (and corresponding supervisor/comms fields) to `partition_key_pattern`
(or similar) to make the regex semantics explicit and keep naming consistent
across layers.
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -267,6 +268,74 @@ def depends_search(
return depends_search
+class _RegexParam(BaseParam[str]):
+ """Filter using database-level regex matching (regexp_match).
+
+ On SQLite, falls back to exact-match (``==``) because SQLite does not
+ natively support ``regexp_match``.
+ """
+
+ def __init__(self, attribute: ColumnElement, skip_none: bool = True) ->
None:
+ super().__init__(skip_none=skip_none)
+ self.attribute: ColumnElement = attribute
+
+ def to_orm(self, select: Select) -> Select:
+ if self.value is None and self.skip_none:
+ return select
+ from airflow.settings import engine
+
+ if engine and engine.dialect.name == "sqlite":
+ return select.where(self.attribute == self.value)
+ return select.where(self.attribute.regexp_match(self.value))
+
+ @classmethod
+ def depends(cls, *args: Any, **kwargs: Any) -> Self:
+ raise NotImplementedError("Use regex_param_factory instead, depends is
not implemented.")
+
+
+def _validate_regex_pattern(value: str | None) -> str | None:
+ """Validate that the regex pattern is syntactically correct.
+
+ On SQLite the value is used as an exact-match string, so no regex
+ validation is performed.
+ """
+ if value is None:
+ return value
+ import re
+
+ from airflow.settings import engine
+
+ if engine and engine.dialect.name == "sqlite":
+ return value
+ try:
+ re.compile(value)
+ except re.error as e:
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail=f"Invalid regular expression: {e}",
+ )
+ return value
+
+
+def regex_param_factory(
+ attribute: ColumnElement,
+ pattern_name: str,
+ skip_none: bool = True,
+) -> Callable[[str | None], _RegexParam]:
+ DESCRIPTION = (
+ "Partition key filter. Uses database-native regex "
+ "(PostgreSQL ~ operator, MySQL REGEXP). On SQLite, only exact match is
supported."
+ )
+
+ def depends_regex(
+ value: str | None = Query(alias=pattern_name, default=None,
description=DESCRIPTION),
+ ) -> _RegexParam:
+ _validate_regex_pattern(value)
+ return _RegexParam(attribute, skip_none).set_value(value)
+
+ return depends_regex
Review Comment:
`regex_param_factory()` is written as a reusable helper (it accepts an
arbitrary `attribute` and `pattern_name`), but the query description is
hard-coded to “Partition key filter…”. This will be misleading if/when the
factory is reused. Consider passing `description` into the factory (or using a
generic regex description) and letting callers specialize it.
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -267,6 +268,74 @@ def depends_search(
return depends_search
+class _RegexParam(BaseParam[str]):
+ """Filter using database-level regex matching (regexp_match).
+
+ On SQLite, falls back to exact-match (``==``) because SQLite does not
+ natively support ``regexp_match``.
+ """
+
+ def __init__(self, attribute: ColumnElement, skip_none: bool = True) ->
None:
+ super().__init__(skip_none=skip_none)
+ self.attribute: ColumnElement = attribute
+
+ def to_orm(self, select: Select) -> Select:
+ if self.value is None and self.skip_none:
+ return select
+ from airflow.settings import engine
+
+ if engine and engine.dialect.name == "sqlite":
+ return select.where(self.attribute == self.value)
+ return select.where(self.attribute.regexp_match(self.value))
+
Review Comment:
`_RegexParam.to_orm()` will call `regexp_match(self.value)` when `skip_none`
is False and `value` is None, which can produce incorrect SQL/behavior (regex
against NULL) on non-SQLite backends. Consider handling the `value is None`
case explicitly (e.g. `IS NULL`/`== None`) before applying regex matching,
regardless of dialect.
##########
airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts:
##########
@@ -2301,6 +2301,10 @@ export type GetAssetEventsData = {
* Attributes to order by, multi criteria sort is supported. Prefix with
`-` for descending order. Supported attributes: `source_task_id, source_dag_id,
source_run_id, source_map_index, timestamp`
*/
orderBy?: Array<(string)>;
+ /**
+ * Regular expression pattern for filtering. Uses database-native regex
(PostgreSQL ~ operator, MySQL REGEXP). Not supported on SQLite.
+ */
+ partitionKeyPattern?: string | null;
Review Comment:
This file is marked as auto-generated (`@hey-api/openapi-ts`). Please ensure
the new `partitionKeyPattern` field comes from regenerating the OpenAPI client
artifacts rather than manual edits, and document/keep the generation command in
sync so future regenerations don’t drop these changes.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
