[ADQL] Re-Fix GROUP BY's columns handling: (7a70c603) · Commits · Sonia Zorba / vollt

src/adql/parser/ADQLParser.java

+5 −19

Original line number	Diff line number	Diff line
		/* ADQLParser.java */
		/* Generated By:JavaCC: Do not edit this line. ADQLParser.java
		*
		* Modified by Gregory Mantelet (ARI), March 2017
		* Modification: line 5686, from:
		*
		* public ADQLParser(java.io.InputStream stream){
		* this(stream, null);
		* }
		*
		* to:
		*
		* public ADQLParser(java.io.InputStream stream){
		* this(stream, (String)null);
		* }
		*/
		/* Generated By:JavaCC: Do not edit this line. ADQLParser.java */
		package adql.parser;

		import java.io.FileReader;
		@@ -82,7 +68,7 @@ import adql.translator.TranslationException;
		* @see ADQLQueryFactory
		*
		* @author Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de
		* @version 1.4 (03/2017)
		* @version 1.4 (04/2017)
		*/
		public class ADQLParser implements ADQLParserConstants {

		@@ -1110,12 +1096,12 @@ public class ADQLParser implements ADQLParserConstants {
		final public ColumnReference ColumnRef() throws ParseException{
		trace_call("ColumnRef");
		try{
		IdentifierItem identifier = null;
		identifier = Identifier();
		IdentifierItems identifiers = null;
		identifiers = ColumnName();
		try{
		{
		if ("" != null)
		return queryFactory.createColRef(identifier);
		return queryFactory.createColRef(identifiers);
		}
		}catch(Exception ex){
		{

src/adql/parser/ADQLQueryFactory.java

+2 −6

Original line number	Diff line number	Diff line
		@@ -16,7 +16,7 @@ package adql.parser;
		* You should have received a copy of the GNU Lesser General Public License
		* along with ADQLLibrary. If not, see <http://www.gnu.org/licenses/>.
		*
		* Copyright 2012-2015 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
		* Copyright 2012-2017 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
		* Astronomisches Rechen Institut (ARI)
		*/

		@@ -83,7 +83,7 @@ import adql.query.operand.function.geometry.RegionFunction;
		* <p>To customize the object representation you merely have to extends the appropriate functions of this class.</p>
		*
		* @author Grégory Mantelet (CDS;ARI)
		* @version 1.4 (08/2015)
		* @version 1.4 (04/2017)
		*
		* @see ADQLParser
		*/
		@@ -430,10 +430,6 @@ public class ADQLQueryFactory {
		return colRef;
		}

		/**
		* @deprecated since 1.4 ; Former version's mistake: a GROUP BY item is either a regular/delimited column name or an integer, not a qualified column name ; Replaced by {@link #createColRef(adql.parser.IdentifierItems.IdentifierItem)} ; This function is no longer used by ADQLParser.
		*/
		@Deprecated
		public ColumnReference createColRef(final IdentifierItems idItems) throws Exception{
		ColumnReference colRef = new ColumnReference(idItems.join("."));
		if (colRef != null){

src/adql/parser/adqlGrammar.jj

+5 −5

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 (03/2017)
		* Version: 1.4 (04/2017)
		*/

		/* ########### */
		@@ -89,7 +89,7 @@ import adql.translator.TranslationException;
		* @see ADQLQueryFactory
		*
		* @author Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de
		* @version 1.4 (03/2017)
		* @version 1.4 (04/2017)
		*/
		public class ADQLParser {

		@@ -916,11 +916,11 @@ ADQLColumn Column(): {IdentifierItems identifiers;} {
		}
		}

		ColumnReference ColumnRef(): {IdentifierItem identifier = null;}{
		identifier=Identifier()
		ColumnReference ColumnRef(): {IdentifierItems identifiers = null;}{
		identifiers=ColumnName()
		{
		try{
		return queryFactory.createColRef(identifier);
		return queryFactory.createColRef(identifiers);
		}catch(Exception ex){
		throw generateParseException(ex);
		}

test/adql/parser/TestADQLParser.java

+1 −9

Original line number	Diff line number	Diff line
		@@ -43,6 +43,7 @@ public class TestADQLParser {
		parser.parseQuery("SELECT * FROM cat ORDER BY 1 DESC;");
		// GROUP BY
		parser.parseQuery("SELECT * FROM cat GROUP BY oid;");
		parser.parseQuery("SELECT * FROM cat GROUP BY cat.oid;");
		// JOIN ... USING(...)
		parser.parseQuery("SELECT * FROM cat JOIN cat2 USING(oid);");
		}catch(Exception e){
		@@ -68,15 +69,6 @@ public class TestADQLParser {
		assertEquals(" Encountered \".\". Was expecting one of: <EOF> \",\" \";\" \"ASC\" \"DESC\" ", e.getMessage());
		}

		try{
		// GROUP BY with a qualified column name
		parser.parseQuery("SELECT * FROM cat GROUP BY cat.oid;");
		fail("A qualified column name is forbidden in GROUP BY! This test should have failed.");
		}catch(Exception e){
		assertEquals(ParseException.class, e.getClass());
		assertEquals(" Encountered \".\". Was expecting one of: <EOF> \",\" \";\" \"HAVING\" \"ORDER BY\" ", e.getMessage());
		}

		try{
		// GROUP BY with a SELECT item index
		parser.parseQuery("SELECT * FROM cat GROUP BY 1;");

test/adql/translator/TestSQLServerTranslator.java

+70 −13

Original line number	Diff line number	Diff line
		package adql.translator;

		import static org.junit.Assert.*;
		import static org.junit.Assert.assertEquals;
		import static org.junit.Assert.fail;

		import java.util.ArrayList;
		import java.util.List;
		@@ -9,13 +10,17 @@ import org.junit.Before;
		import org.junit.Test;

		import adql.db.DBChecker;
		import adql.db.DBColumn;
		import adql.db.DBTable;
		import adql.db.DefaultDBColumn;
		import adql.db.DefaultDBTable;
		import adql.db.SearchColumnList;
		import adql.parser.ADQLParser;
		import adql.parser.ParseException;
		import adql.parser.SQLServer_ADQLQueryFactory;
		import adql.query.ADQLQuery;
		import adql.query.from.ADQLJoin;
		import adql.query.operand.ADQLColumn;

		public class TestSQLServerTranslator {

		@@ -59,6 +64,58 @@ public class TestSQLServerTranslator {
		}
		}

		@Test
		public void testNaturalJoin2(){
		final String adqlquery = "SELECT id, name, aColumn, anotherColumn FROM aTable \"A\" NATURAL JOIN anotherTable B;";

		try{
		ADQLQuery query = (new ADQLParser(new DBChecker(tables), new SQLServer_ADQLQueryFactory())).parseQuery(adqlquery);
		SQLServerTranslator translator = new SQLServerTranslator();

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

		try{
		StringBuffer buf = new StringBuffer();

		// Find duplicated items between the two lists and translate them as ON conditions:
		DBColumn rightCol;
		SearchColumnList leftList = join.getLeftTable().getDBColumns();
		SearchColumnList rightList = join.getRightTable().getDBColumns();
		for(DBColumn leftCol : leftList){
		// search for at most one column with the same name in the RIGHT list
		// and throw an exception is there are several matches:
		rightCol = ADQLJoin.findAtMostOneColumn(leftCol.getADQLName(), (byte)0, rightList, false);
		// if there is one...
		if (rightCol != null){
		// ...check there is only one column with this name in the LEFT list,
		// and throw an exception if it is not the case:
		ADQLJoin.findExactlyOneColumn(leftCol.getADQLName(), (byte)0, leftList, true);
		// ...append the corresponding join condition:
		if (buf.length() > 0)
		buf.append(" AND ");
		ADQLColumn col = new ADQLColumn(leftCol.getADQLName());
		col.setDBLink(leftCol);
		// TODO col.setAdqlTable(adqlTable);
		buf.append(translator.translate(col));
		buf.append("=");
		col = new ADQLColumn(rightCol.getADQLName());
		col.setDBLink(rightCol);
		buf.append(translator.translate(col));
		}
		}

		System.out.println("ON " + buf.toString());
		}catch(Exception uje){
		System.err.println("Impossible to resolve the NATURAL JOIN between " + join.getLeftTable().toADQL() + " and " + join.getRightTable().toADQL() + "!");
		uje.printStackTrace();
		}

		}catch(ParseException pe){
		pe.printStackTrace();
		fail("The given ADQL query is completely correct. No error should have occurred while parsing it. (see the console for more details)");
		}
		}

		@Test
		public void testJoinWithUSING(){
		final String adqlquery = "SELECT B.id, name, aColumn, anotherColumn FROM aTable A JOIN anotherTable B USING(name);";