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

FIX: Fix merge_asof test disabled sql simplifier #2156

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

sfc-gh-nkrishna
Copy link
Contributor

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

    Fixes SNOW-NNNNNNN

  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.
  3. Please describe how your code solves the related issue.

    Please write a short description of how your code change solves the related issue.

Signed-off-by: Naren Krishna <naren.krishna@snowflake.com>
@sfc-gh-nkrishna sfc-gh-nkrishna requested a review from a team as a code owner August 23, 2024 16:39
@sfc-gh-nkrishna sfc-gh-nkrishna changed the title Fix jenkins CI test for merge_asof FIX: Fix merge_asof test Jenkins CI Aug 23, 2024
@sfc-gh-nkrishna sfc-gh-nkrishna added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs labels Aug 23, 2024
@sfc-gh-nkrishna
Copy link
Contributor Author

):
native_pd.merge_asof(left_native_df, right_native_df, on="a")
# Snowpark pandas raises a SnowparkSQLException
# MATCH_CONDITION clause is invalid: The left and right side expressions must be numeric or timestamp expressions.
with pytest.raises(
SnowparkSQLException,
):
pd.merge_asof(left_snow_df, right_snow_df, on="a")
pd.merge_asof(left_snow_df, right_snow_df, on="a").to_pandas()
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sfc-gh-nkrishna are those test skipped in the merge gate? how com it didn't fail in the merge gate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not sure about that as well, it shouldn't be skipped....

Copy link
Collaborator

Choose a reason for hiding this comment

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

They failed when the SQL optimizer was disabled which only run in github daily.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sfc-gh-azhan do you mean SqlSimpflication?

Copy link
Collaborator

Choose a reason for hiding this comment

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

that kind of make sense, the sql simplification tiggers an describing call when extracting ColumnState for select, that could explain it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yes

@sfc-gh-azhan sfc-gh-azhan changed the title FIX: Fix merge_asof test Jenkins CI FIX: Fix merge_asof test disabled sql simplifier Aug 23, 2024
@sfc-gh-nkrishna sfc-gh-nkrishna merged commit 7cbad6f into main Aug 23, 2024
73 of 74 checks passed
@sfc-gh-nkrishna sfc-gh-nkrishna deleted the jenkins_merge_asof branch August 23, 2024 20:07
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs snowpark-pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants