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)), //...so I use the span information here }, }, 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 know 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: 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