Fix #918: support Time (TimeInterval) identifiers in DuckDB time operators - #958
Merged
Conversation
…ators The DuckDB transpiler only recognised Date and Time_Period identifiers, so _resolve_time_identifier returned None for a Time (TimeInterval) identifier and flow_to_stock, stock_to_flow, timeshift and fill_time_series all failed with "cannot unpack non-iterable NoneType object". A TimeInterval series is characterised by the duration of one interval, which the pandas engine reports as Y/S/Q/M/W/D or a P<n>Y<n>M<n>D fallback. Both forms reduce to a (months, days) offset, so the new SQL macros carry the frequency as a STRUCT and materialise it as a native INTERVAL for generate_series and for date arithmetic. A STRUCT rather than an INTERVAL because DuckDB normalises a month to 30 days when comparing intervals. Two DuckDB-vs-pandas divergences on the Date path are fixed alongside: timeshift no longer snaps a month end to the month end of the target month, and flow_to_stock/stock_to_flow no longer accumulate number attributes.
All five surfaced while building the cross-backend parity fixtures for the TimeInterval work, and all five are in the pandas engine: - fill_time_series with the all limits method lost the Data Points that widen each series to the global range whenever the Data Set had an Attribute: the boundary Data Point it appended carried row 0's Attribute values, so it no longer collapsed onto the grid Data Point and the duplicate filter removed both. The interval fill now adds only the Data Points whose key is missing, with no values at all, so there is nothing to deduplicate. - That same rewrite keeps the operand's own intervals when they overlap. The two endpoint grids then come out different lengths, and joining them positionally used to rewrite the operand into intervals it never held. - fill_time_series with the single limits method looked its per-series limits up by the string representation of the other identifiers, so any identifier that was not a String raised KeyError. - A Data Set may hold a single series, with the time identifier as its only identifier. flow_to_stock, stock_to_flow and fill_time_series then have nothing to group by, which raised a Pandas ValueError on every time type. - A Time value may carry a time component, which the input format allows. Measuring an interval's duration read the whole endpoint as a date and raised ValueError; the duration now reads the date part, and the intervals fill_time_series adds keep the operand's representation, time of day included, on both engines.
javihern98
marked this pull request as ready for review
August 3, 2026 10:13
javihern98
requested review from
a team,
albertohernandez1995 and
mla2001
and removed request for
mla2001
August 3, 2026 10:13
2 tasks
albertohernandez1995
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On the DuckDB backend, every time operator crashed with
TypeError: cannot unpack non-iterable NoneType objectwhen the Data Set's time identifier was declared"type": "Time"(VTLTimeInterval, e.g.2001-01-01/2001-12-31)._resolve_time_identifieronly matchedDateandTimePeriod, so it fell off the end and returnedNoneto its three unpacking call sites. The pandas engine handled these Data Sets fine, so the two backends silently diverged.flow_to_stock,stock_to_flow,timeshiftandfill_time_seriesnow return on DuckDB exactly what the pandas engine returns. Building the cross-backend fixtures then surfaced seven more defects — two on the DuckDB Date path and five in the pandas engine — all fixed here.1. TimeInterval support on the DuckDB backend
How the frequency is derived. A
TimeIntervalseries is characterised by the duration of a single interval, which the pandas engine (Time._classify_interval_period) reports asY/S/Q/M/W/D, or as aP<n>Y<n>M<n>Dfallback for spans with no canonical period indicator. Both forms reduce to the same thing — a(months, days)offset — so the new macros carry the frequency as aSTRUCTand materialise it as a native DuckDBINTERVALforgenerate_seriesand for date arithmetic. That covers the non-canonical durations (P2Y,P7M,P1Y2M,P14D, …) with no special casing.Note
The frequency is a
STRUCTand not anINTERVALbecause DuckDB normalises a month to 30 days when comparing intervals:INTERVAL 30 DAY = INTERVAL 1 MONTHevaluates true, which silently misclassifies2020-01-01/2020-01-31and2020-01-01/2020-03-31.sql/time_operators.sql—vtl_interval_start_date/_end_date/_start_ts/_end_ts,vtl_interval_months/_days,vtl_interval_is_canonical,vtl_interval_nonzero,vtl_interval_freq,vtl_interval_freq_to_step,vtl_interval_step,vtl_interval_build,vtl_interval_shift.Transpiler/__init__.py—TimeIntervaladded to_resolve_time_identifier; newTimeIntervalbranch invisit_BinOp_timeshift; new_fill_time_series_interval, which steps the two endpoints separately and pairs the k-th of each, mirroringFill_time_series.fill_time_intervals.io/_execution.py— a Data Set whose intervals do not share one frequency raisesSemanticError 1-1-19-9, as the pandas engine does.flow_to_stock/stock_to_flowneeded no new SQL: for aTimeIntervalidentifier they partition by the other identifiers exactly as they do forDate, and the interval strings sort chronologically.2. DuckDB Date path
timeshiftsnapped a month end to the month end of the target month.pd.DateOffsetclamps but never snaps, so2020-02-29shifted by one month is2020-03-29, not2020-03-31.flow_to_stock/stock_to_flowaccumulated number attributes; only number measures accumulate (the Reference Manual types the operand asmeasure<number>, cf. Stock to flow weird error (DuckDb) #931).3. pandas engine
fill_time_series(ds, all)fill_time_series(ds, single)2020-01-01/2020-01-31+2020-01-31/2020-02-29became2020-01-01/2020-02-29+2020-01-31/2020-01-31fill_time_series(ds, single)KeyErroron any non-String identifierflow_to_stock,stock_to_flow,fill_time_seriesValueError: No group keys passed!, on every time typefill_time_series,timeshiftValueError: Invalid isoformat stringon theTform the input format allowsThe first two share a fix: the interval fill now adds only the Data Points whose key the operand is missing, each carrying no values at all. That removes the appended-boundary-Data-Point and duplicate-filter machinery the old code needed, so there is nothing left to deduplicate wrongly, and the operand's own Data Points are never touched. The intervals it adds keep the operand's representation, time of day included, on both engines.
Checklist
ruff format,ruff check,mypy)pytest)docs/duckdb_engine.rstdoes not enumerate per-operator/type support, so nothing to updateImpact / Risk
TypeError,KeyErrororValueErrornow produce results, and the DuckDB Date path now matches the pandas engine.fill_time_serieschange is the one to review closely:allmode over a Data Set with Attributes now returns more Data Points than before (the ones it should always have returned), and overlapping intervals are preserved rather than rewritten.VTL_ENGINE_BACKEND=duckdb5507 passed / 27 skipped,VTL_ENGINE_BACKEND=pandas4899 passed / 635 skipped.Notes
Verified against the pandas engine throughout: the frequency macro matches all 24 cases of the existing
test_classify_interval_periodspec, and a 12-frequency × 6-operator matrix (canonical, month-end, mid-year and non-canonicalP2Y/P7M/P14Dseries) matches row for row, as do nested calls such asflow_to_stock(fill_time_series(...)).Tests, all file-based under
tests/Bugsso CI pins both backends against the same references:GH_918_1TimeidentifierGH_918_2GH_918_3fill_time_serieswith an AttributeGH_918_4fill_time_series(single)with an Integer identifierGH_918_5Timeidentifier onlyGH_918_6Dateidentifier onlyGH_918_7GH_918_8Plus
timeshift-on-interval and mixed-frequency cases intests/DateTime, and macro unit tests intests/duckdb_transpiler.test_fill_time_series_interval_uniform_frequencynow also runs on DuckDB — it never did, which is why the original bug went unnoticed.Fixes #918