zorba-coders team mailing list archive

[Markos Zaharioudakis <markos_za@xxxxxxxxx>]

Markos Zaharioudakis has proposed merging lp:~zorba-coders/zorba/markos-scratch into lp:zorba.

Requested reviews:
  Markos Zaharioudakis (markos-za)

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/95339

fixed bug involing eval and the no-copy rule
-- 
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/95339
Your team Zorba Coders is subscribed to branch lp:zorba.

=== modified file 'src/compiler/rewriter/rules/nodeid_rules.cpp'
--- src/compiler/rewriter/rules/nodeid_rules.cpp	2012-01-15 09:18:22 +0000
+++ src/compiler/rewriter/rules/nodeid_rules.cpp	2012-03-01 11:31:19 +0000
@@ -862,6 +862,22 @@
 
   case eval_expr_kind:
   {
+    eval_expr* e = static_cast<eval_expr*>(node);
+
+    // Conservatively assume that, when executed, the eval query will apply
+    // a "node-id-sensitive" operation on each of the in-scope variables, so
+    // these variables must be bound to statndalone trees.
+    csize numEvalVars = e->var_count();
+
+    for (csize i = 0; i < numEvalVars; ++i)
+    {
+      expr* arg = e->get_arg_expr(i);
+
+      std::vector<expr*> sources;
+      theSourceFinder->findNodeSources(arg, &udfCaller, sources);
+      markSources(sources);
+    }
+
     std::vector<var_expr_t> globalVars;
     node->get_sctx()->getVariables(globalVars, false, true);
   

=== modified file 'src/functions/udf.cpp'
--- src/functions/udf.cpp	2012-02-27 14:46:27 +0000
+++ src/functions/udf.cpp	2012-03-01 11:31:19 +0000
@@ -91,7 +91,7 @@
 ********************************************************************************/
 user_function::~user_function()
 {
-  if(theLocalUdfs != NULL)
+  if (theLocalUdfs != NULL)
     theLocalUdfs->remove(this);
 }
 
@@ -101,9 +101,11 @@
   uint32_t planStateSize;
   getPlan(compilerCB, planStateSize);
   std::vector<user_function*>::iterator udf_it;
-  for(udf_it=theMutuallyRecursiveUDFs.begin(); udf_it!=theMutuallyRecursiveUDFs.end();udf_it++)
+  for(udf_it = theMutuallyRecursiveUDFs.begin();
+      udf_it != theMutuallyRecursiveUDFs.end();
+      ++udf_it)
   {
-    if((*udf_it)->thePlan == NULL)
+    if ((*udf_it)->thePlan == NULL)
       (*udf_it)->prepare_for_serialize(compilerCB);
   }
 }
@@ -113,45 +115,19 @@
 ********************************************************************************/
 void user_function::serialize(::zorba::serialization::Archiver& ar)
 {
-  //bool  save_plan = true;
-  if(ar.is_serializing_out())
+  if (ar.is_serializing_out())
   {
     try
     {
-#if 0
-      // We shouldn't try to optimize the body during the process of serializing
-      // the query plan, because udfs are serialized in some "random" order, and
-      // as a result, they will be optimized in this random order, instead of a
-      // bottom up order. This can have undesired effect, like const-folding
-      // exprs that contain calls to functions that are non-deterministic, or
-      // access the dynamic context.
-      //
-      // As a result, we don't call getPlan() unless the optimizer is off or
-      // the udf has been optimized already. If getPlan is not called, only
-      // the body expr of the udf will be serialized. 
-      //
-      // Note: The compiler attempts to collect and optimize all udfs that
-      // may actually be invoked during the execution of a query. However,
-      // some such udfs may go undetected; for example, udfs that appear
-      // inside eval statements. Only such undetected udfs are affected by
-      // this if condition.
-      if (ar.compiler_cb->theConfig.opt_level == CompilerCB::config::O0 ||
-          theIsOptimized)
-      {
-        getPlan(ar.compiler_cb);
-      }
-#else
       //uint32_t planStateSize;
       //getPlan(ar.compiler_cb, planStateSize);
       assert(thePlan != NULL);
       ZORBA_ASSERT(thePlan != NULL);
-#endif
     }
     catch(...)
     {
       // cannot compile user defined function, maybe it is not even used,
       // so don't fire an error yet
-      //save_plan = false;
     }
   }
   else
@@ -169,15 +145,10 @@
   ar & theIsLeaf;
   ar & theMutuallyRecursiveUDFs;
   ar & theIsOptimized;
-  //ar.set_is_temp_field(true);
-  //ar & save_plan;
-  //ar.set_is_temp_field(false);
-  //if(save_plan)
   ar & thePlan;
   ar & thePlanStateSize;
   ar & theArgVarsRefs;
 
-  //+ar & theCache;
   ar & theCacheResults;
   ar & theCacheComputed;
 }
@@ -341,24 +312,6 @@
 /*******************************************************************************
 
 ********************************************************************************/
-BoolAnnotationValue user_function::mustCopyNodes(expr* fo, csize input) const
-{
-  BoolAnnotationValue callerMustCopy = fo->getMustCopyNodes();
-  BoolAnnotationValue argMustCopy = theArgVars[input]->getMustCopyNodes();
-
-  if (argMustCopy == ANNOTATION_TRUE)
-  {
-    // The decision depends on the caller
-    return callerMustCopy;
-  }
-
-  return argMustCopy;
-}
-
-
-/*******************************************************************************
-
-********************************************************************************/
 const std::vector<user_function::ArgVarRefs>& user_function::getArgVarsRefs() const
 {
   return theArgVarsRefs;

=== modified file 'src/functions/udf.h'
--- src/functions/udf.h	2012-02-16 12:48:17 +0000
+++ src/functions/udf.h	2012-03-01 11:31:19 +0000
@@ -138,7 +138,7 @@
   SERIALIZABLE_CLASS(user_function)
   user_function(::zorba::serialization::Archiver& ar);
   void serialize(::zorba::serialization::Archiver& ar);
-  void prepare_for_serialize(CompilerCB *compilerCB);
+  void prepare_for_serialize(CompilerCB* compilerCB);
 
 public:
   user_function(
@@ -146,7 +146,7 @@
       const signature& sig,
       expr_t expr_body,
       short kind,
-      CompilerCB  *compilerCB);
+      CompilerCB* compilerCB);
 
   virtual ~user_function();
 
@@ -192,8 +192,6 @@
 
   BoolAnnotationValue ignoresDuplicateNodes(expr* fo, csize input) const;
 
-  BoolAnnotationValue mustCopyNodes(expr* fo, csize input) const;
-
   PlanIter_t getPlan(CompilerCB* cb, uint32_t& planStateSize);
   
   void setPlaneStateSize(uint32_t size) { thePlanStateSize = size; }

=== added file 'test/rbkt/ExpQueryResults/zorba/eval/eval10.xml.res'
--- test/rbkt/ExpQueryResults/zorba/eval/eval10.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/eval/eval10.xml.res	2012-03-01 11:31:19 +0000
@@ -0,0 +1,1 @@
+<root><a><b><c/></b></a></root><a><b><c/></b></a><b><c/></b>

=== added file 'test/rbkt/Queries/zorba/eval/eval10.xq'
--- test/rbkt/Queries/zorba/eval/eval10.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/eval/eval10.xq	2012-03-01 11:31:19 +0000
@@ -0,0 +1,28 @@
+
+import module namespace refl = "http://www.zorba-xquery.com/modules/reflection";;
+
+
+declare function local:dummy($n as node())
+{
+};
+
+
+declare function local:foo($n as node()) as node()*
+{
+  local:dummy($n),
+  $n/ancestor::node()
+};
+
+
+declare variable $doc :=
+<a>
+  <b>
+    <c/>
+  </b>
+</a>
+;
+
+
+let $n := <root>{$doc}</root>
+return refl:eval("local:foo($n//c)")
+