goldmedal commented on code in PR #13856:
URL: https://github.com/apache/datafusion/pull/13856#discussion_r1894761801
##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -22,22 +22,25 @@ use async_trait::async_trait;
use datafusion_catalog::Session;
use datafusion_catalog::TableFunctionImpl;
use datafusion_catalog::TableProvider;
-use datafusion_common::{not_impl_err, plan_err, Result, ScalarValue};
+use datafusion_common::{plan_err, Result, ScalarValue};
use datafusion_expr::{Expr, TableType};
use datafusion_physical_plan::memory::{LazyBatchGenerator, LazyMemoryExec};
use datafusion_physical_plan::ExecutionPlan;
use parking_lot::RwLock;
use std::fmt;
use std::sync::Arc;
+#[derive(Debug, Clone)]
+enum GenSeriesArgs {
+ ContainsNull,
+ AllNotNullArgs { start: i64, end: i64, step: i64 },
+}
+
Review Comment:
It's better to add some doc for the enum and its variants.
##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -22,22 +22,25 @@ use async_trait::async_trait;
use datafusion_catalog::Session;
use datafusion_catalog::TableFunctionImpl;
use datafusion_catalog::TableProvider;
-use datafusion_common::{not_impl_err, plan_err, Result, ScalarValue};
+use datafusion_common::{plan_err, Result, ScalarValue};
use datafusion_expr::{Expr, TableType};
use datafusion_physical_plan::memory::{LazyBatchGenerator, LazyMemoryExec};
use datafusion_physical_plan::ExecutionPlan;
use parking_lot::RwLock;
use std::fmt;
use std::sync::Arc;
+#[derive(Debug, Clone)]
+enum GenSeriesArgs {
+ ContainsNull,
+ AllNotNullArgs { start: i64, end: i64, step: i64 },
+}
+
/// Table that generates a series of integers from `start`(inclusive) to
`end`(inclusive)
#[derive(Debug, Clone)]
struct GenerateSeriesTable {
schema: SchemaRef,
- // None if input is Null
- start: Option<i64>,
- // None if input is Null
- end: Option<i64>,
+ args: GenSeriesArgs,
}
/// Table state that generates a series of integers from `start`(inclusive) to
`end`(inclusive)
Review Comment:
Same as above. It should be updated.
##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -22,22 +22,25 @@ use async_trait::async_trait;
use datafusion_catalog::Session;
use datafusion_catalog::TableFunctionImpl;
use datafusion_catalog::TableProvider;
-use datafusion_common::{not_impl_err, plan_err, Result, ScalarValue};
+use datafusion_common::{plan_err, Result, ScalarValue};
use datafusion_expr::{Expr, TableType};
use datafusion_physical_plan::memory::{LazyBatchGenerator, LazyMemoryExec};
use datafusion_physical_plan::ExecutionPlan;
use parking_lot::RwLock;
use std::fmt;
use std::sync::Arc;
+#[derive(Debug, Clone)]
+enum GenSeriesArgs {
+ ContainsNull,
+ AllNotNullArgs { start: i64, end: i64, step: i64 },
+}
+
/// Table that generates a series of integers from `start`(inclusive) to
`end`(inclusive)
Review Comment:
I believe the comment should be updated for `step`.
--
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
