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/08/31 00:41:47 UTC

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

Author: cheolsoo
Date: Fri Aug 30 22:41:46 2013
New Revision: 1519104

URL: http://svn.apache.org/r1519104
Log:
PIG-3374: CASE and IN fail when expression includes dereferencing operator (cheolsoo)

Modified:
    pig/trunk/CHANGES.txt
    pig/trunk/src/org/apache/pig/parser/AliasMasker.g
    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
    pig/trunk/test/org/apache/pig/test/TestIn.java

Modified: pig/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/trunk/CHANGES.txt?rev=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/CHANGES.txt (original)
+++ pig/trunk/CHANGES.txt Fri Aug 30 22:41:46 2013
@@ -220,6 +220,8 @@ PIG-3013: BinInterSedes improve chararra
 
 BUG FIXES
 
+PIG-3374: CASE and IN fail when expression includes dereferencing operator (cheolsoo)
+
 PIG-2606: union/ join operations are not accepting same alias as multiple inputs (hsubramaniyan via daijy)
 
 PIG-3435: Custom Partitioner not working with MultiQueryOptimizer (knoguchi via daijy)

Modified: pig/trunk/src/org/apache/pig/parser/AliasMasker.g
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/AliasMasker.g?rev=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/AliasMasker.g (original)
+++ pig/trunk/src/org/apache/pig/parser/AliasMasker.g Fri Aug 30 22:41:46 2013
@@ -315,7 +315,7 @@ cond
 ;
 
 in_eval
-    : ^( IN expr expr+ )
+    : ^( IN ( ^( IN_LHS expr ) ^( IN_RHS expr ) )+ )
 ;
 
 func_eval
@@ -357,7 +357,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
@@ -389,7 +389,11 @@ bin_expr
 ;
 
 case_expr
-    : ^( CASE expr+ )
+    : ^( CASE_EXPR ( ^( CASE_EXPR_LHS expr ) ( ^( CASE_EXPR_RHS expr) )+ )+ )
+;
+
+case_cond
+    : ^( CASE_COND ^( WHEN cond+ ) ^( THEN expr+ ) )
 ;
 
 limit_clause

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=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/AstPrinter.g (original)
+++ pig/trunk/src/org/apache/pig/parser/AstPrinter.g Fri Aug 30 22:41:46 2013
@@ -318,7 +318,9 @@ cond
 ;
 
 in_eval
-    : ^( IN { sb.append(" " + $IN.text + "("); } expr ( { sb.append(", "); } expr )+ { sb.append(") "); } )
+    : ^( IN { sb.append(" " + $IN.text + "("); }
+         ( ^( IN_LHS expr ) ^( IN_RHS { sb.append(", "); } expr ) )
+         ( ^( IN_LHS { sb.append(", "); } expr ) ^( IN_RHS  { sb.append(", "); } expr ) )* { sb.append(") "); } )
 ;
 
 func_eval
@@ -395,7 +397,9 @@ bin_expr
 ;
 
 case_expr
-    : ^( CASE_EXPR { sb.append(" CASE ("); } expr ( { sb.append(", "); } expr )+ { sb.append(") "); } )
+    : ^( CASE_EXPR { sb.append(" CASE ("); }
+         ( ^( CASE_EXPR_LHS expr ) ( ^( CASE_EXPR_RHS { sb.append(", "); } expr ) )+ )
+         ( ^( CASE_EXPR_LHS { sb.append(", "); } expr ) ( ^( CASE_EXPR_RHS { sb.append(", "); } expr ) )+ )* { sb.append(")"); } )
 ;
 
 case_cond

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=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/AstValidator.g (original)
+++ pig/trunk/src/org/apache/pig/parser/AstValidator.g Fri Aug 30 22:41:46 2013
@@ -368,7 +368,7 @@ cond : ^( OR cond cond )
      | ^( BOOL_COND expr )
 ;
 
-in_eval: ^( IN expr expr+ )
+in_eval: ^( IN ( ^( IN_LHS expr ) ^( IN_RHS expr ) )+ )
 ;
 
 func_eval: ^( FUNC_EVAL func_name real_arg* ) | ^( INVOKER_FUNC_EVAL func_name IDENTIFIER real_arg* )
@@ -427,7 +427,7 @@ pound_proj : ^( POUND ( QUOTEDSTRING | N
 bin_expr : ^( BIN_EXPR cond expr expr )
 ;
 
-case_expr: ^( CASE_EXPR expr+ )
+case_expr: ^( CASE_EXPR ( ^( CASE_EXPR_LHS expr ) ( ^( CASE_EXPR_RHS expr) )+ )+ )
 ;
 
 case_cond: ^( CASE_COND ^( WHEN cond+ ) ^( THEN 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=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g (original)
+++ pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g Fri Aug 30 22:41:46 2013
@@ -793,20 +793,22 @@ cond[LogicalExpressionPlan exprPlan] ret
 
 in_eval[LogicalExpressionPlan plan] returns[LogicalExpression expr]
 @init {
-    List<LogicalExpression> exprs = new ArrayList<LogicalExpression>();
+    List<LogicalExpression> lhsExprs = new ArrayList<LogicalExpression>();
+    List<LogicalExpression> rhsExprs = new ArrayList<LogicalExpression>();
 }
- : ^( IN ( expr[$plan] { exprs.add($expr.expr); } )+ )
+ : ^( IN ( ^( IN_LHS lhs = expr[$plan] ) { lhsExprs.add($lhs.expr); }
+           ^( IN_RHS rhs = expr[$plan] ) { rhsExprs.add($rhs.expr); } )+ )
     {
         // Convert IN tree to nested or expressions. Please also see
         // QueryParser.g for how IN tree is constructed from IN expression.
-        EqualExpression firstBoolExpr = new EqualExpression(plan, exprs.get(0), exprs.get(1));
-        if (exprs.size() == 2) {
+        EqualExpression firstBoolExpr = new EqualExpression(plan, lhsExprs.get(0), rhsExprs.get(0));
+        if (lhsExprs.size() == 1) {
             $expr = firstBoolExpr;
         } else {
             OrExpression currOrExpr = null;
             OrExpression prevOrExpr = null;
-            for (int i = 2; i < exprs.size(); i = i + 2) {
-                EqualExpression boolExpr = new EqualExpression(plan, exprs.get(i), exprs.get(i+1));
+            for (int i = 1; i < lhsExprs.size(); i++) {
+                EqualExpression boolExpr = new EqualExpression(plan, lhsExprs.get(i), rhsExprs.get(i));
                 currOrExpr = new OrExpression( $plan, prevOrExpr == null ? firstBoolExpr : prevOrExpr, boolExpr );
                 prevOrExpr = currOrExpr;
             }
@@ -1078,22 +1080,24 @@ bin_expr[LogicalExpressionPlan plan] ret
 
 case_expr[LogicalExpressionPlan plan] returns[LogicalExpression expr]
 @init {
-    List<LogicalExpression> exprs = new ArrayList<LogicalExpression>();
+    List<LogicalExpression> lhsExprs = new ArrayList<LogicalExpression>();
+    List<LogicalExpression> rhsExprs = new ArrayList<LogicalExpression>();
 }
- : ^( CASE_EXPR ( expr[$plan] { exprs.add($expr.expr); } )+ )
+ : ^( CASE_EXPR ( ( ^( CASE_EXPR_LHS lhs = expr[$plan] { lhsExprs.add($lhs.expr); } ) )
+                  ( ^( CASE_EXPR_RHS rhs = expr[$plan] { rhsExprs.add($rhs.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() \% 3 == 1;
-        LogicalExpression elseExpr = hasElse ? exprs.get(exprs.size()-1)
+        boolean hasElse = rhsExprs.size() \% 2 == 1;
+        LogicalExpression elseExpr = hasElse ? rhsExprs.get(rhsExprs.size()-1)
                                              : new ConstantExpression($plan, null);
 
-        int numWhenBranches = exprs.size() / 3;
+        int numWhenBranches = rhsExprs.size() / 2;
         BinCondExpression prevBinCondExpr = null;
         BinCondExpression currBinCondExpr = null;
         for (int i = 0; i < numWhenBranches; i++) {
             currBinCondExpr = new BinCondExpression( $plan,
-                new EqualExpression( $plan, exprs.get(3*i), exprs.get(3*i+1) ), exprs.get(3*i+2),
+                new EqualExpression( $plan, lhsExprs.get(i), rhsExprs.get(2*i) ), rhsExprs.get(2*i+1),
                 prevBinCondExpr == null ? elseExpr : prevBinCondExpr);
             prevBinCondExpr = currBinCondExpr;
         }

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=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/QueryParser.g (original)
+++ pig/trunk/src/org/apache/pig/parser/QueryParser.g Fri Aug 30 22:41:46 2013
@@ -41,8 +41,12 @@ tokens {
     FUNC_EVAL;
     INVOKE;
     INVOKER_FUNC_EVAL;
+    IN_LHS;
+    IN_RHS;
     CASE_COND;
     CASE_EXPR;
+    CASE_EXPR_LHS;
+    CASE_EXPR_RHS;
     CAST_EXPR;
     COL_RANGE;
     BIN_EXPR;
@@ -663,11 +667,14 @@ unary_cond
     }
     : exp1 = expr
         ( ( IS NOT? NULL -> ^( NULL $exp1 NOT? ) )
-        | ( IN LEFT_PAREN ( expr ( COMMA expr )* ) RIGHT_PAREN -> ^( IN expr+ ) )
+        | ( IN LEFT_PAREN ( rhs_operand ( COMMA rhs_operand )* ) RIGHT_PAREN -> ^( IN ^( IN_LHS expr ) ^( IN_RHS rhs_operand )+ ) )
         | ( rel_op exp2 = expr -> ^( rel_op $exp1 $exp2 ) )
         | ( -> ^(BOOL_COND expr) ) )
 ;
 
+rhs_operand : expr
+;
+
 expr : multi_expr ( ( PLUS | MINUS )^ multi_expr )*
 ;
 
@@ -754,7 +761,8 @@ cast_expr
           | 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 ( (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*
+                 | expr WHEN rhs_operand THEN rhs_operand ( WHEN rhs_operand THEN rhs_operand )* ( ELSE rhs_operand )? END projection*
+                 -> ^( CASE_EXPR ^(CASE_EXPR_LHS expr) ^(CASE_EXPR_RHS rhs_operand)+ ) projection*
                  )
           | paren_expr
           | curly_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=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestCase.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestCase.java Fri Aug 30 22:41:46 2013
@@ -18,6 +18,7 @@
 package org.apache.pig.test;
 
 import static junit.framework.Assert.assertEquals;
+import static org.apache.pig.builtin.mock.Storage.bag;
 import static org.apache.pig.builtin.mock.Storage.resetData;
 import static org.apache.pig.builtin.mock.Storage.tuple;
 import static org.junit.Assert.fail;
@@ -238,6 +239,42 @@ public class TestCase {
     }
 
     /**
+     * Verify that CASE statement works when expressions contain dereference operators.
+     * @throws Exception
+     */
+    @Test
+    public void testWithDereferenceOperator() throws Exception {
+        PigServer pigServer = new PigServer(ExecType.LOCAL);
+        Data data = resetData(pigServer);
+
+        data.set("foo",
+                tuple("a","x",1),
+                tuple("a","y",1),
+                tuple("b","x",2),
+                tuple("b","y",2),
+                tuple("c","x",3),
+                tuple("c","y",3)
+                );
+
+        pigServer.registerQuery("A = LOAD 'foo' USING mock.Storage() AS (c1:chararray, c2:chararray, i:int);");
+        pigServer.registerQuery("B = GROUP A BY (c1, i);");
+        pigServer.registerQuery("C = FOREACH B GENERATE group.i, (" +
+                "  CASE group.i % 3" +
+                "    WHEN 0 THEN '3n'" +
+                "    WHEN 1 THEN '3n+1'" +
+                "    ELSE        '3n+2'" +
+                "  END" +
+                "), A.(c1, c2);");
+        pigServer.registerQuery("STORE C INTO 'bar' USING mock.Storage();");
+
+        List<Tuple> out = data.get("bar");
+        assertEquals(3, out.size());
+        assertEquals(tuple(1, "3n+1", bag(tuple("a","x"), tuple("a","y"))), out.get(0));
+        assertEquals(tuple(2, "3n+2", bag(tuple("b","x"), tuple("b","y"))), out.get(1));
+        assertEquals(tuple(3, "3n",   bag(tuple("c","x"), tuple("c","y"))), out.get(2));
+    }
+
+    /**
      * Verify that FrontendException is thrown when case expression is missing,
      * and when branches do not contain conditional expressions.
      * @throws Exception

Modified: pig/trunk/test/org/apache/pig/test/TestIn.java
URL: http://svn.apache.org/viewvc/pig/trunk/test/org/apache/pig/test/TestIn.java?rev=1519104&r1=1519103&r2=1519104&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestIn.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestIn.java Fri Aug 30 22:41:46 2013
@@ -18,6 +18,7 @@
 package org.apache.pig.test;
 
 import static junit.framework.Assert.assertEquals;
+import static org.apache.pig.builtin.mock.Storage.bag;
 import static org.apache.pig.builtin.mock.Storage.resetData;
 import static org.apache.pig.builtin.mock.Storage.tuple;
 import static org.junit.Assert.fail;
@@ -126,6 +127,35 @@ public class TestIn {
     }
 
     /**
+     * Verify that IN operator works when expressions contain dereference operators.
+     * @throws Exception
+     */
+    @Test
+    public void testWithDereferenceOperator() throws Exception {
+        PigServer pigServer = new PigServer(ExecType.LOCAL);
+        Data data = resetData(pigServer);
+
+        data.set("foo",
+                tuple("a","x",1),
+                tuple("a","y",2),
+                tuple("b","x",3),
+                tuple("b","y",4),
+                tuple("c","x",5),
+                tuple("c","y",6)
+                );
+
+        pigServer.registerQuery("A = LOAD 'foo' USING mock.Storage() AS (k1:chararray, k2:chararray, i:int);");
+        pigServer.registerQuery("B = GROUP A BY (k1, k2);");
+        pigServer.registerQuery("C = FILTER B BY group.k1 IN ('a', 'b') AND group.k2 IN ('x');");
+        pigServer.registerQuery("STORE C INTO 'bar' USING mock.Storage();");
+
+        List<Tuple> out = data.get("bar");
+        assertEquals(2, out.size());
+        assertEquals(tuple(tuple("a","x"),bag(tuple("a","x",1))), out.get(0));
+        assertEquals(tuple(tuple("b","x"),bag(tuple("b","x",3))), out.get(1));
+    }
+
+    /**
      * Verify that IN operator throws FrontendException when no operand is given.
      * @throws Exception
      */