alamb commented on code in PR #4702:
URL: https://github.com/apache/arrow-datafusion/pull/4702#discussion_r1055843307
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1298,6 +1299,7 @@ impl Projection {
/// Aliased subquery
#[derive(Clone)]
+#[non_exhaustive]
Review Comment:
```suggestion
// mark non_exhaustive to encourage use of try_new/new()
#[non_exhaustive]
```
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1235,6 +1235,7 @@ pub struct Values {
/// Evaluates an arbitrary list of expressions (essentially a
/// SELECT with an expression list) on its input.
#[derive(Clone)]
+#[non_exhaustive]
Review Comment:
```suggestion
// mark non_exhaustive to encourage use of try_new/new()
#[non_exhaustive]
```
I think some rationale about why this is marked non_exhaustive will help
future readers (like ourselves!)
##########
datafusion/optimizer/src/single_distinct_to_groupby.rs:
##########
@@ -98,6 +98,7 @@ impl OptimizerRule for SingleDistinctToGroupBy {
aggr_expr,
schema,
group_expr,
+ ..
Review Comment:
this is unfortunate (previously when new fields were added, the compiler
would tell you all locations that might need to be changed. I realize this is a
consequence of marking the structs `non_exhaustive`
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1571,6 +1563,7 @@ pub struct Distinct {
/// Aggregates its input based on a set of grouping and aggregate
/// expressions (e.g. SUM).
#[derive(Clone)]
+#[non_exhaustive]
Review Comment:
```suggestion
// mark non_exhaustive to encourage use of try_new/new()
#[non_exhaustive]
```
--
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]
