← Back to team overview

zorba-coders team mailing list archive

[Merge] lp:~zorba-coders/zorba/markos-scratch into lp:zorba

 

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

Commit message:
1. Potential fix for bug #1185989
2. Some optimization for the hoisting rule.


Requested reviews:
  Markos Zaharioudakis (markos-za)

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

1. Potential fix for bug #1185989
2. Some optimization for the hoisting rule.

-- 
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/166616
Your team Zorba Coders is subscribed to branch lp:zorba.
=== modified file 'src/compiler/expression/expr.h'
--- src/compiler/expression/expr.h	2013-03-08 00:09:46 +0000
+++ src/compiler/expression/expr.h	2013-05-31 00:27:28 +0000
@@ -938,7 +938,7 @@
 public:
   void add(pragma* p) { thePragmas.push_back(p); }
 
-  expr* get_expr() const { return theExpr; }
+  expr* get_input() const { return theExpr; }
 
   void compute_scripting_kind();
 

=== modified file 'src/compiler/expression/expr_base.cpp'
--- src/compiler/expression/expr_base.cpp	2013-05-24 16:34:11 +0000
+++ src/compiler/expression/expr_base.cpp	2013-05-31 00:27:28 +0000
@@ -17,6 +17,7 @@
 
 #include "compiler/expression/expr_base.h"
 #include "compiler/expression/expr.h"
+#include "compiler/expression/json_exprs.h"
 #include "compiler/expression/fo_expr.h"
 #include "compiler/expression/flwor_expr.h"
 #include "compiler/expression/path_expr.h"
@@ -1090,11 +1091,9 @@
   case namespace_expr_kind:
   case text_expr_kind:
   case pi_expr_kind:
-#ifdef ZORBA_WITH_JSON
   case json_object_expr_kind:
   case json_direct_object_expr_kind:
   case json_array_expr_kind:
-#endif
   {
     return !contains_expr(e);
   }
@@ -1184,6 +1183,171 @@
 
 
 /*******************************************************************************
+  If this expr has a single child, return it; otherwise return NULL
+********************************************************************************/
+expr* expr::get_single_child() const
+{
+  switch (get_expr_kind())
+  {
+  case block_expr_kind:
+  {
+    const block_expr* e = static_cast<const block_expr*>(this);
+
+    if (e->size() == 1)
+      return (*e)[0];
+
+    return NULL;
+  }
+  case var_decl_expr_kind:
+  case var_set_expr_kind:
+  {
+    const var_set_expr* e = static_cast<const var_set_expr*>(this);
+    return e->get_expr();
+  }
+  case relpath_expr_kind:
+  {
+    const relpath_expr* e = static_cast<const relpath_expr*>(this);
+    return (*e)[0];
+  }
+  case fo_expr_kind:
+  {
+    const fo_expr* fo = static_cast<const fo_expr*>(this);
+
+    csize numArgs = fo->num_args();
+
+    if (numArgs == 1)
+      return fo->get_arg(0);
+
+    expr* arg = NULL;
+
+    for (csize i = 0; i < numArgs; ++i)
+    {
+      if (fo->get_arg(i)->get_expr_kind() == const_expr_kind)
+        continue;
+
+      if (arg == NULL)
+      {
+        arg = fo->get_arg(i);
+      }
+      else
+      {
+        arg = NULL;
+        break;
+      }
+    }
+
+    return arg;
+  }
+  case doc_expr_kind:
+  {
+    const doc_expr* e = static_cast<const doc_expr*>(this);
+    return e->getContent();
+  }
+  case text_expr_kind:
+  {
+    const text_expr* e = static_cast<const text_expr*>(this);
+    return e->get_text();
+  }
+  case castable_expr_kind:
+  case cast_expr_kind:
+  case instanceof_expr_kind:
+  case treat_expr_kind:
+  case promote_expr_kind:
+  {
+    const cast_or_castable_base_expr* e =
+    static_cast<const cast_or_castable_base_expr*>(this);
+
+    return e->get_input();
+  }
+  case name_cast_expr_kind:
+  {
+    const name_cast_expr* e = static_cast<const name_cast_expr*>(this);
+    return e->get_input();
+  }
+  case validate_expr_kind:
+  {
+    const validate_expr* e = static_cast<const validate_expr*>(this);
+    return e->get_input();
+  }
+  case extension_expr_kind:
+  {
+    const extension_expr* e = static_cast<const extension_expr*>(this);
+    return e->get_input();
+  }
+  case order_expr_kind:
+  {
+    const order_expr* e = static_cast<const order_expr*>(this);
+    return e->get_input();
+  }
+  case wrapper_expr_kind:
+  {
+    const wrapper_expr* e = static_cast<const wrapper_expr*>(this);
+    return e->get_input();
+  }
+  case function_trace_expr_kind:
+  {
+    const function_trace_expr* e = static_cast<const function_trace_expr*>(this);
+    return e->get_input();
+  }
+  case apply_expr_kind:
+  {
+    const apply_expr* e = static_cast<const apply_expr*>(this);
+    return e->get_expr();
+  }
+  case while_expr_kind:
+  {
+    const while_expr* e = static_cast<const while_expr*>(this);
+    return e->get_body();
+  }
+  case json_object_expr_kind:
+  {
+    const json_object_expr* e = static_cast<const json_object_expr*>(this);
+    return e->get_expr();
+  }
+  case json_array_expr_kind:
+  {
+    const json_array_expr* e = static_cast<const json_array_expr*>(this);
+    return e->get_expr();
+  }
+  
+  case eval_expr_kind:
+  case debugger_expr_kind:
+  case const_expr_kind:
+  case var_expr_kind:
+  case flwor_expr_kind:
+  case if_expr_kind:
+  case trycatch_expr_kind:
+  case elem_expr_kind:
+  case attr_expr_kind:
+  case namespace_expr_kind:
+  case pi_expr_kind:
+  case dynamic_function_invocation_expr_kind:
+  case argument_placeholder_expr_kind:
+  case function_item_expr_kind:
+  case delete_expr_kind:
+  case insert_expr_kind:
+  case rename_expr_kind:
+  case replace_expr_kind:
+  case transform_expr_kind:
+  case exit_expr_kind:
+  case exit_catcher_expr_kind:
+  case flowctl_expr_kind:
+  case json_direct_object_expr_kind:
+  case axis_step_expr_kind:
+  case match_expr_kind:
+  case ft_expr_kind:
+  {
+    return NULL;
+  }
+  default:
+  {
+    ZORBA_ASSERT(false);
+  }
+  }
+}
+
+
+/*******************************************************************************
   If "this" is a const expr that returns a qname, evaluate and return this
   qname. This method is used to extract the qname from the expression that is
   given as an arg to collection and index related functions.

=== modified file 'src/compiler/expression/expr_base.h'
--- src/compiler/expression/expr_base.h	2013-05-24 16:34:11 +0000
+++ src/compiler/expression/expr_base.h	2013-05-31 00:27:28 +0000
@@ -108,11 +108,9 @@
   wrapper_expr_kind,
   function_trace_expr_kind,
 
-#ifdef ZORBA_WITH_JSON
   json_direct_object_expr_kind,
   json_object_expr_kind,
   json_array_expr_kind,
-#endif
 
   unknown_expr_kind
 };
@@ -401,6 +399,8 @@
 
   expr* skip_wrappers() const;
 
+  expr* get_single_child() const;
+
   void clear_annotations();
 
   xqtref_t get_return_type_with_empty_input(const expr* input) const;

=== modified file 'src/compiler/expression/script_exprs.h'
--- src/compiler/expression/script_exprs.h	2013-04-27 03:30:17 +0000
+++ src/compiler/expression/script_exprs.h	2013-05-31 00:27:28 +0000
@@ -127,9 +127,7 @@
 
   csize size() const { return theArgs.size(); }
 
-  const expr* operator[](csize i) const { return theArgs[i]; }
-
-  expr* operator[](csize i) { return theArgs[i]; }
+  expr* operator[](csize i) const { return theArgs[i]; }
 
   bool get_var_pos(const var_expr* v, csize& pos) const;
 

=== modified file 'src/compiler/rewriter/rules/hoist_rules.cpp'
--- src/compiler/rewriter/rules/hoist_rules.cpp	2013-05-24 16:34:11 +0000
+++ src/compiler/rewriter/rules/hoist_rules.cpp	2013-05-31 00:27:28 +0000
@@ -65,6 +65,32 @@
 
 
 /*******************************************************************************
+
+********************************************************************************/
+expr* skip_children(expr* e)
+{
+  FunctionConsts::FunctionKind fkind = e->get_function_kind();
+
+  if (fkind == FunctionConsts::OP_ENCLOSED_1 ||
+      fkind == FunctionConsts::OP_HOIST_1)
+  {
+    return e;
+  } 
+  else
+  {
+    expr* ce = e->get_single_child();
+    while (ce != NULL)
+    {
+      e = ce;
+      ce = e->get_single_child();
+    }
+
+    return e;
+  }
+}
+
+
+/*******************************************************************************
   This rule looks for exprs that are inside a for loop but do not depend on the
   loop variable, and then moves such exprs outside the loop.
 ********************************************************************************/
@@ -169,10 +195,15 @@
             assert(numClauses == flwor->num_clauses());
           }
         }
-        else if (hoistChildren(rCtx, domainExpr, &step))
+        else
         {
-          status = true;
-          numClauses = flwor->num_clauses();
+          domainExpr = skip_children(domainExpr);
+
+          if (hoistChildren(rCtx, domainExpr, &step))
+          {
+            status = true;
+            numClauses = flwor->num_clauses();
+          }
         }
 
         if (c->get_kind() == flwor_clause::window_clause)
@@ -197,10 +228,15 @@
               status = true;
               numClauses = flwor->num_clauses();
             }
-            else if (hoistChildren(rCtx, condExpr, &step))
+            else
             {
-              status = true;
-              numClauses = flwor->num_clauses();
+              condExpr = skip_children(condExpr);
+
+              if (hoistChildren(rCtx, condExpr, &step))
+              {
+                status = true;
+                numClauses = flwor->num_clauses();
+              }
             }
 
             --step.clauseCount;
@@ -222,10 +258,15 @@
               status = true;
               numClauses = flwor->num_clauses();
             }
-            else if (hoistChildren(rCtx, condExpr, &step))
+            else
             {
-              status = true;
-              numClauses = flwor->num_clauses();
+              condExpr = skip_children(condExpr);
+
+              if (hoistChildren(rCtx, condExpr, &step))
+              {
+                status = true;
+                numClauses = flwor->num_clauses();
+              }
             }
 
             --step.clauseCount;
@@ -249,10 +290,15 @@
           status = true;
           numClauses = flwor->num_clauses();
         }
-        else if (hoistChildren(rCtx, we, &step))
+        else
         {
-          status = true;
-          numClauses = flwor->num_clauses();
+          we = skip_children(we);
+
+          if (hoistChildren(rCtx, we, &step))
+          {
+            status = true;
+            numClauses = flwor->num_clauses();
+          }
         }
 
         break;
@@ -277,10 +323,15 @@
             status = true;
             numClauses = flwor->num_clauses();
           }
-          else if (hoistChildren(rCtx, oe, &step))
+          else
           {
-            status = true;
-            numClauses = flwor->num_clauses();
+            oe = skip_children(oe);
+
+            if (hoistChildren(rCtx, oe, &step))
+            {
+              status = true;
+              numClauses = flwor->num_clauses();
+            }
           }
         }
 
@@ -380,6 +431,8 @@
     }
     else
     {
+      re = skip_children(re);
+
       status = hoistChildren(rCtx, re, &step) || status;
     }
   }
@@ -417,16 +470,40 @@
     // do nothing
   }
 
+  else if (e->get_expr_kind() == trycatch_expr_kind)
+  {
+    PathHolder step;
+    step.prev = path;
+    step.theExpr = e;
+    path = &step;
+
+    ExprIterator iter(e);
+
+    while (!iter.done())
+    {
+      expr* ce = **iter;
+      if (ce)
+      {
+        expr* unhoistExpr = hoistExpr(rCtx, ce, path);
+        if (unhoistExpr != NULL)
+        {
+          **iter = unhoistExpr;
+          status = true;
+        }
+        else
+        {
+          ce = skip_children(ce);
+
+          status = hoistChildren(rCtx, ce, path) || status;
+        }
+      }
+
+      iter.next();
+    }
+  }
+
   else
   {
-    if (e->get_expr_kind() == trycatch_expr_kind)
-    {
-      PathHolder step;
-      step.prev = path;
-      step.theExpr = e;
-      path = &step;
-    }
-
     ExprIterator iter(e);
 
     while (!iter.done())
@@ -442,6 +519,8 @@
         }
         else
         {
+          ce = skip_children(ce);
+
           status = hoistChildren(rCtx, ce, path) || status;
         }
       }

=== modified file 'src/compiler/rewriter/rules/index_matching_rule.cpp'
--- src/compiler/rewriter/rules/index_matching_rule.cpp	2013-05-24 16:34:11 +0000
+++ src/compiler/rewriter/rules/index_matching_rule.cpp	2013-05-31 00:27:28 +0000
@@ -96,7 +96,8 @@
     let_clause* lc = static_cast<let_clause*>(theViewExpr->get_clause(i));
     theKeyClauses.push_back(lc);
   }
-
+  
+  // We apply the fold rules on the view expr in order to flatten a
   std::ostringstream msg;
   msg << "normalization of candidate index: " << decl->getName()->getStringValue();
 
@@ -107,12 +108,12 @@
                        true);
   FoldRules foldRules;
   foldRules.rewrite(rCtx);
-
+  
   if (Properties::instance()->printIntermediateOpt() && theDoTrace)
   {
     std::cout << "Canonical view expr for candidate index : " 
               << decl->getName()->getStringValue() << std::endl;
-    rCtx.getRoot()->put(std::cout) << std::endl;
+    theViewExpr->put(std::cout) << std::endl;
   }
 }
 

=== modified file 'src/compiler/rewriter/tools/dataflow_annotations.cpp'
--- src/compiler/rewriter/tools/dataflow_annotations.cpp	2013-05-24 16:34:11 +0000
+++ src/compiler/rewriter/tools/dataflow_annotations.cpp	2013-05-31 00:27:28 +0000
@@ -544,8 +544,8 @@
   default_walk(e);
   if (!generic_compute(e))
   {
-    PROPOGATE_SORTED_NODES(e->get_expr(), e);
-    PROPOGATE_DISTINCT_NODES(e->get_expr(), e);
+    PROPOGATE_SORTED_NODES(e->get_input(), e);
+    PROPOGATE_DISTINCT_NODES(e->get_input(), e);
   }
 }
 

=== modified file 'src/compiler/translator/translator.cpp'
--- src/compiler/translator/translator.cpp	2013-05-25 15:15:30 +0000
+++ src/compiler/translator/translator.cpp	2013-05-31 00:27:28 +0000
@@ -5189,19 +5189,11 @@
     ERROR_PARAMS(index->getName()->getStringValue()));
   }
 
-#ifdef ZORBA_WITH_JSON
   domainExpr = wrap_in_type_match(domainExpr,
                                   theRTM.STRUCTURED_ITEM_TYPE_STAR,
                                   loc,
                                   TREAT_INDEX_DOMAIN,
                                   index->getName());
-#else
-  domainExpr = wrap_in_type_match(domainExpr,
-                                  theRTM.ANY_NODE_TYPE_STAR,
-                                  loc,
-                                  TREAT_INDEX_DOMAIN,
-                                  index->getName());
-#endif
 
   // For general indexes, the domain expression must not return duplicate nodes.
   // To see why, consider the following examples:

=== modified file 'src/compiler/xqddf/value_index.cpp'
--- src/compiler/xqddf/value_index.cpp	2013-05-24 16:34:11 +0000
+++ src/compiler/xqddf/value_index.cpp	2013-05-31 00:27:28 +0000
@@ -475,7 +475,7 @@
   For now, this is done for value indexes only
 
   for $newdot at $newpos in cloned_domain_expr
-  return value-index-entry-builder($$newdot, cloned_key1_expr, ..., cloned_keyN_expr)
+  return $newdot
 *******************************************************************************/
 flwor_expr* IndexDecl::getViewExpr()
 {

=== modified file 'src/compiler/xqddf/value_index.h'
--- src/compiler/xqddf/value_index.h	2013-02-07 17:24:36 +0000
+++ src/compiler/xqddf/value_index.h	2013-05-31 00:27:28 +0000
@@ -282,6 +282,9 @@
 
   std::vector<expr*>              theDomainSourceExprs;
 
+  flwor_expr                    * theViewExpr;
+  std::vector<let_clause*>        theKeyClauses;
+
   expr                          * theBuildExpr;
   PlanIter_t                      theBuildPlan;
 

=== modified file 'test/rbkt/Queries/zorba/xmark/q4.xq'
--- test/rbkt/Queries/zorba/xmark/q4.xq	2013-02-07 17:24:36 +0000
+++ test/rbkt/Queries/zorba/xmark/q4.xq	2013-05-31 00:27:28 +0000
@@ -1,9 +1,10 @@
 declare variable $input-context external;
+
 let $auction := doc($input-context) return
 for $b in $auction/site/open_auctions/open_auction
 where
-some $pr1 in $b/bidder/personref[@person = "person20"],
-$pr2 in $b/bidder/personref[@person = "person51"]
-satisfies $pr1 << $pr2
+  some $pr1 in $b/bidder/personref[@person = "person20"],
+  $pr2 in $b/bidder/personref[@person = "person51"]
+  satisfies $pr1 << $pr2
 return <history>{$b/reserve/text()}</history>
 


Follow ups