codeant-ai-for-open-source[bot] commented on code in PR #35662:
URL: https://github.com/apache/superset/pull/35662#discussion_r4153810783
##########
superset/daos/dataset.py:
##########
@@ -48,6 +48,13 @@
}
+class _Unset:
+ """Sentinel indicating that no filter should be applied."""
+
+
+_UNSET = _Unset()
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `e596b5e`.
The module-level sentinel is no longer declared in `dataset.py`; it now
imports the shared `_UNSET` sentinel together with its `_Unset` type.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset/jinja_context.py:
##########
@@ -1101,28 +1104,91 @@ def get_template_processor(
def dataset_macro(
- dataset_id: int,
+ dataset_id: Union[int, str],
include_metrics: bool = False,
columns: list[str] | None = None,
from_dttm: datetime | None = None,
to_dttm: datetime | None = None,
+ schema: str | None = None,
+ catalog: str | None = None,
+ database_id: Union[int, str] | None = None,
+ alias: str | None = None,
) -> str:
"""
- Given a dataset ID, return the SQL that represents it.
+ Given a dataset ID or name, return the SQL that represents it.
+
+ If ``dataset_id`` is an integer, it is treated as the unique dataset ID and
+ the optional ``schema``, ``catalog`` and ``database_id`` parameters are
+ ignored.
+
+ If ``dataset_id`` is a string, it is treated as a dataset name. The
optional
+ ``schema``, ``catalog`` and ``database_id`` parameters are used to narrow
+ down the search when provided. If multiple datasets match the provided
+ criteria, an error is raised because the dataset name is ambiguous.
The generated SQL includes all columns (including computed) by default.
Optionally
the user can also request metrics to be included, and columns to group by.
- The from_dttm and to_dttm parameters are filled in from filter values in
explore
- views, and we take them to make those properties available to jinja
templates in
- the underlying dataset.
+ The ``from_dttm`` and ``to_dttm`` parameters are filled in from filter
values in
+ explore views, and we take them to make those properties available to jinja
+ templates in the underlying dataset.
+
+ The ``alias`` parameter allows the user to specify an explicit alias for
the
+ returned subquery.
"""
# pylint: disable=import-outside-toplevel
+ from sqlalchemy.orm.exc import MultipleResultsFound
+
from superset.daos.dataset import DatasetDAO
- dataset = DatasetDAO.find_by_id(dataset_id)
+ filters = {
+ key: value
+ for key, value in {
+ "database_id": database_id,
+ "catalog": catalog,
+ "schema": schema,
+ }.items()
+ if value is not None
+ }
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `e596b5e`.
The unannotated `filters` mapping is no longer present; the context entries
are now passed directly as a mapping to `self._context.update()`.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset/jinja_context.py:
##########
@@ -1101,28 +1104,91 @@ def get_template_processor(
def dataset_macro(
- dataset_id: int,
+ dataset_id: Union[int, str],
include_metrics: bool = False,
columns: list[str] | None = None,
from_dttm: datetime | None = None,
to_dttm: datetime | None = None,
+ schema: str | None = None,
+ catalog: str | None = None,
+ database_id: Union[int, str] | None = None,
+ alias: str | None = None,
) -> str:
"""
- Given a dataset ID, return the SQL that represents it.
+ Given a dataset ID or name, return the SQL that represents it.
+
+ If ``dataset_id`` is an integer, it is treated as the unique dataset ID and
+ the optional ``schema``, ``catalog`` and ``database_id`` parameters are
+ ignored.
+
+ If ``dataset_id`` is a string, it is treated as a dataset name. The
optional
+ ``schema``, ``catalog`` and ``database_id`` parameters are used to narrow
+ down the search when provided. If multiple datasets match the provided
+ criteria, an error is raised because the dataset name is ambiguous.
The generated SQL includes all columns (including computed) by default.
Optionally
the user can also request metrics to be included, and columns to group by.
- The from_dttm and to_dttm parameters are filled in from filter values in
explore
- views, and we take them to make those properties available to jinja
templates in
- the underlying dataset.
+ The ``from_dttm`` and ``to_dttm`` parameters are filled in from filter
values in
+ explore views, and we take them to make those properties available to jinja
+ templates in the underlying dataset.
+
+ The ``alias`` parameter allows the user to specify an explicit alias for
the
+ returned subquery.
"""
# pylint: disable=import-outside-toplevel
+ from sqlalchemy.orm.exc import MultipleResultsFound
+
from superset.daos.dataset import DatasetDAO
- dataset = DatasetDAO.find_by_id(dataset_id)
+ filters = {
+ key: value
+ for key, value in {
+ "database_id": database_id,
+ "catalog": catalog,
+ "schema": schema,
+ }.items()
+ if value is not None
+ }
+
+ if isinstance(dataset_id, str):
+ try:
+ dataset = DatasetDAO.get_table_by_catalog_schema_and_name(
+ table_name=dataset_id,
+ **cast(
+ dict[str, Any],
+ filters,
+ ),
+ )
+ except MultipleResultsFound as ex:
+ raise DatasetInvalidError(
+ f"Multiple datasets named '{dataset_id}' match the provided
criteria. "
+ "Please specify additional qualifiers such as schema, catalog,
"
+ "or database_id to identify a unique dataset."
+ ) from ex
+ else:
+ if filters:
+ logger.warning(
+ "Ignoring parameters %s when resolving dataset_id=%r by ID.",
+ ", ".join(filters.keys()),
+ dataset_id,
+ extra={
+ "macro": "dataset",
+ "dataset_id": dataset_id,
+ "ignored_parameters": list(filters.keys()),
+ "warning_type": "JINJA_MACRO_IGNORED_PARAMETERS",
+ },
+ )
+
+ dataset = DatasetDAO.find_by_id(dataset_id)
+
if not dataset:
- raise DatasetNotFoundError(f"Dataset {dataset_id} not found!")
+ criteria = [
+ f"{dataset_id!r}",
+ *[f"{key}={value!r}" for key, value in filters.items()],
+ ]
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `e596b5e`.
The unannotated `criteria` list is absent from the current code; schema and
table values are now handled directly in `_schema_table()`.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
