Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd17384945
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Convert to ISO strings | ||
| if hasattr(min_date, "isoformat"): | ||
| min_date = min_date.isoformat()[:10] | ||
| max_date = max_date.isoformat()[:10] | ||
| else: |
There was a problem hiding this comment.
Here the computed min/max are truncated to date-only strings (isoformat()[:10]). When the time dimension is a timestamp (common), _build_filters later uses these values in a <= 'YYYY-MM-DD' predicate, which DuckDB treats as midnight. That drops all rows on the end date after 00:00, so the widget undercounts the last day. Keep the full timestamp, or make the end bound exclusive (e.g., < next_day) to avoid silently excluding data.
Useful? React with 👍 / 👎.
| if len(values) == 1: | ||
| filter_exprs.append(f"{self._model_name}.{dim_key} = '{values[0]}'") | ||
| else: | ||
| clauses = " OR ".join(f"{self._model_name}.{dim_key} = '{v}'" for v in values) | ||
| filter_exprs.append(f"({clauses})") |
There was a problem hiding this comment.
Dimension filter values are interpolated directly into SQL string literals. If a selected value contains a single quote (e.g., O'Reilly) or other special characters, the generated SQL becomes invalid and the widget errors. Since filter values come from user data, this is a realistic failure case. Use parameterized queries or escape quotes before embedding values.
Useful? React with 👍 / 👎.
1 participant
Summary:
Tests: