alamb commented on code in PR #10650:
URL: https://github.com/apache/datafusion/pull/10650#discussion_r1614042615


##########
datafusion/functions/Cargo.toml:
##########
@@ -80,7 +80,7 @@ itertools = { workspace = true }
 log = { workspace = true }
 md-5 = { version = "^0.10.0", optional = true }
 rand = { workspace = true }
-regex = { worksapce = true, optional = true }
+regex = { workspace = true, optional = true }

Review Comment:
   Thank you -- that is a nice fix -- it would be easier to merge as a separate 
PR



##########
datafusion/common/src/config.rs:
##########
@@ -1555,7 +1555,7 @@ config_namespace! {
         /// Specifies whether there is a CSV header (i.e. the first line
         /// consists of is column names). The value `None` indicates that
         /// the configuration should be consulted.
-        pub has_header: Option<bool>, default = None
+      pub has_header: bool, default = false

Review Comment:
   I don't understand the implication of this change and there are no tests 
that cover it.
   
   Can you please at least update the documentation to reflect the new behavior 
(it can no longer be None)
   
   Also there appears to be some strange indenting going on



##########
datafusion/physical-expr/src/expressions/cast.rs:
##########
@@ -170,7 +170,8 @@ impl PhysicalExpr for CastExpr {
         let target_type = &self.cast_type;
 
         let unbounded = Interval::make_unbounded(target_type)?;
-        if source_datatype.is_numeric() && target_type.is_numeric()
+        if (source_datatype.is_numeric() || source_datatype == Boolean)

Review Comment:
   I don't see any test coverage for this change



##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -1106,7 +1106,7 @@ message CsvWriterOptions {
 
 // Options controlling CSV format
 message CsvOptions {
-  bytes has_header = 1; // Indicates if the CSV has a header row
+  bool has_header = 1; // Indicates if the CSV has a header row

Review Comment:
   that is certainly nicer



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to