Add comma option in for clause

[GoogleCodeExporter]

Here is the query we want to support:

from $user in dataset MugshotUsers,
        $message in dataset MugshotMessages
where $message.author-id = $user.id
group by $name := $user.name with $message
with $avglen := avg(
                   from $m in $message
                   select string-length($m.message))
order by $avglen desc
limit 10
select {
  "uname" : $name,
  "msg-length" : $avglen
};

Original issue reported on code.google.com by buyingyi@gmail.com on 12 Dec 2014 at 2:31

[GoogleCodeExporter]

Hi Yingyi,

1. I made the changes to asterixdb/asterix-aql/src/main/javacc/AQL.jj
Change was one single line in AQL.jj file in ForClause function , see change in 
blue.

Clause ForClause() throws ParseException :
{
        ForClause fc = new ForClause();
        VariableExpr varExp;
        VariableExpr varPos = null;
        Expression inExp;
        extendCurrentScope();
}
{
    // (<FOR>|<FROM>) varExp = Variable() (<AT> varPos = Variable())?  <IN> ( inExp = Expression() )
    // CHANGE to support issue 839: Add comma option in for clause
    // This is a ONE line change
    (( (<FOR>|<FROM>) varExp = Variable() (<AT> varPos = Variable())?  <IN> ( inExp = Expression() )) | ((<COMMA> varExp = Variable()) (<AT> varPos = Variable())? <IN> ( inExp = Expression() )) )
    {
      fc.setVarExpr(varExp);
      getCurrentScope().addNewVarSymbolToScope(varExp.getVar());
      fc.setInExpr(inExp);
      if (varPos != null) {
        fc.setPosExpr(varPos);
            getCurrentScope().addNewVarSymbolToScope(varPos.getVar());
      }
      return fc;
    }
}

2. Changes compiled fine and I did a clean build, build was successful.

3. I also ran existing tests via, mvn clean package in asterixdb folder, and 
there were no test failures. So my change has not affected/regressed any 
existing functionality.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 13:49.565s
[INFO] Finished at: Sun Dec 14 20:46:43 PST 2014
[INFO] Final Memory: 134M/802M
[INFO] ------------------------------------------------------------------------

4. I did not touch/make any other changes in any other source file, just the 
one line in the AQL.jj file

Here is a simple unit test;

drop dataverse test;
create dataverse test if not exists;
use dataverse test;

create type  typ1 as closed {
"id" : int32
}

create dataset t1(typ1) primary key id;

create dataset t2(typ1) primary key id;

insert into dataset t1 ({"id" : 123});

insert into dataset t2 ({"id" : 224});

for $l in dataset t1, $m in dataset t2
return {"l" : $l, "m" : $m}

Results:
{ "l": { "id": 123 }, "m": { "id": 224 } }

It is just a one line change in asterixdb/asterix-aql/src/main/javacc/AQL.jj ; 
please see the diff below. 

khurram@khurram:~/uci/asterixdb/asterix-aql/src/main/javacc$ diff -cb AQL.jj 
AQL.jj.orig
*** AQL.jj  2014-12-14 20:32:41.072507182 -0800
--- AQL.jj.orig 2014-12-14 20:01:32.532493407 -0800
***************
*** 1957,1966 ****
    extendCurrentScope();
  }
  {
!     // (<FOR>|<FROM>) varExp = Variable() (<AT> varPos = Variable())?  <IN> ( 
inExp = Expression() )
!     // CHANGE to support issue 839: Add comma option in for clause
!     // This is a ONE line change
!     (( (<FOR>|<FROM>) varExp = Variable() (<AT> varPos = Variable())?  <IN> ( 
inExp = Expression() )) | ((<COMMA> varExp = Variable()) (<AT> varPos = 
Variable())? <IN> ( inExp = Expression() )) )    
      {
        fc.setVarExpr(varExp);
        getCurrentScope().addNewVarSymbolToScope(varExp.getVar());
--- 1957,1963 ----
    extendCurrentScope();
  }
  {
!     (<FOR>|<FROM>) varExp = Variable() (<AT> varPos = Variable())?  <IN> ( 
inExp = Expression() )
      {
        fc.setVarExpr(varExp);
        getCurrentScope().addNewVarSymbolToScope(varExp.getVar());

Original comment by khfaraaz82 on 18 Dec 2014 at 4:01

[GoogleCodeExporter]

Yingyi, thanks for your review and for trying my change to AQL.jj

Adding your observation here. It looks varExp, varPos, and inExpr are 
overwritten by the ones after comma.

I am debugging to know whats happening, I will share result soon from my 
investigation.

Original comment by khfaraaz82 on 18 Dec 2014 at 4:03