LuciferYang commented on code in PR #11:
URL: https://github.com/apache/spark-connect-rust/pull/11#discussion_r2803126628
##########
crates/connect/src/expressions.rs:
##########
@@ -196,6 +215,7 @@ impl From<chrono::NaiveDate> for spark::expression::Literal
{
value.signed_duration_since(chrono::NaiveDate::from_ymd_opt(1970,
1, 1).unwrap());
spark::expression::Literal {
+ data_type: Some(spark::DataType::from(14)),
Review Comment:
Isn't `#[cfg(feature = "spark-41")]` needed here?
##########
crates/connect/src/dataframe.rs:
##########
@@ -2542,37 +2566,6 @@ mod tests {
Ok(())
}
- #[tokio::test]
Review Comment:
Will this test case be rewritten using the Arrow C FFI interface in the
future?
##########
crates/connect/src/plan.rs:
##########
@@ -834,6 +840,8 @@ impl From<spark::relation::RelType> for LogicalPlanBuilder {
common: Some(RelationCommon {
source_info: "".to_string(),
plan_id: Some(plan_id),
+ #[cfg(spark41)]
Review Comment:
```
#[cfg(feature = "spark-41")]
let relation = Relation {
common: Some(RelationCommon {
source_info: "".to_string(),
plan_id: Some(plan_id),
origin: None,
}),
rel_type: Some(rel_type),
};
#[cfg(not(feature = "spark-41"))]
let relation = Relation {
common: Some(RelationCommon {
source_info: "".to_string(),
plan_id: Some(plan_id),
}),
rel_type: Some(rel_type),
};
```
Maybe it could be changed like this? Would this allow us to avoid defining
`spark41`?
##########
crates/connect/src/expressions.rs:
##########
@@ -147,6 +158,8 @@ impl From<&[u8]> for spark::expression::Literal {
impl From<i16> for spark::expression::Literal {
fn from(value: i16) -> Self {
spark::expression::Literal {
+ #[cfg(feature = "spark-41")]
+ data_type: Some(spark::DataType::from(5)),
Review Comment:
Using hard-coded numbers to represent Spark data types results in poor
readability and maintainability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
