Skip to content

SNOW-3746497 Snowpark Python SDK: add interval type support for UDFs & Sprocs - #4291

Open
sfc-gh-jzhan wants to merge 3 commits into
mainfrom
jing/interval-udf-sdk
Open

SNOW-3746497 Snowpark Python SDK: add interval type support for UDFs & Sprocs#4291
sfc-gh-jzhan wants to merge 3 commits into
mainfrom
jing/interval-udf-sdk

Conversation

@sfc-gh-jzhan

@sfc-gh-jzhan sfc-gh-jzhan commented Jul 21, 2026

Copy link
Copy Markdown
  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-3746497

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
    • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support. Follow the link for more information: AST Support Guidelines
  3. Please describe how your code solves the related issue.
    Enables DayTimeIntervalType and YearMonthIntervalType as parameter and return types for Python UDFs and stored procedures, gated behind the ENABLE_INTERVAL_TYPES_IN_UDF account parameter.

    New user-facing behavior:

    DayTimeIntervalTypedatetime.timedelta is now a recognized annotation:

    # annotation inferred automatically — no explicit return_type/input_types needed
    def add_day(d: datetime.timedelta) -> datetime.timedelta:
        return d + datetime.timedelta(days=1)
    
    f = session.udf.register(add_day)
    # f._return_type  == DayTimeIntervalType()
    # f._input_types  == [DayTimeIntervalType()]
    
    # YearMonthIntervalType — no native Python type exists, so the YearMonthInterval
    # TypeVar must be used as the annotation (or return_type/input_types passed
    # explicitly). Inside the handler, values are plain int (total months):
    def add_year(m: YearMonthInterval) -> YearMonthInterval:
        return m + 12  # m is int at runtime, e.g. 14 for "1 year 2 months"
    
    f = session.udf.register(
         add_year,
         return_type=YearMonthIntervalType(),
         input_types=[YearMonthIntervalType()],
    )

SDK changes:

  • type_utils.py — datetime.timedelta added to PYTHON_TO_SNOW_TYPE_MAPPINGS (→ DayTimeIntervalType); "timedelta" and "YearMonthInterval" string cases added to python_type_str_to_object for register_from_file support.
  • datatype_mapper.py — to_sql() extended to serialize timedelta → INTERVAL DAY TO SECOND literal and int → INTERVAL YEAR TO MONTH literal, enabling direct sproc calls with Python values (e.g. sp(timedelta(days=3))).
  • types.py — docstrings updated to reflect UDF/sproc support and correct annotation guidance for both interval types.
  • mock/_functions.py — cast_column_to extended to pass through DayTimeIntervalType and YearMonthIntervalType columns in Local Testing mode.

@github-actions

Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 9.09091% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.53%. Comparing base (b99b89b) to head (8c7fa69).

Files with missing lines Patch % Lines
...ake/snowpark/_internal/analyzer/datatype_mapper.py 0.00% 18 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4291       +/-   ##
===========================================
- Coverage   95.46%   82.53%   -12.93%     
===========================================
  Files         171      171               
  Lines       44720    44742       +22     
  Branches     7676     7679        +3     
===========================================
- Hits        42694    36930     -5764     
- Misses       1253     5842     +4589     
- Partials      773     1970     +1197     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sfc-gh-jzhan
sfc-gh-jzhan marked this pull request as ready for review July 29, 2026 17:20
@sfc-gh-jzhan
sfc-gh-jzhan requested review from a team as code owners July 29, 2026 17:20

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Snowflake Security Review

Security grade: A — Passed

No security findings after adjudication. This PR passes the Snowflake Security Review.

📊 3 of 6 files (3 tests skipped) · 4,030 lines reviewed · 3 candidates → 3 kept · retrieval: on

if isinstance(value, str) and isinstance(datatype, DayTimeIntervalType):
return f"{str_to_sql_for_day_time_interval(value, datatype)} :: {convert_sp_to_sf_type(datatype)}"

if isinstance(value, timedelta) and isinstance(datatype, DayTimeIntervalType):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we unit test this conversion to SQL string? The math is pretty complicated so some tests would be ideal

Comment thread tests/integ/test_udf.py
assert result[0][0] == datetime.timedelta(days=-3)


def test_udf_yearmonth_interval(session):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can also add a test for negative yearmonth

def test_sproc_yearmonth_interval(session):
"""YearMonthIntervalType sproc: int (total months) arg/return round-trips correctly."""
if not _interval_udf_flag_enabled(session):
pytest.skip("ENABLE_INTERVAL_TYPES_IN_UDF not enabled on this account")

@sfc-gh-lshi sfc-gh-lshi Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Because of the skip, the test passes if the feature isn't enabled so we need to be careful about syncing the parameter rollout and client release. Otherwise, we might merge the change with tests skipped, then turn on the parameter and have untested code in the release.

Imo we should keep this PR open until the parameter is turned on, or turn on the parameter in this client's test account, remove the skip, and merge after testing

total_us %= 60_000_000
s = total_us // 1_000_000
us = total_us % 1_000_000
interval_str = f"{sign}{d} {h:02d}:{m:02d}:{s:02d}.{us:06d}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you check if we can reuse format_day_time_interval() or format_day_time_interval_for_display() in type_utils.py?
another question I have is do we need :: {convert_sp_to_sf_type(datatype)} like we used when value is a string?

abs_months = abs(value)
years = abs_months // 12
months = abs_months % 12
interval_str = f"{sign}{years}-{months:02d}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

similar here, is it possible we reuse format_year_month_interval_for_display() ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants