haochunchang commented on code in PR #26202:
URL: https://github.com/apache/datafusion/pull/26202#discussion_r4389476456
##########
datafusion/sql/tests/cases/diagnostic.rs:
##########
@@ -791,3 +810,211 @@ fn test_window_function_alias_in_having() -> Result<()> {
assert_eq!(diag.span, Some(spans["a"]));
Ok(())
}
+
Review Comment:
Added the `test_duplicate_cte_name_without_spans` test to assert the
programmatically constructed ASTs having the original error text.
##########
datafusion/sql/tests/cases/diagnostic.rs:
##########
@@ -791,3 +810,211 @@ fn test_window_function_alias_in_having() -> Result<()> {
assert_eq!(diag.span, Some(spans["a"]));
Ok(())
}
+
+#[test]
+fn test_duplicate_cte_name() -> Result<()> {
+ let query = "WITH /*a*/cte/*a*/ AS (SELECT 1 AS col), /*b*/cte/*b*/ AS
(SELECT 2 AS col) SELECT 1";
+ let spans = get_spans(query);
+ let diag = do_query(query);
+ assert_snapshot!(diag.message, @r#"WITH query name "cte" specified more
than once"#);
+ assert_eq!(diag.span, Some(spans["b"]));
+ assert_eq!(diag.notes.len(), 1);
+ assert_snapshot!(diag.notes[0].message, @"previously defined here");
+ assert_eq!(diag.notes[0].span, Some(spans["a"]));
+ Ok(())
+}
+
+#[test]
+fn test_duplicate_table_name() {
+ // The duplicated name in each query is marked `a` at its first occurrence
+ // and `b` at the repeated one.
+ // Cases that fail planning because the merged schema holds the same
+ // qualified field twice, as before the warning was added. Every other case
+ // plans successfully.
+ const FAILING: &[&str] = &[
+ "comma_basic_alias",
+ "comma_alias_not_first",
+ "comma_bare_table",
+ "comma_nested_join",
+ "explicit_join_bare_table",
+ ];
+ let cases: &[(&str, &str, &str)] = &[
+ // (label, query, duplicated name)
+ (
+ "comma_basic_alias",
+ "SELECT * FROM person /*a*/a/*a*/, person /*b*/a/*b*/",
+ "a",
+ ),
+ (
+ "comma_alias_not_first",
+ "SELECT * FROM person a, test_decimal /*a*/b/*a*/, person
/*b*/b/*b*/",
+ "b",
+ ),
+ (
+ "comma_bare_table",
+ "SELECT * FROM /*a*/person/*a*/, /*b*/person/*b*/",
+ "person",
+ ),
+ (
+ "comma_non_overlapping_columns",
+ "SELECT * FROM j1 AS /*a*/t/*a*/, j2 AS /*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "comma_non_overlapping_three_tables",
+ "SELECT * FROM j1 AS x, j2 AS /*a*/t/*a*/, j3 AS y, j1 AS
/*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "comma_derived_subquery",
+ "SELECT * FROM (SELECT 1) AS /*a*/t/*a*/, (SELECT 2) AS
/*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "comma_table_and_derived",
+ "SELECT * FROM person AS /*a*/t/*a*/, (SELECT 1) AS /*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "comma_derived_and_table",
+ "SELECT * FROM (SELECT 1) AS /*a*/t/*a*/, person AS /*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "comma_nested_join",
+ "SELECT * FROM (person CROSS JOIN j1) AS /*a*/t/*a*/, (person
CROSS JOIN j2) AS /*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "explicit_join_alias",
+ "SELECT 1 FROM j1 AS /*a*/t/*a*/ JOIN j2 AS /*b*/t/*b*/ ON true",
+ "t",
+ ),
+ (
+ "explicit_join_derived",
+ "SELECT 1 FROM (SELECT 1 AS a) AS /*a*/t/*a*/ JOIN (SELECT 2 AS b)
AS /*b*/t/*b*/ ON true",
+ "t",
+ ),
+ (
+ "comma_and_explicit_join",
+ "SELECT 1 FROM j1 AS /*a*/t/*a*/, j2 JOIN j3 AS /*b*/t/*b*/ ON
true",
+ "t",
+ ),
+ (
+ "explicit_join_bare_table",
+ "SELECT 1 FROM /*a*/person/*a*/ JOIN /*b*/person/*b*/ ON true",
+ "person",
+ ),
+ (
+ "explicit_cross_join",
+ "SELECT 1 FROM j1 AS /*a*/t/*a*/ CROSS JOIN j2 AS /*b*/t/*b*/",
+ "t",
+ ),
+ (
+ "explicit_left_semi_join",
+ "SELECT 1 FROM j1 AS /*a*/t/*a*/ LEFT SEMI JOIN j2 AS /*b*/t/*b*/
ON true",
+ "t",
+ ),
+ (
+ "explicit_left_join",
+ "SELECT 1 FROM j1 AS /*a*/t/*a*/ LEFT JOIN j2 AS /*b*/t/*b*/ ON
true",
+ "t",
+ ),
+ (
+ "explicit_three_way_join",
+ "SELECT 1 FROM j1 AS /*a*/x/*a*/ JOIN j2 AS y ON true JOIN j3 AS
/*b*/x/*b*/ ON true",
+ "x",
+ ),
+ (
+ "explicit_between_joins",
+ "SELECT 1 FROM j1 AS x JOIN j2 AS /*a*/y/*a*/ ON true JOIN j3 AS
/*b*/y/*b*/ ON true",
+ "y",
+ ),
+ (
+ "explicit_nested_join",
+ "SELECT 1 FROM (j1 CROSS JOIN j2) AS /*a*/t/*a*/ JOIN j3 AS
/*b*/t/*b*/ ON true",
+ "t",
+ ),
+ (
+ "inside_nested_join",
+ "SELECT 1 FROM (j1 AS /*a*/t/*a*/ CROSS JOIN j2 AS /*b*/t/*b*/) AS
n",
+ "t",
+ ),
+ (
+ "cross_entry_joins",
+ "SELECT 1 FROM j1 JOIN j2 AS /*a*/t/*a*/ ON true, j3 JOIN person
AS /*b*/t/*b*/ ON true",
+ "t",
+ ),
+ (
+ "case_insensitive",
+ "SELECT 1 FROM j1 AS /*a*/T/*a*/ JOIN j2 AS /*b*/t/*b*/ ON true",
+ "t",
+ ),
+ ];
+
+ for &(label, query, name) in cases {
+ let spans = get_spans(query);
+ let (result, warnings) = plan_query(query);
+ let message = format!("table name {name:?} specified more than once");
+ let check = |diag: &Diagnostic, kind: DiagnosticKind| {
+ assert_eq!(diag.kind, kind, "kind mismatch in {label}");
+ assert_eq!(diag.message, message, "message mismatch in {label}");
+ assert_eq!(diag.span, Some(spans["b"]), "span mismatch in
{label}");
+ assert_eq!(diag.notes.len(), 1, "expected 1 note in {label}");
+ assert_eq!(
+ diag.notes[0].message, "first defined here",
+ "note mismatch in {label}"
+ );
+ assert_eq!(
+ diag.notes[0].span,
+ Some(spans["a"]),
+ "note span mismatch in {label}"
+ );
+ };
+
+ // Planning outcome is unchanged by the duplicate; exactly one warning
+ // is always reported.
+ assert_eq!(warnings.len(), 1, "expected 1 warning in {label}");
+ check(&warnings[0], DiagnosticKind::Warning);
+
+ match (result, FAILING.contains(&label)) {
+ (Ok(()), false) => {}
+ (Err(err), true) => {
+ // The original error text is kept.
+ assert!(
+ err.strip_backtrace()
+ .contains("duplicate qualified field name"),
+ "unexpected error in {label}: {err}"
+ );
+ check(
+ err.diagnostic().expect("expected diagnostic"),
+ DiagnosticKind::Error,
+ );
+ }
+ (Ok(()), true) => panic!("expected {label} to fail planning"),
+ (Err(err), false) => panic!("expected {label} to plan, got:
{err}"),
+ }
+ }
+}
+
+#[test]
+fn test_duplicate_table_name_single_warning_in_multi_entry_from() {
Review Comment:
Added in `test_duplicate_table_name_three_copies_each_point_to_first`
--
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]
