Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1437407: [Local Testing] convert_timezone calls mock_convert_timezone with wrong argument order #1630

Closed
mraraujo opened this issue May 20, 2024 · 6 comments
Assignees
Labels
local testing Local Testing issues/PRs status-triage_done Initial triage done, will be further handled by the driver team

Comments

@mraraujo
Copy link

  1. What version of Python are you using?

3.11.6

  1. What operating system and processor architecture are you using?

Linux-6.5.0-28-generic-x86_64-with-glibc2.38

  1. What are the component versions in the environment (pip freeze)?

Snowpark version 1.16.0

  1. What did you do?

Called convert_timezone(lit('UTC'), col("timestamp"), lit('Asia/Singapore'))

  1. What did you expect to see?

    I expected the timezone to be converted to UTC. Instead, one gets

>       is_ntz = source_time.sf_type.datatype.tz is TimestampTimeZone.NTZ
E       AttributeError: 'StringType' object has no attribute 'tz'

Looking into the code, one can see that both convert_timezone (from functions.py) and mock_convert_timezone (from _functions.py) have their parameters in the same order (target_timezone, source_time, source_timezone).
However, convert_timezone calls

call_builtin(
        "convert_timezone", source_tz, target_tz, source_time_to_convert
    )

flipping them to match the order of the SQL convert_timezone. However, this breaks mock_convert_timezone.

@mraraujo mraraujo added bug Something isn't working needs triage Initial RCA is required labels May 20, 2024
@github-actions github-actions bot changed the title [Local Testing] convert_timezone calls mock_convert_timezone with wrong argument order SNOW-1437407: [Local Testing] convert_timezone calls mock_convert_timezone with wrong argument order May 20, 2024
@mraraujo
Copy link
Author

mraraujo commented May 20, 2024

Also relevant:

mock_convert_timezone calls dateutil.tz.gettz(source_timezone), which is not possible if source_timezone is wrapped in a lit as shown in the documentation: https://docs.snowflake.com/en/developer-guide/snowpark/reference/python/1.1.0/api/snowflake.snowpark.functions.convert_timezone

Note that #1299 did not add tests with source timezones.

@sfc-gh-aling sfc-gh-aling added the local testing Local Testing issues/PRs label May 20, 2024
@sfc-gh-sghosh sfc-gh-sghosh self-assigned this May 23, 2024
@sfc-gh-sghosh sfc-gh-sghosh added status-triage Issue is under initial triage and removed bug Something isn't working needs triage Initial RCA is required labels May 23, 2024
@sfc-gh-sghosh
Copy link

Hello @mraraujo ,

Thanks for raising the issue, we are looking into it and will update you.

Regards,
Sujan

@ColossalFossil
Copy link

ColossalFossil commented Jun 3, 2024

I am also encountering this issue on older Python and newer Snowpark

1. What version of Python are you using?
python 3.8.19

2. What operating system and processor architecture are you using?
Darwin Kernel Version 23.5.0 xnu-10063.121.3~5/RELEASE_X86_64 x86_64

3. What are the component versions in the environment (pip freeze)?
snowflake-snowpark-python==1.18.0

4. What did you do?
Added a Local DateTime column to a DataFrame to supplement the UTC column, reproducing the primary issue documented here. Reordered the parameters during interactive debugging, and the mock rejected the default timezone on the timestamp when source tz provided (though SF accepts the param)* .

5. What did you expect to see?
DataFrame with original UTC DateTime and a companion column converted to Local, which is the behavior experienced when connecting to Snowflake instead of using the Mock.

Code

     tz = "EST"
     result_df = df.with_column(f"{field.name}_LOCAL",
                               f.convert_timezone(
                               target_timezone=f.lit(tz),
                               source_time=f.col(field.name),
                               source_timezone=f.lit("UTC")

Parameters inside convert_timezone Mock.
No parameter matches its passed value through name or position:

source_time{ColumnEmulator: (1,)}=0 EST dtype: object
source_timezone{ColumnEmulator: (1,)}=0 2020-01-01 10:00:00, Name: "C", dtype: object
target_timezone{ColumnEmulator: (1,)}=0 UTC, dtype: object

Stack Trace (path shortened to /Code):

Traceback (most recent call last):
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 454, in handle_function_expression
result = _MOCK_FUNCTION_IMPLEMENTATION_MAP[func_name](*to_pass_args)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_functions.py", line 1713, in mock_convert_timezone
is_ntz = source_time.sf_type.datatype.tz is TimestampTimeZone.NTZ
AttributeError: 'StringType' object has no attribute 'tz'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/Code/tests/test_suite.py", line 39, in test_add_local_timestamp
df_with_new_tz.show()
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/_internal/telemetry.py", line 145, in wrap
result = func(*args, **kwargs)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/dataframe.py", line 3176, in show
self._show_string(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/dataframe.py", line 3294, in _show_string
result, meta = self._session._conn.get_result_and_metadata(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_connection.py", line 806, in get_result_and_metadata
res = execute_mock_plan(plan, plan.expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 708, in execute_mock_plan
column_series = calculate_expression(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 1693, in calculate_expression
return calculate_expression(exp.child, input_data, analyzer, expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 1695, in calculate_expression
return handle_function_expression(exp, input_data, analyzer, expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 456, in handle_function_expression
SnowparkLocalTestingException.raise_from_error(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/exceptions.py", line 20, in raise_from_error
raise cls(
snowflake.snowpark.mock.exceptions.SnowparkLocalTestingException: Error executing mocked function 'convert_timezone'. See error traceback for detailed information.

Forcing parameters while debugging:

swap = source_time
source_time = source_timezone
source_timezone = target_timezone
target_timezone=swap

target_timezone{ColumnEmulator: (1,)}=0 EST dtype: object
source_time{ColumnEmulator: (1,)}=0 2020-01-01 10:00:00, Name: "C", dtype: object
source_timezone{ColumnEmulator: (1,)}=0 UTC, dtype: object

Stack Trace (path shortened to /Code):

ValueError: [Local Testing] convert_timezone can only convert NTZ timestamps when source_timezone is specified.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/Code/tests/test_suite.py", line 39, in test_add_local_timestamp
df_with_new_tz.show()
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/_internal/telemetry.py", line 145, in wrap
result = func(*args, **kwargs)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/dataframe.py", line 3176, in show
self._show_string(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/dataframe.py", line 3294, in _show_string
result, meta = self._session._conn.get_result_and_metadata(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_connection.py", line 806, in get_result_and_metadata
res = execute_mock_plan(plan, plan.expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 708, in execute_mock_plan
column_series = calculate_expression(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 1693, in calculate_expression
return calculate_expression(exp.child, input_data, analyzer, expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 1695, in calculate_expression
return handle_function_expression(exp, input_data, analyzer, expr_to_alias)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 456, in handle_function_expression
SnowparkLocalTestingException.raise_from_error(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/exceptions.py", line 18, in raise_from_error
raise error
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_plan.py", line 454, in handle_function_expression
result = _MOCK_FUNCTION_IMPLEMENTATION_MAP[func_name](*to_pass_args)
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/_functions.py", line 1715, in mock_convert_timezone
SnowparkLocalTestingException.raise_from_error(
File "/Code/.venv/lib/python3.8/site-packages/snowflake/snowpark/mock/exceptions.py", line 20, in raise_from_error
raise cls(
snowflake.snowpark.mock.exceptions.SnowparkLocalTestingException: [Local Testing] Encountered exception "ValueError" with message "[Local Testing] convert_timezone can only convert NTZ timestamps when source_timezone is specified." during execution, please check the error traceback for detailed information.

@mraraujo
Copy link
Author

@sfc-gh-sghosh Any updates on this bugfix?

@sfc-gh-sghosh
Copy link

Hi @mraraujo ,

The team is working on it.

Regards,
Sujan

@sfc-gh-sghosh sfc-gh-sghosh added status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage Issue is under initial triage labels Jul 23, 2024
@sfc-gh-jrose
Copy link
Contributor

A fix for this was released in v1.19.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
local testing Local Testing issues/PRs status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

6 participants