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

[Local testing] Fixes various local testing bugs #2373

Closed
wants to merge 6 commits into from

Conversation

tvdboom
Copy link

@tvdboom tvdboom commented Sep 30, 2024

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

    Fixes SNOW-1668981
    Fixes SNOW-1360263
    Fixes SNOW-1668862
    Fixes SNOW-1694649
    Fixes SNOW-1707286

  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.

SNOW-1668981

The code block

for k, v in expr_to_alias.items():
  if v == exp.child.name:
      expr_to_alias[k] = quoted_name

wrongly overwrites the expr_to_alias of the columns in dataframe anti (see minimal reproducible example in #2305). I am unsure when this code block would be desirable. Tests pass without them. Still, better for a maintainer to have a good look at this change. If it's required, we could make it optional under a condition yet to be determined.

SNOW-1360263

The windows variable needs to be ordered by the order of the index in res_index.

SNOW-1668862

Literal columns were always created with nullable=False, which made lit(None) columns fail. Now, lit(None) returns a nullable column.

SNOW-1694649

In the merge statement, filtering on the join_condition resulted in a dataframe with unordered indices. This resulted in a failure when calling new_column[either_isna]. Reseting the index solves the issue.

SNOW-1707286

Can't operate on None and datetime. Return None directly.

@tvdboom tvdboom requested a review from a team as a code owner September 30, 2024 14:59
Copy link

github-actions bot commented Sep 30, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@tvdboom
Copy link
Author

tvdboom commented Sep 30, 2024

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

1 similar comment
@MarcovdBoom
Copy link

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

@MarcovdBoom
Copy link

recheck

@tvdboom
Copy link
Author

tvdboom commented Oct 2, 2024

Actually, I found another use case where the removed code block

for k, v in expr_to_alias.items():
  if v == exp.child.name:
      expr_to_alias[k] = quoted_name

is required. @sfc-gh-jrose I see you got assigned #2305, could you help out?

@tvdboom
Copy link
Author

tvdboom commented Oct 7, 2024

@sfc-gh-jrose any change you can help out or review the pr?

@sfc-gh-jrose
Copy link
Contributor

I will review and help merge this week.

@sfc-gh-jrose
Copy link
Contributor

I've moved your fork into a branch in order to get our checks to run correctly. I've also added tests to ensure the examples provided pass. See #2410

@tvdboom
Copy link
Author

tvdboom commented Oct 9, 2024

Perfect, thanks. I guess this pr can be closed then.

@tvdboom tvdboom closed this Oct 9, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants