bito-code-review[bot] commented on PR #41895:
URL: https://github.com/apache/superset/pull/41895#issuecomment-4918370563
<!-- Bito Reply -->
The flagged performance issue is correct. The current implementation of
`map_big_number_config` performs redundant dataset lookups when
`temporal_column` is omitted, as both `_find_dataset_main_dttm_col` and
`is_column_truly_temporal` independently fetch the dataset.
To resolve this, you should fetch the dataset once at the beginning of the
function and pass it to the helper functions. Below is the corrected
implementation for `superset/mcp_service/chart/chart_utils.py`:
```python
def map_big_number_config(
config: BigNumberChartConfig, dataset_id: int | str | None = None
) -> Dict[str, Any]:
# ... (existing code)
dataset = _find_dataset_by_id_or_uuid(dataset_id)
main_dttm_col = dataset.main_dttm_col if dataset else None
temporal_column = config.temporal_column or main_dttm_col
if temporal_column and is_column_truly_temporal(temporal_column,
dataset_id):
_ensure_temporal_adhoc_filter(form_data, temporal_column)
return form_data
```
Would you like me to check the remaining comments on this PR and implement
fixes for them as well?
**superset/mcp_service/chart/chart_utils.py**
```
dataset = _find_dataset_by_id_or_uuid(dataset_id)
main_dttm_col = dataset.main_dttm_col if dataset else None
temporal_column = config.temporal_column or main_dttm_col
if temporal_column and is_column_truly_temporal(temporal_column,
dataset_id):
_ensure_temporal_adhoc_filter(form_data, temporal_column)
```
--
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]
