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(snowflake): use get instead of get_path; get_path does not support columns with spaces #10836

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Feb 13, 2025

Closes #10835.

@cpcloud cpcloud added bug Incorrect behavior inside of ibis snowflake The Snowflake backend labels Feb 13, 2025
@github-actions github-actions bot added the tests Issues or PRs related to tests label Feb 13, 2025
@cpcloud
Copy link
Member Author

cpcloud commented Feb 13, 2025

I'm looking into why we were using get_path in the first place.

@cpcloud cpcloud force-pushed the fix-snowflake-temp-table-creation branch 2 times, most recently from bfdf96e to 24281cd Compare February 13, 2025 16:38
@cpcloud
Copy link
Member Author

cpcloud commented Feb 13, 2025

GET_PATH does support quoted names, but for this particular case, GET generates less code and is easier to read.

@cpcloud cpcloud force-pushed the fix-snowflake-temp-table-creation branch from 24281cd to a8d7d5f Compare February 13, 2025 16:44
@cpcloud
Copy link
Member Author

cpcloud commented Feb 13, 2025

Snowflake is passing:

cloud in 🌐 falcon in …/ibis on  fix-snowflake-temp-table-creation is 📦 v10.0.0 via 🐍 v3.12.8 via ❄️  impure (ibis-3.12-env)
❯ pytest -m snowflake -n 8 --dist loadgroup -q
bringing up nodes...
.............................................................xxxxxxx....x....x.............x................................x..........xxx............x.......................x..............................x............................x....x..........................................................s............x.........x.xx...x.....x.x.x...xx..............x......xx......x.xx........x.xx [ 19%]
.......................xx....x............x.......x......x..............s....x..............x.........x...x.....x....x..............................x........x...............x.........................xsx................x........x....x.......................s.....x...................................s..........................s.....................................x.....x....x.............. [ 38%]
..........x..................................x.....x......x.............x..xx.x.....x............x......x....x.........x.........x..............x.......x.................x........................xx...............x...x.x..x.x........xx..........x..x....x.x....xx....x..x.....x..x.........x...x...x..x.x............xx..xx........x.x........x.......x..............x..xx.................x.x... [ 57%]
........x.................................................x..................x....................................x.........................x...................x....x............................................x......x.xxx..x...........xxx.xx..xx......xx.xx...x.xxxx.xxx..xx..xxxx..x..x..xx.....xx.x.x.xxx.x...x...xx.xxxxx.x..x.....xxx....xxxxxxxx.x.x....x..x..x..x...x...x...x............ [ 76%]
x........x......x..x...............xx........................x..........x..x.............x.............x........x.x...........x...................................................x...............x......x....xx.....................x...........x..x..x...........................x.s....s..........s............................................................................................... [ 95%]
........................s.............................................................................                                                                                                                                                                                                                                                                                                [100%]
1817 passed, 10 skipped, 220 xfailed in 219.94s (0:03:39)

@cpcloud
Copy link
Member Author

cpcloud commented Feb 13, 2025

Looks like I introduced get_path usage in a555774, as part of the parquet loading cleanup in the Snowflake backend.

That said, I'm not entirely sure why I used that instead of get.

@cpcloud cpcloud force-pushed the fix-snowflake-temp-table-creation branch from a8d7d5f to 7fa0f74 Compare February 13, 2025 17:00
@cpcloud
Copy link
Member Author

cpcloud commented Feb 13, 2025

Beefed up the test a bit to use hypothesis, in order to find any cases we're not handling.

@cpcloud cpcloud force-pushed the fix-snowflake-temp-table-creation branch from 7fa0f74 to 328f79e Compare February 13, 2025 17:33
@cpcloud cpcloud force-pushed the fix-snowflake-temp-table-creation branch from 328f79e to 46d6e50 Compare February 13, 2025 17:34
@cpcloud cpcloud merged commit 50c978b into ibis-project:main Feb 13, 2025
86 of 87 checks passed
@cpcloud cpcloud deleted the fix-snowflake-temp-table-creation branch February 13, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior inside of ibis snowflake The Snowflake backend tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Snowflake temp table creation fails if whitespace in any column name
1 participant