[GH-3110] Fix STAC Client.search(datetime="YYYY-mm") raising TypeError - #3122
Merged
Conversation
…peError The YYYY-mm branch of CollectionClient._expand_date relied on a bogus import `from pyspark.sql.types import dt`, which resolves to None, so `dt(...)` raised `TypeError: 'NoneType' object is not callable`. The old arithmetic also overflowed for December (int(month) + 1 == 13). Drop the erroneous import and compute the last day of the month with calendar.monthrange, which handles December and leap years correctly. Also make whole-period upper bounds inclusive of the final fractional second. Sedona stores STAC datetimes as timestamps and filters them with an inclusive `datetime <= end` bound, so a rounded `23:59:59Z` end drops items that fall in the last fraction of a second (e.g. `23:59:59.5Z`). Expand YYYY, YYYY-mm, and YYYY-mm-dd periods to end at `23:59:59.999999Z`, matching Spark's microsecond timestamp precision. Add a filter-level regression test that runs the real Spark temporal filter over sub-second timestamps, alongside the _expand_date unit test. Closes apache#3110
jiayuasu
force-pushed
the
fix/stac-expand-date-yyyy-mm
branch
from
July 19, 2026 06:00
38c807c to
df3c5f7
Compare
…sh-down The in-memory temporal filter already ends whole periods at 23:59:59.999999Z, but the remote push-down path truncated that bound to milliseconds, so a real STAC scan could drop items in the final fraction of a second before Spark's residual filter ran. Two truncation points are fixed: - SpatialTemporalFilterPushDownForStacScan converted the Spark TimestampType literal (microseconds since epoch) with `Instant.ofEpochMilli(v / 1000)`, discarding sub-millisecond precision. Replace it with a helper that keeps all six digits via floorDiv/floorMod (also correct for pre-epoch timestamps). - StacUtils.getFilterTemporal serialized the pushed bound with a millisecond (.SSS) pattern. Emit six fractional digits (.SSSSSS) to match Spark's TimestampType precision, so the remote request bound equals the residual filter bound. Add a push-down serialization test asserting the Catalyst-predicate to remote URL path preserves microseconds (the in-memory .5Z test bypasses push-down), update the getFilterTemporal expectations to six digits, and refresh the Client.search datetime docstring to show the .999999Z bounds.
…mestamps STAC permits timestamps with up to nine fractional digits, but Spark's TimestampType keeps only microseconds. Spark therefore truncates a legal item at ...999999500Z down to ...999999, which the residual `datetime <= end` filter retains -- yet the pushed-down remote bound of ...999999Z excluded it, so the remote catalog dropped an item Spark would have kept. Serialize the pushed temporal bound with nine fractional digits and widen the inclusive upper bound (from LessThan/LessThanOrEqual and the upper side of an equality) by 999 ns to the last nanosecond of its microsecond. The lower bound stays exact; a slightly wider remote window is always safe because Spark's residual filter re-checks each row at microsecond precision, so the remote request now returns a superset of the residual result. Add a 7-to-9-digit push-down regression test and update the existing getFilterTemporal expectations to the nine-digit, widened bounds.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes STAC datetime handling across the Python client and Spark-side STAC pushdown so that partial-date inputs (e.g. YYYY-mm) expand correctly and temporal bounds preserve microsecond precision end-to-end, avoiding dropped items at period boundaries.
Changes:
- Python: fix
_expand_dateforYYYY-mm(remove erroneousdtimport, compute month end viacalendar.monthrange, and use23:59:59.999999Zinclusive upper bounds). - Spark/Scala: preserve microseconds when converting Spark
TimestampTypeliterals for STAC temporal pushdown and widen inclusive upper bounds to the last nanosecond within the final microsecond for remote filtering. - Tests: add/extend Scala and Python test coverage for month-end/leap-year expansion and boundary/fractional-second inclusion.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/optimization/SpatialTemporalFilterPushDownForStacScan.scala | Convert Spark timestamp literals from micros to LocalDateTime without truncating to milliseconds. |
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/io/stac/StacUtils.scala | Serialize temporal filters with 9 fractional digits and widen inclusive upper bounds by +999 ns. |
| spark/common/src/test/scala/org/apache/spark/sql/sedona_sql/optimization/SpatialTemporalFilterPushDownForStacScanTest.scala | New tests validating microsecond preservation and widened upper-bound pushdown behavior. |
| spark/common/src/test/scala/org/apache/spark/sql/sedona_sql/io/stac/StacUtilsTest.scala | Update/add expectations to match nanosecond-precision serialization and widened upper bounds. |
| python/sedona/spark/stac/collection_client.py | Fix _expand_date YYYY-mm expansion and make whole-period upper bounds microsecond-inclusive. |
| python/sedona/spark/stac/client.py | Refresh Client.search datetime docstring to reflect microsecond-inclusive bounds. |
| python/tests/stac/test_collection_client.py | Add tests for expanded date forms and for retaining items in the final fractional second of a period. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes STAC Client.search(datetime="YYYY-mm") raises TypeError: NoneType object is not callable #3110What changes were proposed in this PR?
Calling the STAC Python
Client.search()(orCollectionClient.get_dataframe()/get_items()) with adatetimeargument inYYYY-mmform failed withRuntimeError: Failed to get filtered dataframewrappingTypeError: 'NoneType' object is not callable.Root cause:
python/sedona/spark/stac/collection_client.pyimportedfrom pyspark.sql.types import dt.pyspark.sql.typesexposes nodt, so the name resolved toNone, and theYYYY-mmbranch of_expand_datethen calleddt(...), raising theTypeError. The old arithmetic also overflowed for December (int(month) + 1== 13).This PR:
from pyspark.sql.types import dtimport.calendar.monthrange, which handles December and leap years correctly, and zero-pads the day so the emitted timestamp stays valid ISO 8601.datetime <= endbound, so a rounded23:59:59Zend silently drops items in the last fraction of a second. TheYYYY,YYYY-mm, andYYYY-mm-ddforms now expand to end at23:59:59.999999Z(Spark's microsecond timestamp precision), which is the bound the residual Spark filter uses.SpatialTemporalFilterPushDownForStacScanpreviously converted the SparkTimestampTypeliteral withInstant.ofEpochMilli(v / 1000), discarding sub-millisecond digits; it now keeps full microseconds via afloorDiv/floorModhelper....999999500Zdown to...999999, which the residualdatetime <= endfilter retains.StacUtils.getFilterTemporalnow serializes nine fractional digits and widens the inclusive upper bound (fromLessThan/LessThanOrEqualand the upper side of an equality) by 999 ns to the last nanosecond of its microsecond, so the remote catalog no longer drops items Spark would keep. The lower bound is left exact; a slightly wider remote window is always safe because the residual filter re-checks each row at microsecond precision._expand_date("2020-05")now returns["2020-05-01T00:00:00Z", "2020-05-31T23:59:59.999999Z"], consistent with the docstring and the STAC tutorial.How was this patch tested?
Python (
python/tests/stac/test_collection_client.py):test_expand_datecovers all supported forms plus the previously broken edge cases: December (2020-12-> 31), leap-year February (2020-02-> 29), and non-leap February (2021-02-> 28).test_expand_date_filter_includes_final_fractional_secondruns the real Spark temporal filter over sub-second timestamps and asserts rows at23:59:59.5Zon the last day/month/year of a period are retained while the first instant of the next period is excluded.Scala (
spark/common), all run via the two suites below (38 tests, all passing locally):SpatialTemporalFilterPushDownForStacScanTest(new) feeds Catalyst<=/>=predicates with microsecondTimestampTypeliterals through the push-down and asserts the serializeddatetime=request preserves microseconds and widens the inclusive upper bound to nine digits. Itscovers 7-to-9 digit sub-microsecond timestampscase asserts that legal 7-, 8- and 9-digit timestamps in the bound's final microsecond fall within the pushed remote bound.StacUtilsTestgains agetFilterTemporal widens the inclusive upper bound to nanosecond precisioncase; the existinggetFilterTemporal/addFiltersToUrlexpectations were updated to the nine-digit, widened bounds (exact lower bound,+999 nsupper bound).Did this PR include necessary documentation updates?
Client.searchdatetime docstring was refreshed to show the.999999Zbounds.)