paleolimbot commented on code in PR #18243:
URL: https://github.com/apache/datafusion/pull/18243#discussion_r2462546188
##########
datafusion/sql/src/expr/identifier.rs:
##########
@@ -113,13 +118,18 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
.into_iter()
.map(|id| self.ident_normalizer.normalize(id))
.collect();
- let ty = self
+ let field = self
.context_provider
- .get_variable_type(&var_names)
+ .get_variable_field(&var_names)
+ .or_else(|| {
+ self.context_provider
+ .get_variable_type(&var_names)
+ .map(|ty| Arc::new(Field::new("", ty, true)))
Review Comment:
```suggestion
.map(|ty| ty.into_nullable_field_ref())
```
##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -2169,12 +2169,15 @@ mod tests {
r#"TRY_CAST(a AS INTEGER UNSIGNED)"#,
),
(
- Expr::ScalarVariable(Int8, vec![String::from("@a")]),
+ Expr::ScalarVariable(
+ Arc::new(Field::new("", Int8, true)),
+ vec![String::from("@a")],
+ ),
r#"@a"#,
),
(
Expr::ScalarVariable(
- Int8,
+ Arc::new(Field::new("", Int8, true)),
Review Comment:
```suggestion
Int8.into_nullable_field_ref(),
```
##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -2169,12 +2169,15 @@ mod tests {
r#"TRY_CAST(a AS INTEGER UNSIGNED)"#,
),
(
- Expr::ScalarVariable(Int8, vec![String::from("@a")]),
+ Expr::ScalarVariable(
+ Arc::new(Field::new("", Int8, true)),
Review Comment:
```suggestion
Int8.into_nullable_field_ref(),
```
##########
datafusion/sql/src/expr/identifier.rs:
##########
@@ -39,13 +39,18 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
if id.value.starts_with('@') {
// TODO: figure out if ScalarVariables should be insensitive.
let var_names = vec![id.value];
- let ty = self
+ let field = self
.context_provider
- .get_variable_type(&var_names)
+ .get_variable_field(&var_names)
+ .or_else(|| {
+ self.context_provider
+ .get_variable_type(&var_names)
+ .map(|ty| Arc::new(Field::new("", ty, true)))
Review Comment:
```suggestion
.map(|ty| ty.into_nullable_field_ref())
```
##########
datafusion/expr/src/planner.rs:
##########
@@ -103,6 +103,17 @@ pub trait ContextProvider {
/// A user defined variable is typically accessed via `@var_name`
fn get_variable_type(&self, variable_names: &[String]) -> Option<DataType>;
+ /// Return metadata about a system/user-defined variable, if any.
+ ///
+ /// By default, this wraps [`Self::get_variable_type`] in an Arrow
[`Field`]
+ /// with nullable set to `true` and no metadata. Implementations that can
+ /// provide richer information (such as nullability or extension metadata)
+ /// should override this method.
+ fn get_variable_field(&self, variable_names: &[String]) ->
Option<FieldRef> {
+ self.get_variable_type(variable_names)
+ .map(|data_type| Arc::new(Field::new("", data_type, true)) as
FieldRef)
Review Comment:
```suggestion
.map(|data_type| data_type.into_nullable_field_ref())
```
##########
datafusion/core/"/var/folders/xf/78z9wmtx4r9g_9pmytgm2ysm0000gn/T/.tmpU3MvU3/target.csv":
##########
Review Comment:
This seems like it was included by accident (it also shows up for me when
tests are run)
--
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]
