-
Notifications
You must be signed in to change notification settings - Fork 12
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
refactor: modularize tests #78
Conversation
78fc3c9
to
ed3e766
Compare
`COPY FROM parquet` is too strict when matching Postgres tupledesc schema to the parquet file schema. e.g. `INT32` type in the parquet schema cannot be read into a Postgres column with `int64` type. We can avoid this situation by casting arrow array to the array that is expected by the tupledesc schema, if the cast is possible. We can make use of `arrow-cast` crate, which is in the same project with `arrow`. Its public api lets us check if a cast possible between 2 arrow types and perform the cast. To make sure the cast is possible, we need to do 2 checks: 1. arrow-cast allows the cast from "arrow type at the parquet file" to "arrow type at the schema that is generated for tupledesc", 2. the cast is meaningful at Postgres. We check if there is an explicit cast from "Postgres type that corresponds for the arrow type at Parquet file" to "Postgres type at tupledesc". With that we can cast between many castable types as shown below: - INT16 => INT32 - UINT32 => INT64 - FLOAT32 => FLOAT64 - LargeUtf8 => UTF8 - LargeBinary => Binary - Struct, Array, and Map with castable fields, e.g. [UINT16] => [INT64] or struct {'x': UINT16} => struct {'x': INT64} **NOTE**: Struct fields must match by name and position to be cast. Closes #67.
ed3e766
to
b408fac
Compare
c69fd25
to
25d6e69
Compare
src/pgrx_tests/common.rs
Outdated
CopyOptionValue::StringOption("parquet".to_string()), | ||
); | ||
|
||
let uri = "/tmp/test.parquet".to_string(); |
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.
maybe should use more obscure file names. A user/developer might have an unrelated file named this way and get very confused
src/pgrx_tests/common.rs
Outdated
} | ||
} | ||
|
||
pub(crate) fn test_helper<T: IntoDatum + FromDatum + Debug + PartialEq>(test_table: TestTable<T>) { |
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.
could be nice to have a more descriptive name here now that things are getting more dispersed, e.g. test_export_import_equality test_round_trip_equality
25d6e69
to
c59b9b7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
==========================================
+ Coverage 91.94% 92.01% +0.06%
==========================================
Files 62 70 +8
Lines 8795 8879 +84
==========================================
+ Hits 8087 8170 +83
- Misses 708 709 +1 ☔ View full report in Codecov by Sentry. |
Single test file gets bigger and bigger. Lets have separate test files for different scenarios.