changsun20 commented on code in PR #15143:
URL: https://github.com/apache/datafusion/pull/15143#discussion_r1990535965
##########
datafusion/sql/tests/cases/diagnostic.rs:
##########
@@ -286,3 +286,121 @@ fn test_invalid_function() -> Result<()> {
assert_eq!(diag.span, Some(spans["whole"]));
Ok(())
}
+
+#[test]
+fn test_scalar_subquery_multiple_columns() -> Result<(), Box<dyn
std::error::Error>> {
+ let query = "SELECT (SELECT 1 AS /*x*/x/*x*/, 2 AS /*y*/y/*y*/) AS col";
+ let spans = get_spans(query);
+ let diag = do_query(query);
+
+ assert_eq!(
+ diag.message,
+ "Scalar subquery should only return one column"
+ );
Review Comment:
Yes, I totally agree with your suggestion. Here I would like to explain my
current implementation in detail.
For the main `Diagnostic` error, it now contains information in the larger
scope:
```
SELECT (SELECT 1 AS x, 2 AS y) AS col
^^^^^^^^^
Too many columns! The subquery should only return one
column
```
This time, the span includes from "x" to "y"("x, 2 AS y") for the example
above. The reason why I didn't implement it as going from "1" to "y"("1 AS x, 2
AS y") is that the value 1 doesn't include a span parameter. The projection of
this query is:
```
projection: [
ExprWithAlias {
expr: Value(
Number(
"1", // no span information here
false,
),
),
alias: Ident {
value: "x",
quote_style: None,
span: Span(Location(1,21)..Location(1,22)),
},
},
ExprWithAlias {
expr: Value(
Number(
"2",
false,
),
),
alias: Ident {
value: "y",
quote_style: None,
span: Span(Location(1,29)..Location(1,30)),
},
},
],
```
The span actually combines the start point of "x" and end point of "y",
which might seem to be a bit asymmetric comparing to "1 AS x, 2 AS y". I think
this is acceptable, and to include more information of the span may require
significant change to other existing structs. Please let me if you have any
comments or concerns on this implementation.
--
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]