SNOW-1690923 Encode Intervals

[sfc-gh-vbudati]

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

   Fixes SNOW-1690923

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

3. Please describe how your code solves the related issue.

Encoding the Interval object in AST. Relevant test is interval.test.

[sfc-gh-vbudati]

Had to import here to prevent circular imports issue.

[sfc-gh-lspiegelberg]

So far in the code, we tried to avoid building expressions directly. The reasoning was that Expressions should disappear as they're not a first-class entity in snowpark. Instead we chose an approach where always the caller (i.e. the function that creates Interval) creates the AST and assigns it to the expression if need be. I.e.,

def build_expr_from_snowpark_column_or_python_val(
    expr_builder: proto.Expr, value: ColumnOrLiteral
) -> None:
    """Copy from a Column object's AST, or copy a literal value into an AST expression.

    Args:
        ast (proto.Expr): A previously created Expr() IR entity intance to be filled
        value (ColumnOrLiteral): The value from which to populate the provided ast parameter.

    Raises:
        TypeError: The Expr provided should only be populated from a Snowpark Column with a valid _ast field or a literal value
    """
    if isinstance(value, snowflake.snowpark.Column):
        build_expr_from_snowpark_column(expr_builder, value)
    elif isinstance(value, VALID_PYTHON_TYPES_FOR_LITERAL_VALUE):
        build_expr_from_python_val(expr_builder, value)
    elif isinstance(value, Expression):
        # Expressions must be handled by caller.
        pass
    else:
        raise TypeError(f"{type(value)} is not a valid type for Column or literal AST.")

[sfc-gh-lspiegelberg]

we prob. can move the AST logic to build_expr_from_snowpark_column_or_python_val and have an Interval case there. I don't think the original intention of snowpark was to use Interval directly, but rather the make_interval function.

[sfc-gh-vbudati]

I ran the linter which is why there's 100+ file changes - most of these changes are in the test data directory and I think are superficial. I'm not sure how to fix the pyright/precommit type checking test.

[sfc-gh-evandenberg]

/home/runner/work/snowpark-python/snowpark-python/src/snowflake/snowpark/_internal/analyzer/expression.py:413:26 - error: "Expr" is not a known member of module "snowflake.snowpark._internal.proto.ast_pb2" (reportGeneralTypeIssues)

[sfc-gh-evandenberg]

Can you repro locally?

python -m tox -e pyright

It is saying you may need a new version:
"Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to latest"

[sfc-gh-vbudati]

I get the same errors locally, I'm not sure what to do about the first error though. There's other parts of the code that use proto.Expr() as well so I'm not sure why it's erroring here

[sfc-gh-evandenberg]

Update to protobuf to 5.28.2 to avoid so many conflicts?

[sfc-gh-lspiegelberg]

let's try to avoid indirection via getattr and use the values explicitly. It's much easier to read and reason about :)

[sfc-gh-lspiegelberg]

let's use make_interval here to test. Not sure we should support the Column(Interval(...)) syntax at all.

[sfc-gh-lspiegelberg]

does this work in snowpark?

[sfc-gh-vbudati]

I believe it does - I pulled this from test_interval:

def test_interval(session):
    df1 = session.create_dataframe(
        [
            [datetime.datetime(2010, 1, 1), datetime.datetime(2011, 1, 1)],
            [datetime.datetime(2012, 1, 1), datetime.datetime(2013, 1, 1)],
        ],
        schema=["a", "b"],
    )
    df2 = df1.select(
        df1["a"]
        + Column(
            Interval(
                quarters=1,
                months=1,
                weeks=2,
                days=2,
                hours=2,
                minutes=3,
                seconds=3,
                milliseconds=3,
                microseconds=4,
                nanoseconds=4,
            )
        )
    )
    verify_column_result(
        session,
        df2,
        [
            '"(""A"" + INTERVAL \'1 QUARTER,1 MONTH,2 WEEK,2 DAY,2 HOUR,3 MINUTE,3 SECOND,3 MILLISECOND,4 MICROSECOND,4 NANOSECOND\')"',
        ],
        [TimestampType(timezone=TimestampTimeZone.NTZ)],
        None,
    )

but I see that you added the make_interval changes - I'll update my tests accordingly