Commit 7ca49f81 authored by gmantele's avatar gmantele
Browse files

[ADQL] Fix the tree generated by the parsing of NATURAL JOINs. 

The "normal" JOIN:
    A JOIN B ON A.id = B.id JOIN C ON B.id = C.id
is correctly interpreted as:
    ( (A JOIN B ON A.id = B.id) JOIN C ON B.id = C.id )
But with a NATURAL JOIN, the tree is mirrored:
    A NATURAL JOIN B NATURAL JOIN C
gives:
	( A NATURAL JOIN (B NATURAL JOIN C) )
instead of:
    ( (A NATURAL JOIN B) NATURAL JOIN C )
This is not a problem when the SQL translation is identical to the ADQL
expression, but for some DBMS a conversion into a INNER JOIN ON is necessary
and in this case we got the following SQL:
    A JOIN B JOIN C ON A.id = B.id ON B.id = C.id
Which seems to work, but is syntactically strange.

This commit should fix the generated tree. A "normal" JOIN and a NATURAL JOIN
should now have the same form. A JUnit test has been added into TestADQLParser
to check that: testJoinTree().
parent db9b63aa

src/adql/parser/ADQLParser.java

+233 −239

File changed.

Preview size limit exceeded, changes collapsed.

src/adql/parser/adqlGrammar.jj

+4 −4
Original line number Diff line number Diff line
@@ -26,7 +26,7 @@ 
*  If the syntax is not conform to the ADQL definition an error message is printed else it will be the message "Correct syntax".

*

*  Author:  Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de

*  Version: 1.4 (07/2016)

*  Version: 1.4 (09/2016)

*/



							/* ########### */
@@ -89,7 +89,7 @@ import adql.translator.TranslationException; 
* @see ADQLQueryFactory

*

* @author Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de

* @version 1.4 (07/2016)

* @version 1.4 (09/2016)

*/

public class ADQLParser {
@@ -998,14 +998,14 @@ FromContent JoinedTable(): { FromContent content; } { 
ADQLJoin JoinSpecification(FromContent leftTable): { boolean natural = false; JoinType type = JoinType.INNER;  ClauseConstraints condition = new ClauseConstraints("ON"); ArrayList<ADQLColumn> lstColumns=new ArrayList<ADQLColumn>(); IdentifierItem id; FromContent rightTable; ADQLJoin join; Token lastPar;} {

	try{

		(

			<NATURAL> {natural=true;} [<INNER> | ((<LEFT> {type = JoinType.OUTER_LEFT;}|<RIGHT> {type = JoinType.OUTER_RIGHT;}|<FULL> {type = JoinType.OUTER_FULL;}) [<OUTER>])] <JOIN> rightTable=TableRef()

			<NATURAL> {natural=true;} [<INNER> | ((<LEFT> {type = JoinType.OUTER_LEFT;}|<RIGHT> {type = JoinType.OUTER_RIGHT;}|<FULL> {type = JoinType.OUTER_FULL;}) [<OUTER>])] <JOIN> rightTable=SimpleTableRef()

			{

			  join = queryFactory.createJoin(type, leftTable, rightTable);

			  join.setPosition(new TextPosition(leftTable.getPosition(), rightTable.getPosition()));

			  return join;

			}

		|

			[<INNER> | ((<LEFT> {type = JoinType.OUTER_LEFT;}|<RIGHT> {type = JoinType.OUTER_RIGHT;}|<FULL> {type = JoinType.OUTER_FULL;}) [<OUTER>])] <JOIN> rightTable=TableRef()

			[<INNER> | ((<LEFT> {type = JoinType.OUTER_LEFT;}|<RIGHT> {type = JoinType.OUTER_RIGHT;}|<FULL> {type = JoinType.OUTER_FULL;}) [<OUTER>])] <JOIN> rightTable=SimpleTableRef()

			(

				<ON> ConditionsList(condition)

				{

test/adql/parser/TestADQLParser.java

+30 −0
Original line number Diff line number Diff line
@@ -2,6 +2,7 @@ package adql.parser; 


import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertTrue;

import static org.junit.Assert.fail;



import org.junit.After;
@@ -11,6 +12,8 @@ import org.junit.BeforeClass; 
import org.junit.Test;



import adql.query.ADQLQuery;

import adql.query.from.ADQLJoin;

import adql.query.from.ADQLTable;

import adql.query.operand.StringConstant;



public class TestADQLParser {
@@ -114,6 +117,33 @@ public class TestADQLParser { 
		}

	}



	@Test

	public void testJoinTree(){

		ADQLParser parser = new ADQLParser();

		try{

			String[] queries = new String[]{"SELECT * FROM aTable A JOIN aSecondTable B ON A.id = B.id JOIN aThirdTable C ON B.id = C.id;","SELECT * FROM aTable A NATURAL JOIN aSecondTable B NATURAL JOIN aThirdTable C;"};

			for(String q : queries){

				ADQLQuery query = parser.parseQuery(q);



				assertTrue(query.getFrom() instanceof ADQLJoin);



				ADQLJoin join = ((ADQLJoin)query.getFrom());

				assertTrue(join.getLeftTable() instanceof ADQLJoin);

				assertTrue(join.getRightTable() instanceof ADQLTable);

				assertEquals("aThirdTable", ((ADQLTable)join.getRightTable()).getTableName());



				join = (ADQLJoin)join.getLeftTable();

				assertTrue(join.getLeftTable() instanceof ADQLTable);

				assertEquals("aTable", ((ADQLTable)join.getLeftTable()).getTableName());

				assertTrue(join.getRightTable() instanceof ADQLTable);

				assertEquals("aSecondTable", ((ADQLTable)join.getRightTable()).getTableName());

			}

		}catch(Exception e){

			e.printStackTrace(System.err);

			fail("The ADQL query is strictly correct! No error should have occured. (see stdout for more details)");

		}

	}



	@Test

	public void test(){

		ADQLParser parser = new ADQLParser();