fix(gsheets): correctly format Date-column filter literals - #43355
Conversation
GSheetsEngineSpec inherited SqliteEngineSpec.convert_dttm, which returns None for types.Date, so Superset fell back to a datetime-with-microseconds literal for Date-typed filter bounds. shillelagh's virtual table layer parses that value with datetime.date.fromisoformat, which rejects the trailing time and silently drops it to None, causing the GSheets adapter to embed a bare, unquoted `null` literal in the query sent to Google's API instead of a real value -- producing "Invalid query: NO_COLUMN: null". Override convert_dttm on GSheetsEngineSpec to emit a plain 'YYYY-MM-DD' literal for Date columns, which shillelagh can parse and quote correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a standalone, runnable script plus its captured before/after output as an appendix, and point every "verified" claim in the root-cause trace at it, so the analysis is auditable from the tracked file alone rather than resting on unrecorded local runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43355 +/- ##
==========================================
- Coverage 66.74% 66.73% -0.02%
==========================================
Files 2876 2876
Lines 164228 164226 -2
Branches 37891 37889 -2
==========================================
- Hits 109617 109593 -24
- Misses 52453 52474 +21
- Partials 2158 2159 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #1a1b1bActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
LGTM. Narrow, well-scoped fix — only intercepts types.Date, so the working DateTime path through SqliteEngineSpec.convert_dttm is untouched. Test reuses the shared assert_convert_dttm helper and covers Date/DateTime/unknown-type, and I confirmed it fails on master per the PR description. CI's green.
2 participants
SUMMARY
Filtering a Date-typed column on a Google Sheets-backed dataset (for example, a built-in "Previous Calendar Month" time range) raised
Invalid query: NO_COLUMN: null.GSheetsEngineSpecinheritedSqliteEngineSpec.convert_dttm, which only handlesString/DateTimeSQLAlchemy types and returnsNoneforDate. Superset then fell back to a full'YYYY-MM-DD HH:MM:SS.ffffff'literal for the filter bound, even for a pure Date column. Shillelagh'sISODateparser (used internally by the GSheets adapter to turn the SQL literal into a filter value) rejects the trailing time-of-day and silently returnsNone, which the adapter then renders as the bare, unquoted literalnullin the generated query. Google's API parses that bareword as a column reference rather than a NULL value, producing exactly the reported error.This adds a
GSheetsEngineSpec.convert_dttmoverride that emits a plain'YYYY-MM-DD'literal for Date columns (noTO_DATE()-style wrapper needed, since shillelagh's query layer is SQLite/apsw, not Postgres), matching the format the adapter actually parses. DateTime columns are unaffected — they already go through the inherited, working code path.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend query-generation fix, no UI change.
TESTING INSTRUCTIONS
pytest tests/unit_tests/db_engine_specs/test_gsheets.py -k test_convert_dttm— new parametrized test covering Date, DateTime, and an unrecognized type. The Date case fails onmaster(AssertionError: None) and passes with this change.tests/unit_tests/db_engine_specs/andtests/unit_tests/models/helpers_test.pysuites pass with no new failures.ADDITIONAL INFORMATION
NO_COLUMN: nullerror reported for Google Sheets dashboard filters generally; this fixes the mechanism for Date-typed columns specifically)