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/94776

Fixed bug in codegen: do not create a materialize clause if one exists already
-- 
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/94776
Your team Zorba Coders is subscribed to branch lp:zorba.

=== modified file 'ChangeLog'
--- ChangeLog	2012-02-24 05:07:01 +0000
+++ ChangeLog	2012-02-27 14:49:38 +0000
@@ -29,6 +29,7 @@
   * Fixed bug #907872 (segfault when returning an input ItemSequence from an external function).
   * Fixed bug #905050 (setting and getting the context item type via the c++ api).
   * Added createDayTimeDuration, createYearMonthDuration, createDocumentNode, createCommentNode, createPiNode to api's ItemFactory.
+  * Fixed bug in codegen: do not create a materialize clause if one exists already
   * Added split function to the string module that allows for streamable tokenization but doesn't have regular expression
     support.
   * Fixed bug involving positional var and groupby 

=== modified file 'src/compiler/codegen/plan_visitor.cpp'
--- src/compiler/codegen/plan_visitor.cpp	2012-01-11 17:30:25 +0000
+++ src/compiler/codegen/plan_visitor.cpp	2012-02-27 14:49:38 +0000
@@ -875,7 +875,7 @@
 
   bool isGeneral = v.is_general();
 
-  ulong numClauses = v.num_clauses();
+  csize numClauses = v.num_clauses();
 
   if (v.is_sequential())
   {
@@ -883,9 +883,9 @@
     {
       if (v.has_sequential_clauses())
       {
-        ulong numForClauses = 0;
+        csize numForClauses = 0;
 
-        for (ulong i = 0; i < numClauses; ++i)
+        for (csize i = 0; i < numClauses; ++i)
         {
           const flwor_clause* c = v[i];
 
@@ -921,8 +921,11 @@
         }
       }
 
+      // Note: a materialize clause may exist already in case plan serialization
+      // is on (see comment in materialize_clause::clone)
       if (!isGeneral &&
           v.get_return_expr()->is_sequential() &&
+          v.get_clause(numClauses-1)->get_kind() != flwor_clause::materialize_clause &&
           (v.get_order_clause() != NULL || v.get_group_clause() == NULL))
       {
         materialize_clause* mat = 
@@ -938,8 +941,8 @@
       std::vector<OrderModifier> modifiers;
       std::vector<expr_t> orderingExprs;
 
-      ulong numForClauses = 0;
-      ulong i = 0;
+      csize numForClauses = 0;
+      csize i = 0;
 
       while (i < numClauses)
       {
@@ -1038,7 +1041,7 @@
     }
   }
 
-  for (ulong i = 0; i < numClauses; ++i)
+  for (csize i = 0; i < numClauses; ++i)
   {
     const flwor_clause* c = v[i];
 

=== modified file 'src/compiler/expression/expr_put.cpp'
--- src/compiler/expression/expr_put.cpp	2012-01-11 17:30:25 +0000
+++ src/compiler/expression/expr_put.cpp	2012-02-27 14:49:38 +0000
@@ -210,17 +210,19 @@
   PUT_SUB("IN", theDomainExpr);
 
 #else
-  os << indent << "FOR" << expr_addr(this) << " " << inc_indent;
+  os << indent << "FOR" << expr_addr(this) << " ";
 
   put_qname(theVarExpr->get_name(), os);
+
   os << expr_addr(theVarExpr.getp());
+
   if (thePosVarExpr != NULL)
   {
     os << " AT ";
     put_qname(thePosVarExpr->get_name(), os);
     os << expr_addr(thePosVarExpr.getp());
   }
-  os << endl << dec_indent << indent << "[\n" << inc_indent;
+  os << endl << indent << "[\n" << inc_indent;
 
   theDomainExpr->put(os);
 #endif
@@ -241,8 +243,12 @@
 
 #else
   os << indent << "LET" << expr_addr(this) << " ";
+
   put_qname(theVarExpr->get_name(), os);
-  os << expr_addr(theVarExpr.getp()) << " [\n" << inc_indent;
+
+  os << expr_addr(theVarExpr.getp());
+
+  os << endl << indent << " [\n" << inc_indent;
 
   theDomainExpr->put(os);
 #endif
@@ -313,9 +319,9 @@
 
   //os << indent << "ORDER BY ";
 
-  unsigned numColumns = num_columns();
+  csize numColumns = num_columns();
 
-  for (unsigned i = 0; i < numColumns; i++) 
+  for (csize i = 0; i < numColumns; i++) 
   {
     theOrderingExprs[i]->put(os);
   }
@@ -348,39 +354,58 @@
 {
   BEGIN_PUT(flwor_expr);
 
-  for (unsigned i = 0; i < num_clauses(); i++) 
+  for (csize i = 0; i < num_clauses(); i++) 
   {
     const flwor_clause& c = *((*this)[i]);
 
-    if (c.get_kind() == flwor_clause::where_clause)
+    switch (c.get_kind())
+    {
+    case flwor_clause::where_clause:
     {
       PUT_SUB( "WHERE", static_cast<const where_clause *>(&c)->get_expr() );
+      break;
     }
-    else if (c.get_kind() == flwor_clause::count_clause) 
+    case flwor_clause::count_clause: 
     {
       os << indent << "COUNT $"; 
       put_qname(static_cast<const count_clause *>(&c)->get_var()->get_name(), os);
       os << endl;
+      break;
     }
-    else if (c.get_kind() == flwor_clause::for_clause) 
+    case flwor_clause::for_clause:
     {
       static_cast<const for_clause *>(&c)->put(os);
+      break;
     }
-    else if (c.get_kind() == flwor_clause::let_clause) 
+    case flwor_clause::let_clause:
     {
       static_cast<const let_clause *>(&c)->put(os);
+      break;
     }
-    else if (c.get_kind() == flwor_clause::window_clause) 
+    case flwor_clause::window_clause:
     {
       static_cast<const window_clause *>(&c)->put(os);
+      break;
     }
-    else if (c.get_kind() == flwor_clause::group_clause) 
+    case flwor_clause::group_clause:
     {
       static_cast<const group_clause *>(&c)->put(os);
+      break;
     }
-    else if (c.get_kind() == flwor_clause::order_clause) 
+    case flwor_clause::order_clause:
     {
       static_cast<const orderby_clause *>(&c)->put(os);
+      break;
+    }
+    case flwor_clause::materialize_clause:
+    {
+      static_cast<const materialize_clause *>(&c)->put(os);
+      break;
+    }
+    default:
+    {
+      ZORBA_ASSERT(false);
+    }
     }
   }
 

=== modified file 'src/functions/udf.cpp'
--- src/functions/udf.cpp	2012-02-16 12:48:17 +0000
+++ src/functions/udf.cpp	2012-02-27 14:49:38 +0000
@@ -51,7 +51,7 @@
     const signature& sig,
     expr_t expr_body,
     short scriptingKind,
-    CompilerCB  *compilerCB)
+    CompilerCB* compilerCB)
   :
   function(sig, FunctionConsts::FN_UNKNOWN),
   theLoc(loc),

=== modified file 'src/runtime/core/flwor_iterator.cpp'
--- src/runtime/core/flwor_iterator.cpp	2012-02-15 11:53:19 +0000
+++ src/runtime/core/flwor_iterator.cpp	2012-02-27 14:49:38 +0000
@@ -343,13 +343,14 @@
 void MaterializeClause::serialize(::zorba::serialization::Archiver& ar)
 {
   ar & theLocation;
-  ar & theOrderSpecs;
-  ar & theStable;
 
   ar & theInputForVars;
   ar & theInputLetVars;
   ar & theOutputForVarsRefs;
   ar & theOutputLetVarsRefs;
+
+  ar & theOrderSpecs;
+  ar & theStable;
 }
 
 

=== modified file 'test/unit/CMakeLists.txt'
--- test/unit/CMakeLists.txt	2012-02-16 14:11:02 +0000
+++ test/unit/CMakeLists.txt	2012-02-27 14:49:38 +0000
@@ -15,6 +15,7 @@
 
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/guestbook_main.xq ${CMAKE_CURRENT_BINARY_DIR}/guestbook_main.xq)
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/mini_http.xq ${CMAKE_CURRENT_BINARY_DIR}/mini_http.xq)
+CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/mini_audit.xq ${CMAKE_CURRENT_BINARY_DIR}/mini_audit.xq)
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/guestbook.xq ${CMAKE_CURRENT_BINARY_DIR}/guestbook.xq)
 
 #belongs to test cxx_api_changes.cpp

=== added file 'test/unit/mini_audit.xq'
--- test/unit/mini_audit.xq	1970-01-01 00:00:00 +0000
+++ test/unit/mini_audit.xq	2012-02-27 14:49:38 +0000
@@ -0,0 +1,13 @@
+import module namespace refl = "http://www.zorba-xquery.com/modules/reflection";;
+
+declare namespace ann = "http://www.zorba-xquery.com/annotations";;
+
+declare %ann:sequential function local:create() 
+{
+  let $seq := for $i in 1 to 50 return <a>{$i}</a>
+  return 
+    insert nodes $seq into <b/>;
+};
+
+
+refl:eval-s("local:create()")

=== modified file 'test/unit/plan_serializer.cpp'
--- test/unit/plan_serializer.cpp	2012-02-16 14:11:02 +0000
+++ test/unit/plan_serializer.cpp	2012-02-27 14:49:38 +0000
@@ -24,18 +24,24 @@
 #include <zorba/store_manager.h>
 #include <zorba/zorba_exception.h>
 
+#include "system/properties.h"
+
 using namespace zorba;
 
 bool
 save_and_load(Zorba* aZorba, const char* aQueryFile)
 {
-  try {
+  try 
+  {
      std::ifstream lIn(aQueryFile);
      std::ostringstream lOut;
 
      {
+       Zorba_CompilerHints_t lHints;
+       lHints.opt_level = ZORBA_OPT_LEVEL_O1;
+
        StaticContext_t lSctx = aZorba->createStaticContext();
-       XQuery_t lQuery = aZorba->compileQuery(lIn, lSctx);
+       XQuery_t lQuery = aZorba->compileQuery(lIn, lSctx, lHints);
        lQuery->saveExecutionPlan(lOut, ZORBA_USE_BINARY_ARCHIVE, SAVE_UNUSED_FUNCTIONS);
      }
 
@@ -46,7 +52,9 @@
        std::cout << lQuery << std::endl;
      }
      return true;
-  } catch (ZorbaException& e) {
+  }
+  catch (ZorbaException& e) 
+  {
     std::cerr << e << std::endl;
     return false;
   }
@@ -64,12 +72,22 @@
   return save_and_load(aZorba, "mini_http.xq");
 }
 
+
+bool
+plan_serializer3(Zorba* aZorba)
+{
+  return save_and_load(aZorba, "mini_audit.xq");
+}
+
+
 int
 plan_serializer(int argc, char* argv[]) 
 {
   void* lStore = zorba::StoreManager::getStore();
   Zorba* lZorba = Zorba::getInstance(lStore);
 
+  zorba::Properties::load(0, NULL);
+
   std::cout << "executing test 1" << std::endl;
   if (!plan_serializer1(lZorba))
     return 1;
@@ -78,6 +96,11 @@
   std::cout << "executing test 2" << std::endl;
   if (!plan_serializer2(lZorba))
     return 2;
+
+  std::cout << "executing test 3" << std::endl;
+  if (!plan_serializer3(lZorba))
+    return 3;
+
   std::cout << std::endl;
 
   lZorba->shutdown();