-
Notifications
You must be signed in to change notification settings - Fork 81
Revise SQL handling in #2172 #2203
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
Revise SQL handling in #2172 #2203
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/handle_empty_sql_output #2203 +/- ##
===================================================================
- Coverage 63.52% 63.52% -0.01%
===================================================================
Files 100 100
Lines 8508 8512 +4
Branches 886 886
===================================================================
+ Hits 5405 5407 +2
- Misses 2936 2938 +2
Partials 167 167 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
✅ 52/52 passed, 5 flaky, 5m32s total Flaky tests:
Running from acceptance #3217 |
sundarshankar89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
This PR revises #2172 so that SQL is handled more safely: in particular we no longer dynamically build SQL queries to run.
Relevant implementation details
DuckDB can bulk insert from Pandas data frames, and with CTAS will properly (and safely) set the table schema. This is preferable to building SQL statements dynamically.
Linked issues
Revises #2172.