fix(timeshift): Add a more reliable strategy for correct temporal col #36309
Conversation
- Fix function name collision in test_query_context_processor.py - Remove trailing whitespace in helpers_test.py - Update test expectations to reflect removal of datasource column fallback - Fix line length and formatting issues 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
| elif isinstance(col, dict) and col.get("sqlExpression"): | ||
| return str(col.get("label") or col.get("sqlExpression")) |
There was a problem hiding this comment.
When would col be a dict? From my tests, if you use custom SQL to create the original time range filter, the chart won't allow you to add a time comparison config. You can use a calculated column, but looking at queries[0].filter I still see this in the payload:
{col: "${COLUMN_NAME}", op: "TEMPORAL_RANGE", val: "2005 : 2006"}
Is this scenario a precautionary measure, or something you were able to reproduce? I'm asking it because I noticed you're looking for label or sqlExpression, and I was wondering if we should look for name too (which I think it depends if this is for a particular calculated column flow I couldn't reproduce, or specifically for custom SQL in the filter).
There was a problem hiding this comment.
Hey, AdhocColumn type is a dict containing label and no name. That's why i am checking it here. If the payload doesn't match the type we should do the update there but i don't think that would be the case.
class AdhocColumn(TypedDict, total=False):
hasCustomLabel: bool | None
label: str
sqlExpression: str
isColumnReference: bool | None
columnType: Literal["BASE_AXIS", "SERIES"] | None
timeGrain: str | None
…apache#36309) Co-authored-by: Claude <[email protected]>
…#36309) Co-authored-by: Claude <[email protected]> (cherry picked from commit a754258)
2 participants
SUMMARY
TESTING INSTRUCTIONS
Tests should pass.
ADDITIONAL INFORMATION