bito-code-review[bot] commented on PR #43130:
URL: https://github.com/apache/superset/pull/43130#issuecomment-5283203489
<!-- Bito Reply -->
The flagged issue is correct. The current implementation in
`FilterConfig.validate_value` silently overwrites any provided `value` with
`None` when the operator is `IS NULL` or `IS NOT NULL`, rather than rejecting
invalid payloads that include a value. To resolve this, the validator should
raise a `ValueError` if a `value` is provided for these operators.
Here is the corrected implementation for
`superset/mcp_service/chart/schemas.py`:
```python
@model_validator(mode="after")
def validate_value(self) -> "FilterConfig":
"""Null checks have no comparator; every other operator requires
one."""
if self.op in {"IS NULL", "IS NOT NULL"}:
if self.value is not None:
raise ValueError(f"Filter operator {self.op!r} must not have
a 'value'.")
elif self.value is None:
raise ValueError(f"Filter operator {self.op!r} requires
'value'.")
return self
```
There are no other comments on this PR to address.
**superset/mcp_service/chart/schemas.py**
```
@model_validator(mode="after")
def validate_value(self) -> "FilterConfig":
"""Null checks have no comparator; every other operator requires
one."""
if self.op in {"IS NULL", "IS NOT NULL"}:
if self.value is not None:
raise ValueError(f"Filter operator {self.op!r} must not have
a 'value'.")
elif self.value is None:
raise ValueError(f"Filter operator {self.op!r} requires
'value'.")
return self
```
--
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]
