Skip to content

Commit cd64a37

Browse files
committed
fix(diagnostics): keep node names and guessed aliases out of the unaliased column message
The SQ0009 label named the internal node kind, such as `j_s_o_n_extract`, and its help suggested an alias guessed from the text under the span. For a path that was only its first name, and for a literal a quoted copy of the literal. The label now reads plainly, and a literal or path gets the help to add an alias without a guessed name.
1 parent b802ca9 commit cd64a37

6 files changed

Lines changed: 25 additions & 8 deletions

File tree

‎crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ use serde_json::{Map, Value, json};
44
use sqlquack_blocks::blocks::main::quoted_identifier::quoted_identifier;
55
use sqlquack_diagnostics::classification::models::{Construct, ConstructFamily};
66
use sqlquack_diagnostics::diagnostics::constants::{
7-
REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT,
7+
REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT, UNALIASED_QUOTED_NAME,
8+
UNALIASED_TEXT_NAME,
89
};
910
use sqlquack_diagnostics::provenance::constants::QUERY_CONSTRUCT;
1011
use sqlquack_diagnostics::provenance::models::ShapeArgument;
@@ -34,6 +35,7 @@ use crate::snowflake::conversion::_helpers::support::tree::{
3435
function_construct, identifier_name, list, node_construct, node_kind, payload_object,
3536
require_understood,
3637
};
38+
use crate::snowflake::conversion::constants::UNALIASED_QUOTED_KINDS;
3739
use crate::snowflake::conversion::constants::WINDOW_MARKER;
3840
use crate::snowflake::conversion::constants::{
3941
ALIAS, COMMENT, COMMON_TABLE, DISTINCT, GENERATOR_SEQUENCE_COLUMN, GROUP_BY, HAVING,
@@ -694,7 +696,10 @@ fn convert_projection(
694696
location: site.context.locator.locate_written(item),
695697
..missing(
696698
Construct::new(ConstructFamily::Syntax, UNALIASED_OUTPUT_NAME_CONSTRUCT),
697-
format!("Snowflake names this `{kind}` column after its text"),
699+
match UNALIASED_QUOTED_KINDS.contains(&kind) {
700+
true => UNALIASED_QUOTED_NAME.to_owned(),
701+
false => UNALIASED_TEXT_NAME.to_owned(),
702+
},
698703
)
699704
}),
700705
}

‎crates/sqlquack-convert/src/snowflake/conversion/constants.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,3 +1384,6 @@ pub(crate) const CORRELATED_DERIVED: &str =
13841384
pub(crate) const AMBIGUOUS_SUFFIX: &str = "is ambiguous";
13851385
/// A JSON-held VARIANT as a sort key, in ARRAY_SORT's recorded order of kinds.
13861386
pub const VARIANT_ORDER_KEY: &str = "rule.variant_order_key";
1387+
1388+
/// The select item kinds whose Snowflake name keeps quotes or a path: literals and paths into semi-structured values.
1389+
pub(crate) const UNALIASED_QUOTED_KINDS: &[&str] = &["j_s_o_n_extract", "literal", "subscript"];

‎crates/sqlquack-diagnostics/src/diagnostics/constants.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ pub const CASE_ONLY_MATCH_HELP: &str = "Snowflake folds unquoted names to upper
1212
quote the name in the SQL to keep its case";
1313
/// The words of an unresolved name's reason when two result columns share a name.
1414
pub const REPEATED_RESULT_NAME: &str = "repeats an earlier result column's name";
15+
/// The label of an unaliased item Snowflake names after its text.
16+
pub const UNALIASED_TEXT_NAME: &str = "Snowflake names this column after its text";
17+
/// The label of an unaliased literal or path, whose Snowflake name keeps its quotes or path, so no alias is suggested.
18+
pub const UNALIASED_QUOTED_NAME: &str =
19+
"Snowflake names this column after its full text, quotes and path included";
20+
/// The help for an unaliased literal or path.
21+
pub const UNALIASED_QUOTED_NAME_HELP: &str = "add an alias, which names the column in both engines";
1522
/// The help for such a name, replacing SQ0004's own.
1623
pub const REPEATED_RESULT_NAME_HELP: &str =
1724
"give each result column its own alias, such as `SELECT 1 AS first, 1 AS second`";

‎crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use crate::diagnostics::constants::{
1010
CASE_ONLY_MATCH, CASE_ONLY_MATCH_HELP, CONVERSION_BUDGET, DIVERGENCE_REFUSED,
1111
FUNCTION_OPERAND_HELP, GUARD_PREFIX, JINJA_HELP, MISSING_RULE, NOT_EXPRESSIBLE, PARSE_FIDELITY,
1212
REPEATED_RESULT_NAME, REPEATED_RESULT_NAME_HELP, SUGGESTED_NAME_MAX_CHARS, TYPE_CONDITION,
13-
UNALIASED_OUTPUT_NAME, UNPARSED_PREFIX, UNRESOLVED_NAME, ZONED_LOCAL_TIME,
14-
ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE,
13+
UNALIASED_OUTPUT_NAME, UNALIASED_QUOTED_NAME, UNALIASED_QUOTED_NAME_HELP, UNPARSED_PREFIX,
14+
UNRESOLVED_NAME, ZONED_LOCAL_TIME, ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE,
1515
};
1616
use crate::diagnostics::models::Diagnostic;
1717
use crate::refusals::models::{Refusal, RefusalCategory};
@@ -52,7 +52,9 @@ pub fn diagnose(refusals: &[Refusal], source: &str) -> Vec<Diagnostic> {
5252
{
5353
diagnostic.help = Some(FUNCTION_OPERAND_HELP.to_owned());
5454
}
55-
if code == UNALIASED_OUTPUT_NAME
55+
if code == UNALIASED_OUTPUT_NAME && refusal.reason.contains(UNALIASED_QUOTED_NAME) {
56+
diagnostic.help = Some(UNALIASED_QUOTED_NAME_HELP.to_owned());
57+
} else if code == UNALIASED_OUTPUT_NAME
5658
&& let Some(name) = diagnostic
5759
.span
5860
.and_then(|span| suggested_name(source, span.start, span.end))

‎crates/sqlquack/tests/cli/test_convert.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1566,7 +1566,7 @@ fn given_refused_sql_when_converting_then_text_and_json_carry_the_same_coded_dia
15661566
expected_text: concat!(
15671567
"error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ",
15681568
"Snowflake gives it\n --> <stdin>:1:18\n |\n1 | SELECT 'a' AS a, UPPER('é')\n",
1569-
" | ^^^^^^^^^^ Snowflake names this `function` column after its text\n",
1569+
" | ^^^^^^^^^^ Snowflake names this column after its text\n",
15701570
" |\n = help: add an alias, such as `AS total`, which names the column in both engines\n",
15711571
"sqlquack convert: refused, 1 refusal\n",
15721572
),

‎crates/sqlquack/tests/cli/test_output_names.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -489,8 +489,8 @@ fn given_a_refused_result_name_when_converting_then_the_diagnostic_underlines_th
489489
expected_text: concat!(
490490
"error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ",
491491
"Snowflake gives it\n --> <stdin>:1:18\n |\n1 | SELECT order_id, 'it''s' FROM orders\n",
492-
" | ^^^^^^^ Snowflake names this `literal` column after its text\n",
493-
" |\n = help: add an alias, such as `AS \"'IT''S'\"`, which names the column in both engines\n",
492+
" | ^^^^^^^ Snowflake names this column after its full text, quotes and ",
493+
"path included\n |\n = help: add an alias, which names the column in both engines\n",
494494
"sqlquack convert: refused, 1 refusal\n",
495495
),
496496
expected_span: ("SQ0009", 17, 24),

0 commit comments

Comments
 (0)