diff --git a/query-by-graph/src/components/EntitySelectorInputControl.vue b/query-by-graph/src/components/EntitySelectorInputControl.vue index 6969d96..a6df931 100644 --- a/query-by-graph/src/components/EntitySelectorInputControl.vue +++ b/query-by-graph/src/components/EntitySelectorInputControl.vue @@ -16,6 +16,15 @@ container-classes="px-2" label-classes="text-white" /> + @@ -50,6 +59,19 @@ export default { } } } + }, + includeAsDistinct: { + get() { + return this.data.value?.distinct === true; + }, + set(value) { + if (this.data.value) { + this.data.value.distinct = value; + if (this.data.options?.change) { + this.data.options.change(this.data.value); + } + } + } } }, methods: { @@ -72,6 +94,9 @@ export default { if (this.data.value.selectedForProjection === undefined) { this.data.value.selectedForProjection = true; } + if (this.data.value.distinct === undefined) { + this.data.value.distinct = false; + } } }, } diff --git a/query-by-graph/src/components/ProjectionCheckbox.vue b/query-by-graph/src/components/ProjectionCheckbox.vue index e3ff476..f7d1997 100644 --- a/query-by-graph/src/components/ProjectionCheckbox.vue +++ b/query-by-graph/src/components/ProjectionCheckbox.vue @@ -8,7 +8,7 @@ class="w-4 h-4 text-blue-600 bg-gray-100 border-gray-300 rounded focus:ring-blue-500 focus:ring-2" /> @@ -27,6 +27,10 @@ defineProps({ type: Boolean, default: false }, + label: { + type: String, + default: 'Select?' + }, containerClasses: { type: String, default: '' diff --git a/query-by-graph/src/lib.rs b/query-by-graph/src/lib.rs index c104646..f38459d 100644 --- a/query-by-graph/src/lib.rs +++ b/query-by-graph/src/lib.rs @@ -6,7 +6,7 @@ use serde_json::{from_str, to_string}; use spargebra::algebra::{GraphPattern, PropertyPathExpression}; use spargebra::term::{TriplePattern, TermPattern, NamedNodePattern}; use spargebra::{Query, SparqlSyntaxError}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use wasm_bindgen::prelude::*; const INDENTATION_COUNT: usize = 4; @@ -21,6 +21,8 @@ pub struct Entity { pub prefix: Prefix, #[serde(default = "default_selected_for_projection")] pub selected_for_projection: bool, + #[serde(default)] + pub distinct: bool, } #[derive(Serialize, Deserialize, Clone)] @@ -122,20 +124,22 @@ fn vqg_to_query( if connections.is_empty() { String::from("") } else { - fn collect_vars(id: &str, selected: bool, add_service_statement: bool) -> Vec { + fn collect_vars(id: &str, selected: bool, distinct: bool, add_service_statement: bool) -> Vec<(String, bool)> { let mut vars = Vec::new(); if id.starts_with('?') && selected { let var = id.to_string(); - vars.push(var.clone()); + vars.push((var.clone(), distinct)); if add_service_statement { - vars.push(format!("?{}Label", var.trim_start_matches('?'))); + // Label variables are never marked distinct + vars.push((format!("?{}Label", var.trim_start_matches('?')), false)); } } vars } - fn collect_vars_from_property(property: &Property, add_service_statement: bool) -> Vec { - let mut vars = collect_vars(&property.id, property.selected_for_projection, add_service_statement); + fn collect_vars_from_property(property: &Property, add_service_statement: bool) -> Vec<(String, bool)> { + // Properties never carry the distinct flag + let mut vars = collect_vars(&property.id, property.selected_for_projection, false, add_service_statement); for p in &property.properties { vars.extend(collect_vars_from_property(p, add_service_statement)); } @@ -153,26 +157,45 @@ fn vqg_to_query( prefixes } - let projection_set = connections + // Collect (variable, is_distinct) pairs; deduplicate with OR on the distinct flag + let projection_raw: Vec<(String, bool)> = connections .iter() .flat_map(|connection| { - let mut vars = Vec::new(); + let mut vars: Vec<(String, bool)> = Vec::new(); for entity in &[&connection.source, &connection.target] { - vars.extend(collect_vars(&entity.id, entity.selected_for_projection, add_service_statement)); + vars.extend(collect_vars(&entity.id, entity.selected_for_projection, entity.distinct, add_service_statement)); } for property in &connection.properties { vars.extend(collect_vars_from_property(property, add_service_statement)); } vars }) - .collect::>(); + .collect(); - let projection_list = if projection_set.is_empty() { + let mut projection_map: HashMap = HashMap::new(); + for (var, is_distinct) in projection_raw { + projection_map + .entry(var) + .and_modify(|d| *d = *d || is_distinct) + .or_insert(is_distinct); + } + + let projection_list = if projection_map.is_empty() { String::from("*") } else { - let mut sorted_projection_set: Vec<_> = projection_set.into_iter().collect(); - sorted_projection_set.sort(); // Sort the collection - sorted_projection_set.join(" ") + let mut sorted: Vec<(String, bool)> = projection_map.into_iter().collect(); + sorted.sort_by_key(|(var, _)| var.clone()); + sorted + .iter() + .map(|(var, is_distinct)| { + if *is_distinct { + format!("DISTINCT({})", var) + } else { + var.clone() + } + }) + .collect::>() + .join(" ") }; let prefix_set = connections @@ -400,12 +423,21 @@ fn query_to_vqg(query: &str) -> Vec { match parsed_query { Ok(Query::Select { pattern: p, .. }) => { let (connections, projection_vars) = match p { + // SELECT DISTINCT ... is treated as SELECT ... for import purposes; + // per-variable distinct is set by the user via the UI checkbox. + GraphPattern::Distinct { inner } => match *inner { + GraphPattern::Project { variables: v, inner: i } => ( + match_bgp_or_path_to_vqg(*i), + Some(v.iter().map(|var| format!("?{}", var.as_str())).collect::>()), + ), + other => (match_bgp_or_path_to_vqg(other), None), + }, GraphPattern::Project { variables: v, inner: i, } => ( match_bgp_or_path_to_vqg(*i), - Some(v.iter().map(|var| format!("?{}", var.as_str())).collect::>()) + Some(v.iter().map(|var| format!("?{}", var.as_str())).collect::>()), ), _ => (match_bgp_or_path_to_vqg(p), None), }; @@ -472,6 +504,7 @@ fn match_bgp_or_path_to_vqg(p: GraphPattern) -> Vec { let r_parsed = match_bgp_or_path_to_vqg(*r); l_parsed.into_iter().chain(r_parsed).collect() } + GraphPattern::Distinct { inner } => match_bgp_or_path_to_vqg(*inner), GraphPattern::Path { subject: s, path: p, @@ -503,6 +536,7 @@ fn connection_constructor( abbreviation: "".to_string(), }, selected_for_projection: true, // Default to true + distinct: false, }, target: Entity { id: object_name.clone(), @@ -512,6 +546,7 @@ fn connection_constructor( abbreviation: "".to_string(), }, selected_for_projection: true, // Default to true + distinct: false, }, properties: vec![Property { id: predicate_name.clone(), diff --git a/query-by-graph/src/lib/rete/constants.ts b/query-by-graph/src/lib/rete/constants.ts index ab07407..6772b7d 100644 --- a/query-by-graph/src/lib/rete/constants.ts +++ b/query-by-graph/src/lib/rete/constants.ts @@ -11,7 +11,8 @@ const variableEntity: EntityType = { // EntityType abbreviation: "", }, dataSource: noDataSource, - selectedForProjection: true + selectedForProjection: true, + distinct: false }; const variableEntityConstructor = (name: string): EntityType => { diff --git a/query-by-graph/src/lib/types/EntityType.ts b/query-by-graph/src/lib/types/EntityType.ts index 11865d5..34e0bdd 100644 --- a/query-by-graph/src/lib/types/EntityType.ts +++ b/query-by-graph/src/lib/types/EntityType.ts @@ -7,6 +7,7 @@ export interface EntityType { prefix: PrefixType, dataSource: WikibaseDataSource; selectedForProjection?: boolean; + distinct?: boolean; isLiteral?: boolean; } diff --git a/query-by-graph/tests/logic.rs b/query-by-graph/tests/logic.rs index 70e940b..6d1dc9b 100644 --- a/query-by-graph/tests/logic.rs +++ b/query-by-graph/tests/logic.rs @@ -324,3 +324,103 @@ SELECT ?o ?s WHERE { }"###; assert_sparql_equivalent(&result, expected); } + +#[test] +fn test_distinct_variable_generates_per_variable_distinct() { + let graph = r###"[{"properties":[{"id":"P69","label":"educated at","prefix":{"iri":"http://www.wikidata.org/prop/direct/","abbreviation":"wdt"},"selectedForProjection":false}],"source":{"id":"Q5879","label":"Johann Wolfgang von Goethe","prefix":{"iri":"http://www.wikidata.org/entity/","abbreviation":"wd"},"selectedForProjection":false,"distinct":false},"target":{"id":"?university","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":true,"distinct":true}}]"###; + + let result = vqg_to_query_wasm(graph, false, false); + let select = select_line(&result); + + // Per-variable DISTINCT: SELECT DISTINCT(?university) WHERE { ... } + assert!(select.contains("DISTINCT(?university)"), "Expected DISTINCT(?university) in: {}", select); + // SELECT keyword itself should not be followed by the global DISTINCT keyword + assert!(!select.starts_with("SELECT DISTINCT "), "Should not use global SELECT DISTINCT, got: {}", select); +} + +#[test] +fn test_no_distinct_generates_plain_select() { + let graph = r###"[{"properties":[{"id":"P69","label":"educated at","prefix":{"iri":"http://www.wikidata.org/prop/direct/","abbreviation":"wdt"},"selectedForProjection":false}],"source":{"id":"Q5879","label":"Johann Wolfgang von Goethe","prefix":{"iri":"http://www.wikidata.org/entity/","abbreviation":"wd"},"selectedForProjection":false,"distinct":false},"target":{"id":"?university","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":true,"distinct":false}}]"###; + + let result = vqg_to_query_wasm(graph, false, false); + let select = select_line(&result); + + assert!(!select.contains("DISTINCT"), "Expected plain SELECT but got: {}", select); + assert!(select.contains("?university")); +} + +#[test] +fn test_parse_select_distinct_query_does_not_set_per_variable_distinct_flag() { + // Importing SELECT DISTINCT ?university does NOT set distinct=true on individual variables. + // Per-variable distinct is only set via the UI checkbox; the global DISTINCT keyword is ignored on import. + let query = r###"PREFIX wd: +PREFIX wdt: +SELECT DISTINCT ?university WHERE { + wd:Q5879 wdt:P69 ?university . +}"###; + + let result = query_to_vqg_wasm(query); + let connections = parse_connections_json(&result); + + assert_eq!(connections.len(), 1); + let c = &connections[0]; + + assert_eq!(c["target"]["id"], Value::String("?university".to_string())); + assert_eq!(c["target"]["selectedForProjection"], Value::Bool(true)); + assert_eq!(c["target"]["distinct"], Value::Bool(false)); +} + +#[test] +fn test_parse_non_distinct_query_does_not_set_distinct_flag() { + let query = r###"PREFIX wd: +PREFIX wdt: +SELECT ?university WHERE { + wd:Q5879 wdt:P69 ?university . +}"###; + + let result = query_to_vqg_wasm(query); + let connections = parse_connections_json(&result); + + assert_eq!(connections.len(), 1); + let c = &connections[0]; + + assert_eq!(c["target"]["id"], Value::String("?university".to_string())); + assert_eq!(c["target"]["distinct"], Value::Bool(false)); +} + +#[test] +fn test_distinct_only_applies_when_variable_is_selected_for_projection() { + // distinct=true but selectedForProjection=false → DISTINCT(?var) must NOT appear + let graph = r###"[{"properties":[{"id":"P69","label":"educated at","prefix":{"iri":"http://www.wikidata.org/prop/direct/","abbreviation":"wdt"},"selectedForProjection":false}],"source":{"id":"Q5879","label":"Goethe","prefix":{"iri":"http://www.wikidata.org/entity/","abbreviation":"wd"},"selectedForProjection":false,"distinct":false},"target":{"id":"?university","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":false,"distinct":true}}]"###; + + let result = vqg_to_query_wasm(graph, false, false); + + assert!(!result.contains("DISTINCT"), "DISTINCT should not appear when variable is not selected for projection: {}", result); +} + +#[test] +fn test_mixed_distinct_and_non_distinct_variables() { + // ?university is distinct, ?person is not → only ?university gets DISTINCT(...) + let graph = r###"[{"properties":[{"id":"P69","label":"educated at","prefix":{"iri":"http://www.wikidata.org/prop/direct/","abbreviation":"wdt"},"selectedForProjection":false}],"source":{"id":"?person","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":true,"distinct":false},"target":{"id":"?university","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":true,"distinct":true}}]"###; + + let result = vqg_to_query_wasm(graph, false, false); + let select = select_line(&result); + + assert!(select.contains("DISTINCT(?university)"), "Expected DISTINCT(?university) in: {}", select); + // ?person should appear as a plain variable (no DISTINCT wrapper) + assert!(select.contains("?person"), "Expected ?person in: {}", select); + assert!(!select.contains("DISTINCT(?person)"), "?person should not be wrapped in DISTINCT: {}", select); +} + +#[test] +fn test_label_vars_not_distinct_even_when_base_var_is_distinct() { + // With label service, ?universityLabel must NOT be wrapped in DISTINCT + let graph = r###"[{"properties":[{"id":"P69","label":"educated at","prefix":{"iri":"http://www.wikidata.org/prop/direct/","abbreviation":"wdt"},"selectedForProjection":false}],"source":{"id":"Q5879","label":"Goethe","prefix":{"iri":"http://www.wikidata.org/entity/","abbreviation":"wd"},"selectedForProjection":false,"distinct":false},"target":{"id":"?university","label":"Variable","prefix":{"iri":"","abbreviation":""},"selectedForProjection":true,"distinct":true}}]"###; + + let result = vqg_to_query_wasm(graph, true, true); + let select = select_line(&result); + + assert!(select.contains("DISTINCT(?university)"), "Expected DISTINCT(?university) in: {}", select); + assert!(!select.contains("DISTINCT(?universityLabel)"), "Label variable must not be wrapped in DISTINCT: {}", select); + assert!(select.contains("?universityLabel"), "Label variable should still appear: {}", select); +}