Skip to content

Conversation

@FBruzzesi
Copy link
Member

@FBruzzesi FBruzzesi commented Nov 4, 2025

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

Very first iteration, mostly a proof of concept to make sure there is a brute forced way for each backend.

I am not targeting main, rather #3276, so that I already have:

  • correct signature elsewhere
  • all the tests I need to check that the behavior is as expected

@FBruzzesi FBruzzesi added the enhancement New feature or request label Nov 4, 2025
@FBruzzesi
Copy link
Member Author

CI failures are interesting:

  • pyspark succeeds, but pyspark connect doesn't. It might be worth reporting this upstream
  • no coverage is because sqlframe does fail. I will make sure to open an issue with all the methods we are using that are currently missing

Base automatically changed from feat/replace-strict-default to main November 7, 2025 22:04
@FBruzzesi
Copy link
Member Author

Alright, I reported the issue to SQLFrame and added the relevant link and comments in the code. Yesterday I learned that reporting spark issue is just not that simple, so I settled on xfailing for pyspark connect.

Regarding code sharing, I tried to abstract away a few parts but I didn't like the amount of inputs required to make it work, so I am marking this ready for review as is for now

@FBruzzesi FBruzzesi marked this pull request as ready for review November 8, 2025 09:57
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks @FBruzzesi !

I just simplified it slightly at the compliant level, as there were some unnecessary checks

@MarcoGorelli MarcoGorelli merged commit f731f1a into main Nov 10, 2025
35 of 36 checks passed
@MarcoGorelli MarcoGorelli deleted the feat/lazy-expr-replace-strict branch November 10, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants