-
Notifications
You must be signed in to change notification settings - Fork 14k
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(tests): Ensure deterministic SELECT ordering for CSV upload tests #23856
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23856 +/- ##
===========================================
+ Coverage 53.58% 68.02% +14.44%
===========================================
Files 1938 1938
Lines 74957 74957
Branches 8140 8140
===========================================
+ Hits 40167 50993 +10826
+ Misses 32706 21880 -10826
Partials 2084 2084
Flags with carried forward coverage won't be shown. Click here to find out more. see 334 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@hughhhh this should fix the issue you were trying to resolve in cc23d5d. I realize that @betodealmeida's recommendation was to sort the output, but it seems like the row ordering likely matters, i.e., should be deterministic. |
data = engine.execute(f"SELECT * from {PARQUET_UPLOAD_TABLE}").fetchall() | ||
assert data == [("max", 3), ("bob", 4), ("john", 1), ("paul", 2)] | ||
assert data == [("john", 1), ("paul", 2), ("max", 3), ("bob", 4)] |
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.
A SELECT
does not guarantee order without an ORDER BY
; if we don't want to use sorted()
here we need to add an explicit ORDER BY
to the query.
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.
@betodealmeida I've re-authored the PR based on your 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.
bd0136c
to
50db0de
Compare
SUMMARY
Some unit tests are failing (example) as
SELECT
statements do not guarantee deterministic ordering unless anORDER BY
clause is used. It's unclear why these unit tests are failing somewhat consistently now, though it might be due to a non-deterministic file ordering when adding files to a zip file.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION