[tracker/rtcom: 3/5] SPARQL: Fix variable handling in scalar subqueries

[Jürg Billeter <juergbi src gnome org>]

commit 704a68466874da6dea71db0b49a5598eaafeeb31
Author: Jürg Billeter <j bitron ch>
Date:   Mon May 17 13:02:11 2010 +0200

    SPARQL: Fix variable handling in scalar subqueries
    
    Scalar subqueries may only capture variables of very specific outer
    scopes.

 src/libtracker-data/tracker-sparql-pattern.vala    |   32 ++++++++++----------
 src/libtracker-data/tracker-sparql-query.vala      |   17 ++++-------
 .../libtracker-data/algebra/two-nested-opt-alt.rq  |    2 +-
 tests/libtracker-data/algebra/two-nested-opt.rq    |    2 +-
 4 files changed, 24 insertions(+), 29 deletions(-)
---
diff --git a/src/libtracker-data/tracker-sparql-pattern.vala b/src/libtracker-data/tracker-sparql-pattern.vala
index 552d423..008faf0 100644
--- a/src/libtracker-data/tracker-sparql-pattern.vala
+++ b/src/libtracker-data/tracker-sparql-pattern.vala
@@ -266,7 +266,7 @@ class Tracker.Sparql.Pattern : Object {
 		set_location (select_variables_location);
 
 		// report use of undefined variables
-		foreach (var variable in context.var_map.get_values ()) {
+		foreach (var variable in context.var_set.get_keys ()) {
 			if (variable.binding == null) {
 				throw get_error ("use of undefined variable `%s'".printf (variable.name));
 			}
@@ -277,7 +277,7 @@ class Tracker.Sparql.Pattern : Object {
 
 		bool first = true;
 		if (accept (SparqlTokenType.STAR)) {
-			foreach (var variable in context.var_map.get_values ()) {
+			foreach (var variable in context.var_set.get_keys ()) {
 				if (!first) {
 					sql.append (", ");
 				} else {
@@ -1091,30 +1091,30 @@ class Tracker.Sparql.Pattern : Object {
 		if (triple_context != null) {
 			binding_list = triple_context.var_bindings.lookup (variable);
 		}
-		if (binding_list == null && context.in_scalar_subquery) {
-			// in scalar subquery: check variables of outer queries
-			var parent_context = context.parent_context;
-			while (parent_context != null) {
-				var outer_var = parent_context.var_map.lookup (variable.name);
-				if (outer_var != null && outer_var.binding != null) {
+		if (binding_list == null && variable.binding != null) {
+			// might be in scalar subquery: check variables of outer queries
+			var current_context = context;
+			while (current_context != null) {
+				// only allow access to variables of immediate parent context of the subquery
+				// allowing access to other variables leads to invalid SQL or wrong results
+				if (current_context.scalar_subquery && current_context.parent_context.var_set.lookup (variable) != 0) {
 					// capture outer variable
 					var binding = new VariableBinding ();
-					binding.data_type = outer_var.binding.data_type;
+					binding.data_type = variable.binding.data_type;
 					binding.variable = context.get_variable (variable.name);
-					binding.type = outer_var.binding.type;
-					binding.sql_expression = outer_var.sql_expression;
+					binding.type = variable.binding.type;
+					binding.sql_expression = variable.sql_expression;
 					binding_list = new VariableBindingList ();
 					if (triple_context != null) {
-						triple_context.variables.append (binding.variable);
-						triple_context.var_bindings.insert (binding.variable, binding_list);
+						triple_context.variables.append (variable);
+						triple_context.var_bindings.insert (variable, binding_list);
 					}
 
-					context.var_set.insert (binding.variable, VariableState.BOUND);
+					context.var_set.insert (variable, VariableState.BOUND);
 					binding_list.list.append (binding);
-					binding.variable.binding = binding;
 					break;
 				}
-				parent_context = parent_context.parent_context;
+				current_context = current_context.parent_context;
 			}
 		}
 		return binding_list;
diff --git a/src/libtracker-data/tracker-sparql-query.vala b/src/libtracker-data/tracker-sparql-query.vala
index 4cf5cbd..0a9867b 100644
--- a/src/libtracker-data/tracker-sparql-query.vala
+++ b/src/libtracker-data/tracker-sparql-query.vala
@@ -113,7 +113,7 @@ namespace Tracker.Sparql {
 		// Keep track of used sql identifiers to avoid using the same for multiple SPARQL variables
 		public HashTable<string,bool> used_sql_identifiers;
 
-		public bool in_scalar_subquery;
+		public bool scalar_subquery;
 
 		public Context (Context? parent_context = null) {
 			this.parent_context = parent_context;
@@ -129,7 +129,6 @@ namespace Tracker.Sparql {
 				var_map = parent_context.var_map;
 				predicate_variable_map = parent_context.predicate_variable_map;
 				used_sql_identifiers = parent_context.used_sql_identifiers;
-				in_scalar_subquery = parent_context.in_scalar_subquery;
 			}
 		}
 
@@ -138,10 +137,10 @@ namespace Tracker.Sparql {
 			this.var_set = new HashTable<Variable,int>.full (direct_hash, direct_equal, g_object_unref, null);
 
 			select_var_set = new HashTable<Variable,int>.full (direct_hash, direct_equal, g_object_unref, null);
-			var_map = new HashTable<string,Variable>.full (str_hash, str_equal, g_free, g_object_unref);
+			var_map = parent_context.var_map;
 			predicate_variable_map = new HashTable<Variable,PredicateVariable>.full (direct_hash, direct_equal, g_object_unref, g_object_unref);
 			used_sql_identifiers = new HashTable<string,bool>.full (str_hash, str_equal, g_free, null);
-			in_scalar_subquery = true;
+			scalar_subquery = true;
 		}
 
 		internal unowned Variable get_variable (string name) {
@@ -525,16 +524,12 @@ public class Tracker.Sparql.Query : Object {
 	string get_select_query () throws DBInterfaceError, SparqlError, DateError {
 		// SELECT query
 
-		context = new Context ();
-
 		// build SQL
 		var sql = new StringBuilder ();
 		pattern.translate_select (sql);
 
 		expect (SparqlTokenType.EOF);
 
-		context = context.parent_context;
-
 		return sql.str;
 	}
 
@@ -645,7 +640,7 @@ public class Tracker.Sparql.Query : Object {
 		// build SQL
 		sql.append ("SELECT ");
 		bool first = true;
-		foreach (var variable in context.var_map.get_values ()) {
+		foreach (var variable in context.var_set.get_keys ()) {
 			if (!first) {
 				sql.append (", ");
 			} else {
@@ -688,10 +683,10 @@ public class Tracker.Sparql.Query : Object {
 				// get values of all variables to be bound
 				var var_value_map = new HashTable<string,string>.full (str_hash, str_equal, g_free, g_free);
 				int var_idx = 0;
-				foreach (string var_name in context.var_map.get_keys ()) {
+				foreach (var variable in context.var_set.get_keys ()) {
 					Value value;
 					result_set._get_value (var_idx++, out value);
-					var_value_map.insert (var_name, get_string_for_value (value));
+					var_value_map.insert (variable.name, get_string_for_value (value));
 				}
 
 				set_location (template_location);
diff --git a/tests/libtracker-data/algebra/two-nested-opt-alt.rq b/tests/libtracker-data/algebra/two-nested-opt-alt.rq
index 3c6820d..82125a2 100644
--- a/tests/libtracker-data/algebra/two-nested-opt-alt.rq
+++ b/tests/libtracker-data/algebra/two-nested-opt-alt.rq
@@ -2,7 +2,7 @@ PREFIX :    <http://example/>
 
 ## The nested optional example, rewritten to a form that is the same
 ## for the SPARQL algebra and the declarative semantics.
-SELECT *
+SELECT ?v ?w
 { 
     :x1 :p ?v .
     OPTIONAL { :x3 :q ?w }
diff --git a/tests/libtracker-data/algebra/two-nested-opt.rq b/tests/libtracker-data/algebra/two-nested-opt.rq
index 71b4046..39d351b 100644
--- a/tests/libtracker-data/algebra/two-nested-opt.rq
+++ b/tests/libtracker-data/algebra/two-nested-opt.rq
@@ -1,6 +1,6 @@
 PREFIX :    <http://example/>
 
-SELECT *
+SELECT ?v ?w
 { 
     :x1 :p ?v .
     OPTIONAL