You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by ch...@apache.org on 2013/06/04 18:03:19 UTC

svn commit: r1489491 - in /pig/trunk: ./ src/org/apache/pig/parser/ test/org/apache/pig/test/

Author: cheolsoo
Date: Tue Jun  4 16:03:12 2013
New Revision: 1489491

URL: http://svn.apache.org/r1489491
Log:
PIG-3342: Allow conditions in case statement (cheolsoo)

Modified:
    pig/trunk/CHANGES.txt
    pig/trunk/src/org/apache/pig/parser/AstPrinter.g
    pig/trunk/src/org/apache/pig/parser/AstValidator.g
    pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g
    pig/trunk/src/org/apache/pig/parser/QueryParser.g
    pig/trunk/test/org/apache/pig/test/TestCase.java

Modified: pig/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/trunk/CHANGES.txt?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/CHANGES.txt (original)
+++ pig/trunk/CHANGES.txt Tue Jun  4 16:03:12 2013
@@ -28,6 +28,8 @@ PIG-3174:  Remove rpm and deb artifacts 
 
 IMPROVEMENTS
 
+PIG-3342: Allow conditions in case statement (cheolsoo)
+
 PIG-3327: Pig hits OOM when fetching task reports (rohini)
 
 PIG-3336: Change IN operator to use or-expressions instead of EvalFunc (cheolsoo)

Modified: pig/trunk/src/org/apache/pig/parser/AstPrinter.g
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/AstPrinter.g?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/AstPrinter.g (original)
+++ pig/trunk/src/org/apache/pig/parser/AstPrinter.g Tue Jun  4 16:03:12 2013
@@ -331,7 +331,7 @@ var_expr
 ;
 
 projectable_expr
-    : func_eval | col_ref | bin_expr | case_expr
+    : func_eval | col_ref | bin_expr | case_expr | case_cond
 ;
 
 dot_proj
@@ -364,7 +364,13 @@ bin_expr
 ;
 
 case_expr
-    : ^( CASE { sb.append(" " + $CASE.text + "("); } expr ( { sb.append(", "); } expr )+ { sb.append(") "); } )
+    : ^( CASE_EXPR { sb.append(" CASE ("); } expr ( { sb.append(", "); } expr )+ { sb.append(") "); } )
+;
+
+case_cond
+    : ^( CASE_COND { sb.append(" CASE ("); }
+        ^( WHEN cond ( { sb.append(", "); } cond )* { sb.append(", "); } )
+        ^( THEN expr ( { sb.append(", "); } expr )* { sb.append(") "); } ) )
 ;
 
 limit_clause

Modified: pig/trunk/src/org/apache/pig/parser/AstValidator.g
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/AstValidator.g?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/AstValidator.g (original)
+++ pig/trunk/src/org/apache/pig/parser/AstValidator.g Tue Jun  4 16:03:12 2013
@@ -398,7 +398,7 @@ bag_type_cast : ^( BAG_TYPE_CAST tuple_t
 var_expr : projectable_expr ( dot_proj | pound_proj )*
 ;
 
-projectable_expr: func_eval | col_ref | bin_expr | case_expr
+projectable_expr: func_eval | col_ref | bin_expr | case_expr | case_cond
 ;
 
 dot_proj : ^( PERIOD col_alias_or_index+ )
@@ -423,7 +423,10 @@ pound_proj : ^( POUND ( QUOTEDSTRING | N
 bin_expr : ^( BIN_EXPR cond expr expr )
 ;
 
-case_expr: ^( CASE expr+ )
+case_expr: ^( CASE_EXPR expr+ )
+;
+
+case_cond: ^( CASE_COND ^( WHEN cond+ ) ^( THEN expr+ ) )
 ;
 
 limit_clause : ^( LIMIT rel ( INTEGER | LONGINTEGER | expr ) )

Modified: pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g (original)
+++ pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g Tue Jun  4 16:03:12 2013
@@ -1013,6 +1013,10 @@ projectable_expr[LogicalExpressionPlan p
    {
        $expr = $case_expr.expr;
    }
+ | case_cond[$plan]
+   {
+       $expr = $case_cond.expr;
+   }
 ;
 
 dot_proj returns[List<Object> cols]
@@ -1066,7 +1070,7 @@ case_expr[LogicalExpressionPlan plan] re
 @init {
     List<LogicalExpression> exprs = new ArrayList<LogicalExpression>();
 }
- : ^( CASE ( expr[$plan] { exprs.add($expr.expr); } )+ )
+ : ^( CASE_EXPR ( expr[$plan] { exprs.add($expr.expr); } )+ )
     {
         // Convert CASE tree to nested bincond expressions. Please also see
         // QueryParser.g for how CASE tree is constructed from case statement.
@@ -1078,17 +1082,9 @@ case_expr[LogicalExpressionPlan plan] re
         BinCondExpression prevBinCondExpr = null;
         BinCondExpression currBinCondExpr = null;
         for (int i = 0; i < numWhenBranches; i++) {
-            if (i == 0) {
-                currBinCondExpr = new BinCondExpression( $plan,
-                    new EqualExpression( $plan, exprs.get(3*i), exprs.get(3*i+1) ),
-                    exprs.get(3*i+2),
-                    elseExpr );
-            } else {
-                currBinCondExpr = new BinCondExpression( $plan,
-                    new EqualExpression( $plan, exprs.get(3*i), exprs.get(3*i+1) ),
-                    exprs.get(3*i+2),
-                    prevBinCondExpr );
-            }
+            currBinCondExpr = new BinCondExpression( $plan,
+                new EqualExpression( $plan, exprs.get(3*i), exprs.get(3*i+1) ), exprs.get(3*i+2),
+                prevBinCondExpr == null ? elseExpr : prevBinCondExpr);
             prevBinCondExpr = currBinCondExpr;
         }
         $expr = currBinCondExpr;
@@ -1096,6 +1092,33 @@ case_expr[LogicalExpressionPlan plan] re
     }
 ;
 
+case_cond[LogicalExpressionPlan plan] returns[LogicalExpression expr]
+@init {
+    List<LogicalExpression> conds = new ArrayList<LogicalExpression>();
+    List<LogicalExpression> exprs = new ArrayList<LogicalExpression>();
+}
+ : ^( CASE_COND ^( WHEN ( cond[$plan] { conds.add($cond.expr); } )+ )
+                ^( THEN ( expr[$plan] { exprs.add($expr.expr); } )+ ) )
+    {
+        // Convert CASE tree to nested bincond expressions. Please also see
+        // QueryParser.g for how CASE tree is constructed from case statement.
+        boolean hasElse = exprs.size() \% 2 == 1;
+        LogicalExpression elseExpr = hasElse ? exprs.get(exprs.size() - 1)
+                                             : new ConstantExpression($plan, null);
+        int numWhenBranches = conds.size();
+        BinCondExpression prevBinCondExpr = null;
+        BinCondExpression currBinCondExpr = null;
+        for (int i = 0; i < numWhenBranches; i++) {
+            currBinCondExpr = new BinCondExpression( $plan,
+                conds.get(i), exprs.get(i),
+                prevBinCondExpr == null ? elseExpr : prevBinCondExpr);
+            prevBinCondExpr = currBinCondExpr;
+        }
+        $expr = currBinCondExpr;
+        $expr.setLocation( new SourceLocation( (PigParserNode)$case_cond.start ) );
+    }
+;
+
 limit_clause returns[String alias]
 scope GScope;
 @init {

Modified: pig/trunk/src/org/apache/pig/parser/QueryParser.g
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/QueryParser.g?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/QueryParser.g (original)
+++ pig/trunk/src/org/apache/pig/parser/QueryParser.g Tue Jun  4 16:03:12 2013
@@ -41,6 +41,8 @@ tokens {
     FUNC_EVAL;
     INVOKE;
     INVOKER_FUNC_EVAL;
+    CASE_COND;
+    CASE_EXPR;
     CAST_EXPR;
     COL_RANGE;
     BIN_EXPR;
@@ -724,7 +726,7 @@ cast_expr
         // This is needed because in LogicalPlanGenerator.g, we translate this
         // tree to nested bincond expressions, and we need to construct a new
         // LogicalExpression object per when branch.
-        if(tree.getType() == CASE) {
+        if(tree.getType() == CASE_EXPR) {
             Tree caseExpr = tree.getChild(0);
             int childCount = tree.getChildCount();
             boolean hasElse = childCount \% 2 == 0;
@@ -744,7 +746,9 @@ cast_expr
           | identifier_plus projection*
           | identifier_plus func_name_suffix? LEFT_PAREN ( real_arg ( COMMA real_arg )* )? RIGHT_PAREN projection* -> ^( FUNC_EVAL identifier_plus func_name_suffix? real_arg* ) projection*
           | func_name_without_columns LEFT_PAREN ( real_arg ( COMMA real_arg )* )? RIGHT_PAREN projection* -> ^( FUNC_EVAL func_name_without_columns real_arg* ) projection*
-          | CASE expr WHEN expr THEN expr ( WHEN expr THEN expr )* ( ELSE expr )? END projection* -> ^( CASE expr+ ) projection*
+          | CASE ( (WHEN)=> WHEN cond THEN expr ( WHEN cond THEN expr )* ( ELSE expr )? END projection* -> ^( CASE_COND ^(WHEN cond+) ^(THEN expr+) ) projection*
+                 | expr WHEN expr THEN expr ( WHEN expr THEN expr )* ( ELSE expr )? END projection* -> ^( CASE_EXPR expr+ ) projection*
+                 )
           | paren_expr
           | curly_expr
           | bracket_expr

Modified: pig/trunk/test/org/apache/pig/test/TestCase.java
URL: http://svn.apache.org/viewvc/pig/trunk/test/org/apache/pig/test/TestCase.java?rev=1489491&r1=1489490&r2=1489491&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestCase.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestCase.java Tue Jun  4 16:03:12 2013
@@ -117,7 +117,91 @@ public class TestCase {
     }
 
     /**
-     * Verify that FrontendException is thrown when case expression is missing.
+     * Verify that conditional CASE statement without else branch works correctly.
+     * @throws Exception
+     */
+    @Test
+    public void testConditionalWithNoElse() throws Exception {
+        PigServer pigServer = new PigServer(ExecType.LOCAL);
+        Data data = resetData(pigServer);
+
+        data.set("foo",
+                tuple(1),
+                tuple(2),
+                tuple(3),
+                tuple(4),
+                tuple(5),
+                tuple(6),
+                tuple(7)
+                );
+
+        pigServer.registerQuery("A = LOAD 'foo' USING mock.Storage() AS (i:int);");
+        pigServer.registerQuery("B = FOREACH A GENERATE i, (" +
+                "  CASE " +
+                "    WHEN i % 5 == 0 THEN '5n'" + // Conditional expression in when branch
+                "    WHEN i % 5 == 1 THEN '5n+1'" +
+                "    WHEN i % 5 == 2 THEN '5n+2'" +
+                "    WHEN i % 5 == 3 THEN '5n+3'" +
+                "  END" +
+                ") AS s;");
+        pigServer.registerQuery("C = FILTER B BY s IS NOT NULL;");
+        pigServer.registerQuery("STORE C INTO 'bar' USING mock.Storage();");
+
+        List<Tuple> out = data.get("bar");
+        assertEquals(6, out.size());
+        assertEquals(tuple(1,"5n+1"), out.get(0));
+        assertEquals(tuple(2,"5n+2"), out.get(1));
+        assertEquals(tuple(3,"5n+3"), out.get(2));
+        assertEquals(tuple(5,"5n"),   out.get(3));
+        assertEquals(tuple(6,"5n+1"), out.get(4));
+        assertEquals(tuple(7,"5n+2"), out.get(5));
+    }
+
+    /**
+     * Verify that conditional CASE statement with else branch works correctly.
+     * @throws Exception
+     */
+    @Test
+    public void testConditionalWithElse() throws Exception {
+        PigServer pigServer = new PigServer(ExecType.LOCAL);
+        Data data = resetData(pigServer);
+
+        data.set("foo",
+                tuple(1),
+                tuple(2),
+                tuple(3),
+                tuple(4),
+                tuple(5),
+                tuple(6),
+                tuple(7)
+                );
+
+        pigServer.registerQuery("A = LOAD 'foo' USING mock.Storage() AS (i:int);");
+        pigServer.registerQuery("B = FOREACH A GENERATE i, (" +
+                "  CASE " +
+                "    WHEN i % 5 == 0 THEN '5n'" + // Conditional expression in when branch
+                "    WHEN i % 5 == 1 THEN '5n+1'" +
+                "    WHEN i % 5 == 2 THEN '5n+2'" +
+                "    WHEN i % 5 == 3 THEN '5n+3'" +
+                "    ELSE                 '5n+4'" +
+                "  END" +
+                ");");
+        pigServer.registerQuery("STORE B INTO 'bar' USING mock.Storage();");
+
+        List<Tuple> out = data.get("bar");
+        assertEquals(7, out.size());
+        assertEquals(tuple(1,"5n+1"), out.get(0));
+        assertEquals(tuple(2,"5n+2"), out.get(1));
+        assertEquals(tuple(3,"5n+3"), out.get(2));
+        assertEquals(tuple(4,"5n+4"), out.get(3));
+        assertEquals(tuple(5,"5n"),   out.get(4));
+        assertEquals(tuple(6,"5n+1"), out.get(5));
+        assertEquals(tuple(7,"5n+2"), out.get(6));
+    }
+
+    /**
+     * Verify that FrontendException is thrown when case expression is missing,
+     * and when branches do not contain conditional expressions.
      * @throws Exception
      */
     @Test(expected = FrontendException.class)
@@ -136,7 +220,7 @@ public class TestCase {
         pigServer.registerQuery("A = LOAD 'foo' USING mock.Storage() AS (i:int);");
         pigServer.registerQuery("B = FOREACH A GENERATE (" +
                 "  CASE " + // No case expression
-                "    WHEN 0 THEN '3n'" +
+                "    WHEN 0 THEN '3n'" + // When expression is not conditional
                 "    WHEN 1 THEN '3n+1'" +
                 "    ELSE        '3n+2'" +
                 "  END" +