Open GoogleCodeExporter opened 8 years ago
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
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
Original issue reported on code.google.com by
buyingyi@gmail.com
on 12 Dec 2014 at 2:31