fix: charge json_fidelity only for VARIANT columns a statement may read

crates/sqlquack-rules/src/catalogue/constants.rs

@@ -85,7 +85,7 @@ pub const RULES: &[RuleSpec] = &[
     RuleSpec {
         id: FROM_RELATION,
         constructs: &["node:table"],
-        conditions: "The relation is a CTE or has typed columns in the schema.",
+        conditions: "The relation is a CTE or has typed columns in the schema. A base table's VARIANT, OBJECT or ARRAY column the statement may read, by name, through a star other than COUNT(*)'s, or by a NATURAL join, is guarded as PARSE_JSON's result is and needs json_fidelity; a column it never reads needs nothing.",
         failures: &[],
     },
     RuleSpec {

crates/sqlquack-rules/src/conversion/_helpers/expressions/semi_structured.rs

@@ -84,9 +84,8 @@ pub(crate) fn parse_json(
 }
 
 /// A VARIANT column of a base table, guarded as PARSE_JSON's result is.
-pub(crate) fn guarded_column(sql: &str, context: &Context<'_>) -> Result<String, Refusal> {
-    diverge(context, JSON_FIDELITY, &node_construct("column"))?;
-    Ok(json_guarded(sql, false, context.log))
+pub(crate) fn guarded_column(sql: &str, log: &ProvenanceLog) -> String {
+    json_guarded(sql, false, log)
 }
 
 /// `v:a.b[0]`: a literal path into a VARIANT.

crates/sqlquack-rules/src/conversion/_helpers/queries/sources.rs

@@ -9,6 +9,7 @@ use sqlquack_core::refusals::main::outside_conditions::outside_conditions;
 use sqlquack_core::refusals::main::unresolved::unresolved;
 use sqlquack_core::refusals::models::{Refusal, RefusalCategory};
 use sqlquack_duckdb::blocks::main::quoted_identifier::quoted_identifier;
+use sqlquack_duckdb::semi_structured::constants::JSON_FIDELITY;
 use sqlquack_snowflake::typing::models::SnowflakeType;
 
 use crate::conversion::_helpers::expressions::project_functions::{exact_arguments, held_type};
@@ -21,11 +22,12 @@ use crate::conversion::_helpers::queries::query::{convert_query, require_distinc
 use crate::conversion::_helpers::support::collecting::{
     collecting, events, opaque, record, recorded, replace_since, suspended, unresolved_since,
 };
+use crate::conversion::_helpers::support::divergence::diverge;
 use crate::conversion::_helpers::support::scope::{Context, Scope};
 use crate::conversion::_helpers::support::site::{Placement, Site};
 use crate::conversion::_helpers::support::tree::{
-    Payload, field, function_construct, identifier_name, integer_literal, list, node_construct,
-    node_kind, payload_object, require_understood,
+    Payload, field, function_construct, identifier_name, integer_literal, list, may_read_column,
+    node_construct, node_kind, payload_object, require_understood,
 };
 use crate::conversion::constants::{
     ASOF_JOIN, ASOF_KIND, CORRELATED_DERIVED, FROM_RELATION, FROM_SUBQUERY, GENERATOR_MAX_ROWS,
@@ -563,8 +565,11 @@ fn table_source(
         .iter()
         .filter(|column| !is_cte && column.data_type == SnowflakeType::SemiStructured)
     {
+        if may_read_column(context.locator.tree(), &column.name) {
+            diverge(context, JSON_FIDELITY, &node_construct("column"))?;
+        }
         let quoted = quoted_identifier(&column.name, log);
-        guarded.push(format!("{} AS {quoted}", guarded_column(&quoted, context)?));
+        guarded.push(format!("{} AS {quoted}", guarded_column(&quoted, log)));
     }
     let relation = match guarded.is_empty() {
         true => quoted_identifier(&name, log),

crates/sqlquack-rules/src/conversion/_helpers/support/locations.rs

@@ -52,6 +52,11 @@ impl<'a> Locator<'a> {
         }
     }
 
+    /// The statement tree the conversion reads.
+    pub(crate) fn tree(&self) -> &'a Value {
+        self.tree
+    }
+
     /// `sql`, converted from `node`, with its unidentified raises identified and located at `node`.
     pub(crate) fn identified(&self, sql: &str, node: &Value) -> String {
         let (sql, ids) = identify_raises(sql);

crates/sqlquack-rules/src/conversion/_helpers/support/tree.rs

@@ -47,6 +47,12 @@ pub(crate) type Payload = Map<String, Value>;
 pub(crate) const LITERAL_NUMBER: &str = "number";
 /// The parser's literal type for strings.
 pub(crate) const LITERAL_STRING: &str = "string";
+/// The join kind prefix of NATURAL joins, which compare every column the two sides share.
+const NATURAL_JOIN_KIND: &str = "Natural";
+/// The fields of an identifier node: its name and whether it was quoted.
+const IDENTIFIER_FIELDS: [&str; 2] = ["name", "quoted"];
+/// The field holding a call's arguments.
+const ARGUMENTS_FIELD: &str = "args";
 
 /// The kind and payload of a one-key node such as `{"add": {...}}`.
 pub(crate) fn node_kind(node: &Value) -> Option<(&str, &Value)> {
@@ -237,3 +243,62 @@ pub(crate) fn strip_positions(value: &Value) -> Value {
         other => other.clone(),
     }
 }
+
+/// Whether `tree` may read column `name`: by name, by a star other than COUNT(*)'s, or by NATURAL.
+pub(crate) fn may_read_column(tree: &Value, name: &str) -> bool {
+    match tree {
+        Value::Array(items) => items.iter().any(|item| may_read_column(item, name)),
+        Value::Object(fields) => {
+            if fields.len() == IDENTIFIER_FIELDS.len()
+                && IDENTIFIER_FIELDS
+                    .iter()
+                    .all(|field| fields.contains_key(*field))
+            {
+                return identifier_name(tree).is_some_and(|resolved| resolved == name);
+            }
+            match node_kind(tree) {
+                Some(("star", _)) => return true,
+                Some(("aggregate_function", payload)) if counts_rows(payload) => {
+                    return payload.as_object().is_some_and(|call| {
+                        call.iter().any(|(field, value)| {
+                            field != ARGUMENTS_FIELD && may_read_column(value, name)
+                        })
+                    });
+                }
+                _ => {}
+            }
+            if fields
+                .get("kind")
+                .and_then(Value::as_str)
+                .is_some_and(|kind| kind.starts_with(NATURAL_JOIN_KIND))
+                && fields.contains_key("using")
+            {
+                return true;
+            }
+            fields.values().any(|value| may_read_column(value, name))
+        }
+        _ => false,
+    }
+}
+
+/// Whether an aggregate call is COUNT(*) with a bare star, which counts rows and reads no column.
+fn counts_rows(payload: &Value) -> bool {
+    let named_count = payload
+        .get("name")
+        .and_then(Value::as_str)
+        .is_some_and(|called| called.eq_ignore_ascii_case("count"));
+    let bare_star = match payload
+        .get("args")
+        .and_then(Value::as_array)
+        .map(Vec::as_slice)
+    {
+        Some([argument]) => match node_kind(argument) {
+            Some(("star", star)) => star
+                .as_object()
+                .is_some_and(|star| star.values().all(Value::is_null)),
+            _ => false,
+        },
+        _ => false,
+    };
+    named_count && bare_star
+}

crates/sqlquack/tests/cli/test_convert.rs

@@ -9,7 +9,7 @@ use crate::test_types::{
     ConvertedRunTestCase, CountedCallsTestCase, CrossingTestCase, DedicatedOverrideTestCase,
     DeepChainTestCase, DivergenceHelpTestCase, FailureKindTestCase, OutputFormatTestCase,
     OverrideFailureTestCase, OverrideTestCase, PolicyFreeTestCase, PythonRefusalTestCase,
-    PythonRunTestCase, RunErrorTestCase, SchemaErrorTestCase, ValueTestCase,
+    PythonRunTestCase, ReadColumnTestCase, RunErrorTestCase, SchemaErrorTestCase, ValueTestCase,
 };
 use sha2::{Digest, Sha256};
 
@@ -1759,3 +1759,64 @@ fn given_a_malformed_schema_when_converting_then_the_error_names_what_is_wrong_i
         );
     }
 }
+
+#[test]
+fn given_a_table_with_an_unread_variant_column_when_converting_strictly_then_json_fidelity_is_not_needed()
+ {
+    let directory = tempfile::tempdir().expect("temporary directory");
+    let schema = directory.path().join("schema.json");
+    std::fs::write(
+        &schema,
+        r#"{"relations": {"TICKETS": [["TICKET_ID", "NUMBER(38,0)"], ["SUBJECT", "VARCHAR"], ["TAGS", "ARRAY"]], "ORDERS": [["ORDER_ID", "NUMBER(38,0)"]]}}"#,
+    )
+    .expect("schema written");
+    let schema = schema.to_string_lossy().into_owned();
+    let test_cases = [
+        ReadColumnTestCase {
+            description: "a scalar column of a table with an unread ARRAY column",
+            sql: "SELECT ticket_id FROM tickets",
+            expected_json_fidelity: false,
+        },
+        ReadColumnTestCase {
+            description: "a UNION ALL whose branch reads only scalar columns",
+            sql: "SELECT order_id FROM orders UNION ALL SELECT ticket_id FROM tickets",
+            expected_json_fidelity: false,
+        },
+        ReadColumnTestCase {
+            description: "the ARRAY column named in the select list",
+            sql: "SELECT ticket_id, tags FROM tickets",
+            expected_json_fidelity: true,
+        },
+        ReadColumnTestCase {
+            description: "the ARRAY column read only in WHERE",
+            sql: "SELECT subject FROM tickets WHERE ARRAY_SIZE(tags) > 0",
+            expected_json_fidelity: true,
+        },
+        ReadColumnTestCase {
+            description: "a star, which reads every column",
+            sql: "SELECT * FROM tickets",
+            expected_json_fidelity: true,
+        },
+        ReadColumnTestCase {
+            description: "a NATURAL join, which compares columns without naming them",
+            sql: "SELECT t.ticket_id FROM tickets t NATURAL JOIN orders o",
+            expected_json_fidelity: true,
+        },
+    ];
+    for test_case in test_cases {
+        let (stdout, stderr) =
+            convert_streams(test_case.sql, &["--builtin-policy", "--schema", &schema]);
+        assert_eq!(
+            (
+                stderr.contains("json_fidelity"),
+                stdout.trim_start().starts_with(['S', '('])
+            ),
+            (
+                test_case.expected_json_fidelity,
+                !test_case.expected_json_fidelity
+            ),
+            "{}: {stderr}",
+            test_case.description
+        );
+    }
+}

crates/sqlquack/tests/cli/test_types.rs

@@ -293,3 +293,9 @@ pub(crate) struct SchemaErrorTestCase {
     pub(crate) schema: &'static str,
     pub(crate) expected_error: &'static str,
 }
+
+pub(crate) struct ReadColumnTestCase {
+    pub(crate) description: &'static str,
+    pub(crate) sql: &'static str,
+    pub(crate) expected_json_fidelity: bool,
+}