[ 
https://issues.apache.org/jira/browse/PIG-693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681960#action_12681960
 ] 

Thejas M Nair commented on PIG-693:
-----------------------------------

Re: Review comments on 693.patch 
 Re: Overall comment:  The private member variables are not part of the 
interface, so removing them should be fine.  I can add deprecated versions of 
the constructor and set methods. But is it OK to keep even deprecated versions 
of them around if we not using the input parameters passed , or in case of set 
methods they are no-op ? 

Re Complete package name used in Index: 
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/LogToPhyTranslationVisitor.java
 ,
There are two ExpressionOperator classes used in there, from - 
org.apache.pig.backend.hadoop.executionengine.physicalLayer.expressionOperators.ExpressionOperator;
 and org.apache.pig.impl.logicalLayer.ExpressionOperator. Either of those have 
to be used with full package name. The one logicalLayer one is only used once, 
so I am using full name for it.


Re: Index: 
src/org/apache/pig/impl/logicalLayer/validators/TypeCheckingVisitor.java , 
If i use node.getClass().getSimpleName(), it will print names like LOOr, 
LOBinCond which are also not very user friendly. I think we should change the 
name() function in these classes to not print the scope.

Re: Index: src/org/apache/pig/impl/plan/OperatorPlan.java 
1. Not sure if i understood what you meant there . The following code will not 
work - 
        Integer i = null;
        System.out.println(i.toString());

( I am reviewing rest of the suggestions.)

> Parameter to UDF which is an alias returned in another UDF in nested foreach 
> causes incorrect results
> -----------------------------------------------------------------------------------------------------
>
>                 Key: PIG-693
>                 URL: https://issues.apache.org/jira/browse/PIG-693
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 1.0.0
>            Reporter: Viraj Bhat
>            Assignee: Thejas M Nair
>         Attachments: 693.patch, 693.utest.patch, one.txt, REPLACEALL.java, 
> URLDECODE.java
>
>
> Consider the following Pig Script
> {code}
> register myudf.jar;
> A = load 'one.txt' using PigStorage() as ( one: int ); --use this dummy file 
> to start execution
> B = foreach A {
>         dec = myudf.URLDECODE('hello');
>         str1 = myudf.REPLACEALL(dec, '[\\u0000-\\u0020]', ' '); -- ERROR
>         str2 = myudf.REPLACEALL('hello', '[\\u0000-\\u0020]', ' ');
>         generate
>                 dec,
>                 str1,
>                 str2;
>         };
> describe B;
> dump B;
> {code}
> where one.txt is a file which contains number one (1) for starting execution 
> of the Pig script!!
> {code}
> describe B; 
> {code}
>  returns the following:
> B: {urldecode_9: chararray,replaceall_urldecode_10_11: 
> chararray,replaceall_12: chararray}
> {code}
> dump B;
> {code}
> returns 
> (hello,[\u0000-\u0020],hello)
> The result should be:
> There is a workaround for the same, 
> {code}
> register myudf.jar;
> A = load 'one.txt' using PigStorage() as ( one: int );
> B = foreach A {
>         dec = myudf.URLDECODE('hello');
>         generate
>                 dec as dec,
>                 myudf.REPLACEALL(dec, '[\\u0000-\\u0020]', ' ') as str1,
>                 myudf.REPLACEALL('hello', '[\\u0000-\\u0020]', ' ') as str2;
>         };
> describe B;
> dump B;
> {code}
> where 
> {code}
> dump B;
> {code}
> returns (hello,hello,hello)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to