houqp commented on a change in pull request #910:
URL: https://github.com/apache/arrow-datafusion/pull/910#discussion_r697964158



##########
File path: datafusion/src/datasource/mod.rs
##########
@@ -17,6 +17,7 @@
 
 //! DataFusion data sources
 
+pub mod avro;

Review comment:
       this should be gated by feature flag, no?

##########
File path: datafusion/src/physical_plan/expressions/nth_value.rs
##########
@@ -78,7 +78,7 @@ impl NthValue {
     }
 
     /// Create a new NTH_VALUE window aggregate function
-    pub fn nth_value(
+    pub fn value(

Review comment:
       maybe `first`, `last` and `nth` would be better names here, cc @Jimexist 

##########
File path: datafusion/Cargo.toml
##########
@@ -69,6 +70,8 @@ regex = { version = "^1.4.3", optional = true }
 lazy_static = { version = "^1.4.0", optional = true }
 smallvec = { version = "1.6", features = ["union"] }
 rand = "0.8"
+avro-rs = { version = "0.13", features = ["snappy"], optional = true }
+num-traits = "0.2"

Review comment:
       It looks like num-traits should be marked as optional and be included as 
part of `avro` feature flag?




-- 
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]


Reply via email to