Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20260107-163537.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Fix time period end boundaries to include the full end date
time: 2026-01-07T16:35:37.657489-07:00
custom:
Author: akuttig-block
Issue: "1957"
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def adjust_to_end_of_period(
# Strip time data that is irrelevant to avoid rendering an unnecessarily complex time constraint.
date_to_adjust = date_to_adjust.replace(microsecond=0)
if time_granularity.to_int() >= TimeGranularity.DAY.to_int():
date_to_adjust = date_to_adjust.replace(hour=0, minute=0, second=0)
date_to_adjust = date_to_adjust.replace(hour=23, minute=59, second=59)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of these extra values is causing the rendered SQL to change in cases where the output expression was correct (i.e., for anything >= DAY grain). We'll need to consider the best approach here. This may be it, although I'll need to recreate a bunch of snapshots and run further tests before we can get this merged, but there are some other options which might be less sensitive to the underlying column granularity (i.e., in the warehouse rather than in the MetricFlow model spec).


if (
time_granularity is TimeGranularity.NANOSECOND
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ParseQueryResult(
),
time_range_constraint=TimeRangeConstraint(
start_time=datetime.datetime(2020, 1, 15, 0, 0),
end_time=datetime.datetime(2020, 2, 15, 0, 0),
end_time=datetime.datetime(2020, 2, 15, 23, 59, 59),
),
filter_intersection=PydanticWhereFilterIntersection(
where_filters=[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ParseQueryResult(
),
time_range_constraint=TimeRangeConstraint(
start_time=datetime.datetime(2020, 1, 1, 0, 0),
end_time=datetime.datetime(2020, 2, 29, 0, 0),
end_time=datetime.datetime(2020, 2, 29, 23, 59, 59),
),
filter_intersection=PydanticWhereFilterIntersection(),
filter_spec_resolution_lookup=FilterSpecResolutionLookUp(),
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,38 @@ def date_times_to_check() -> Sequence[datetime.datetime]: # noqa: D103
current_date_time += datetime.timedelta(days=1)
if current_date_time == end_date_time:
break

# Add specific times throughout the day to test sub-day granularities (hour, minute, second)
# Test various hours, minutes, and seconds across different dates
test_date = datetime.datetime(year=2021, month=6, day=15)
times_to_test = [
(0, 0, 0), # Midnight
(0, 0, 30), # 30 seconds past midnight
(0, 30, 0), # 30 minutes past midnight
(0, 30, 45), # 30 minutes 45 seconds past midnight
(12, 0, 0), # Noon
(12, 30, 30), # 12:30:30 PM
(23, 59, 59), # Last second of day
(1, 1, 1), # 1:01:01 AM
(13, 45, 22), # 1:45:22 PM
]
for hour, minute, second in times_to_test:
date_times.append(
datetime.datetime(
year=test_date.year, month=test_date.month, day=test_date.day, hour=hour, minute=minute, second=second
)
)

return date_times


@pytest.fixture(scope="session")
def grain_to_count_in_year() -> Dict[TimeGranularity, int]:
"""Returns the maximum number of times the given item occurs in a year."""
return {
TimeGranularity.SECOND: 31622400, # 366 days * 24 hours * 60 minutes * 60 seconds
TimeGranularity.MINUTE: 527040, # 366 days * 24 hours * 60 minutes
TimeGranularity.HOUR: 8784, # 366 days * 24 hours
TimeGranularity.DAY: 366,
TimeGranularity.WEEK: 53,
TimeGranularity.MONTH: 31,
Expand All @@ -54,10 +79,36 @@ def test_start_and_end_periods( # noqa: D103
rows: List[Tuple[str, ...]] = []
for date_time in date_times_to_check:
for time_granularity in TimeGranularity:
if time_granularity.to_int() < TimeGranularity.DAY.to_int():
# Skip sub-second granularities as they are not supported
if time_granularity.to_int() < TimeGranularity.SECOND.to_int():
continue
dateutil_start_of_period = dateutil_adjuster.adjust_to_start_of_period(time_granularity, date_time)
dateutil_end_of_period = dateutil_adjuster.adjust_to_end_of_period(time_granularity, date_time)

# Validate that end of period is actually at the end, not the beginning
# For all granularities, the end should be >= start
assert dateutil_end_of_period >= dateutil_start_of_period, (
f"End of period {dateutil_end_of_period.isoformat()} should be >= start {dateutil_start_of_period.isoformat()} "
f"for {time_granularity.name} granularity on {date_time.isoformat()}"
)

# For MINUTE and larger granularities, the end time should always be the last second of the period
if time_granularity.to_int() >= TimeGranularity.MINUTE.to_int():
assert dateutil_end_of_period.second == 59, (
f"End of {time_granularity.name} period should have second=59, "
f"got {dateutil_end_of_period.isoformat()} for input {date_time.isoformat()}"
)
if time_granularity.to_int() >= TimeGranularity.HOUR.to_int():
assert dateutil_end_of_period.minute == 59, (
f"End of {time_granularity.name} period should have minute=59, "
f"got {dateutil_end_of_period.isoformat()} for input {date_time.isoformat()}"
)
if time_granularity.to_int() >= TimeGranularity.DAY.to_int():
assert dateutil_end_of_period.hour == 23, (
f"End of {time_granularity.name} period should have hour=23, "
f"got {dateutil_end_of_period.isoformat()} for input {date_time.isoformat()}"
)

rows.append(
(
date_time.isoformat(),
Expand Down
Loading