Helper methods for CDF Physical to Logical Transformation #579
Conversation
|
Tests are currently failing because we they depend on the FileMeta fix. |
OussamaSaoudi-db
force-pushed
the
cdf_phys_to_log
branch
from
94d8cbb to
5622874
Compare
kernel/src/table_changes/scan.rs
Outdated
| ColumnType::Selected(CHANGE_TYPE_COL_NAME.to_string()), | ||
| ColumnType::Selected(COMMIT_VERSION_COL_NAME.to_string()), | ||
| ColumnType::Selected(COMMIT_TIMESTAMP_COL_NAME.to_string()), |
There was a problem hiding this comment.
actually (and sorry for the churn) I think I like the literal strings here: this let's us check that they are the actual strings we expect in the protocol (failure case we prevent is someone changes the const and then this test would change and pass)
There was a problem hiding this comment.
aha that makes sense. So we get to assert more about the code through this test 👍
| scan_file: &CdfScanFile, | ||
| global_state: &GlobalScanState, | ||
| logical_schema: &StructType, |
There was a problem hiding this comment.
nice! i like taking the more constrained data
| @@ -198,6 +213,7 @@ mod tests { | |||
| use crate::expressions::{column_expr, Scalar}; | |||
| use crate::scan::ColumnType; | |||
| use crate::schema::{DataType, StructField, StructType}; | |||
| use crate::table_changes::COMMIT_VERSION_COL_NAME; | |||
There was a problem hiding this comment.
can just do use super::*?
There was a problem hiding this comment.
Doesn't seem like it. super would point to the scan module, but we need its parent.
OussamaSaoudi-db
changed the title
What changes are proposed in this pull request?
This PR introduces methods methods that will be useful to read CDF data and transform it from its physical form into the logical schema. The methods are:
get_cdf_columns: Generates a map from cdf column name to expression that fills that columnphysical_to_logical_expression: Generates the physical to logical expression used to transform the engine datascan_file_read_schema: Gets the physical schema. This depends on the cdf scan file typeWe also introduce the method
Scalar::timestamp_ntz_from_milliswhich converts from ani64millisecond value to aScalar::TimestampNtz.How was this change tested?
We test that
physical_to_logical_expressiongenerates the correct expression for aCdfScanFilewith:_change_typecolumn in the case of add, cdc, and remove CdfScanFile.