Commit d7927d84 authored by gmantele's avatar gmantele
Browse files

[ADQL] Prevent using the real name of a parent table inside subqueries when 

this table is declared with an alias. Instead, the table alias must be used.

Note: This problem occurred only when ADQLParser was used with a DBChecker.

This commit fixes the GitHub issue #53
parent 739eeec9

src/adql/db/DBChecker.java

+175 −149
Original line number Diff line number Diff line
@@ -99,7 +99,7 @@ import adql.search.SimpleSearchHandler; 
 * </i></p>

 *

 * @author Gr&eacute;gory Mantelet (CDS;ARI)

 * @version 1.4 (09/2017)

 * @version 1.4 (11/2017)

 */

public class DBChecker implements QueryChecker {
@@ -476,28 +476,53 @@ public class DBChecker implements QueryChecker { 
	}



	/**

	 * <p>Search all table references inside the given query, resolve them against the available tables, and if there is only one match,

	 * attach the matching metadata to them.</p>

	 * Search all table references inside the given query, resolve them against

	 * the available tables, and if there is only one match, attach the matching

	 * metadata to them.

	 *

	 * <b>Management of sub-query tables</b>

	 * <p>

	 * 	If a table is not a DB table reference but a sub-query, this latter is first checked (using {@link #check(ADQLQuery, Stack)} ;

	 * 	but the father list must not contain tables of the given query, because on the same level) and then corresponding table metadata

	 * 	are generated (using {@link #generateDBTable(ADQLQuery, String)}) and attached to it.

	 * 	If a table is not a DB table reference but a sub-query, this latter is

	 * 	first checked (using {@link #check(ADQLQuery, Stack)} ; but the father

	 * 	list must not contain tables of the given query, because on the same

	 * 	level) and then corresponding table metadata are generated (using

	 * 	{@link #generateDBTable(ADQLQuery, String)}) and attached to it.

	 * </p>

	 *

	 * <b>Management of "{table}.*" in the SELECT clause</b>

	 * <p>

	 * 	For each of this SELECT item, this function tries to resolve the table name. If only one match is found, the corresponding ADQL table object

	 * 	is got from the list of resolved tables and attached to this SELECT item (thus, the joker item will also have the good metadata,

	 * 	particularly if the referenced table is a sub-query).

	 * 	For each of this SELECT item, this function tries to resolve the table

	 * 	name. If only one match is found, the corresponding ADQL table object

	 * 	is got from the list of resolved tables and attached to this SELECT item

	 * 	(thus, the joker item will also have the good metadata, particularly if

	 * 	the referenced table is a sub-query).

	 * </p>

	 *

	 * @param query			Query in which the existence of tables must be checked.

	 * @param fathersList	List of all columns available in the father queries and that should be accessed in sub-queries.

	 *                      Each item of this stack is a list of columns available in each father-level query.

	 *                   	<i>Note: this parameter is NULL if this function is called with the root/father query as parameter.</i>

	 * @param errors		List of errors to complete in this function each time an unknown table or column is encountered.

	 * <b>Table alias</b>

	 * <p>

	 * 	When a simple table (i.e. not a sub-query) is aliased, the metadata of

	 * 	this table will be wrapped inside a {@link DBTableAlias} in order to

	 * 	keep the original metadata but still declare use the table with the

	 * 	alias instead of its original name. The original name will be used

	 * 	only when translating the corresponding FROM item ; the rest of the time

	 * 	(i.e. for references when using a column), the alias name must be used.

	 * </p>

	 * <p>

	 * 	In order to avoid unpredictable behavior at execution of the SQL query,

	 * 	the alias will be put in lower case if not defined between double

	 * 	quotes.

	 * </p>

	 *

	 * @param query			Query in which the existence of tables must be

	 *             			checked.

	 * @param fathersList	List of all columns available in the father queries

	 *                   	and that should be accessed in sub-queries.

	 *                      Each item of this stack is a list of columns

	 *                      available in each father-level query.

	 *                   	<i>Note: this parameter is NULL if this function is

	 *                   	called with the root/father query as parameter.</i>

	 * @param errors		List of errors to complete in this function each

	 *              		time an unknown table or column is encountered.

	 *

	 * @return	An associative map of all the resolved tables.

	 */
@@ -521,8 +546,9 @@ public class DBChecker implements QueryChecker { 
					dbTable = generateDBTable(table.getSubQuery(), table.getAlias());

				}else{

					dbTable = resolveTable(table);

					if (table.hasAlias())

						dbTable = dbTable.copy(null, table.getAlias());

					// wrap this table metadata if an alias should be used:

					if (dbTable != null && table.hasAlias())

						dbTable = new DBTableAlias(dbTable, (table.isCaseSensitive(IdentifierField.ALIAS) ? table.getAlias() : table.getAlias().toLowerCase()));

				}



				// link with the matched DBTable:

src/adql/db/DBTableAlias.java

0 → 100644
+77 −0
Original line number Diff line number Diff line 
package adql.db;



/*

 * This file is part of ADQLLibrary.

 *

 * ADQLLibrary is free software: you can redistribute it and/or modify

 * it under the terms of the GNU Lesser General Public License as published by

 * the Free Software Foundation, either version 3 of the License, or

 * (at your option) any later version.

 *

 * ADQLLibrary is distributed in the hope that it will be useful,

 * but WITHOUT ANY WARRANTY; without even the implied warranty of

 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

 * GNU Lesser General Public License for more details.

 *

 * 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 2017 - Astronomisches Rechen Institut (ARI)

 */



/**

 * This {@link DBTable} wraps another {@link DBTable} with a different ADQL and

 * DB name.

 *

 * <p>

 * 	This wrapper aims to represent in the metadata the aliasing of a table.

 * 	This table should not be part of any schema, in ADQL but also in SQL...it is

 * 	just an alias of an existing table.

 * </p>

 *

 * <p>

 * 	All columns of the origin table are completely copied into this

 * 	{@link DBTable} thanks to {@link DBColumn#copy(String, String, DBTable)},

 * 	with the same ADQL and DB name but a different parent table (this one is

 * 	used of the original one).

 * </p>

 *

 * <p><i>Note:

 * 	The origin table is still available thanks to the function

 * 	{@link #getOriginTable()}.

 * </i></p>

 *

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 1.4 (11/2017)

 * @since 1.4

 */

public class DBTableAlias extends DefaultDBTable {



	/** Wrapped table. */

	protected final DBTable originTable;



	/**

	 * Wrap the given table under the given ADQL/DB name.

	 *

	 * @param originTable	The table to wrap/alias.

	 * @param tableAlias	The alias name.

	 */

	public DBTableAlias(final DBTable originTable, final String tableAlias){

		super(null, null, tableAlias);



		this.originTable = originTable;



		for(DBColumn col : originTable)

			addColumn(col.copy(col.getDBName(), col.getADQLName(), this));

	}



	/**

	 * Get the aliased/wrapped table.

	 *

	 * @return	The aliased table.

	 */

	public DBTable getOriginTable(){

		return originTable;

	}



}

src/adql/translator/JDBCTranslator.java

+103 −79
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import java.util.List; 


import adql.db.DBColumn;

import adql.db.DBTable;

import adql.db.DBTableAlias;

import adql.db.DBType;

import adql.db.STCS.Region;

import adql.db.exception.UnresolvedJoinException;
@@ -166,7 +167,7 @@ import adql.query.operand.function.geometry.RegionFunction; 
 * </p>

 *

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 1.4 (09/2017)

 * @version 1.4 (11/2017)

 * @since 1.4

 *

 * @see PostgreSQLTranslator
@@ -573,8 +574,16 @@ public abstract class JDBCTranslator implements ADQLTranslator { 
		// CASE: TABLE REFERENCE:

		else{

			// Use the corresponding DB table, if known:

			if (table.getDBLink() != null)

			if (table.getDBLink() != null){

				/* Note: if the table is aliased, the aliased table is wrapped

				 *       inside a DBTableAlias. So, to get the real table name

				 *       we should get first the original table thanks to

				 *       DBTableAlias.getOriginTable(). */

				if (table.getDBLink() instanceof DBTableAlias)

					sql.append(getQualifiedTableName(((DBTableAlias)table.getDBLink()).getOriginTable()));

				else

					sql.append(getQualifiedTableName(table.getDBLink()));

			}

			// Otherwise, use the whole table name given in the ADQL query:

			else

				sql.append(table.getFullTableName());
@@ -583,6 +592,18 @@ public abstract class JDBCTranslator implements ADQLTranslator { 
		// Add the table alias, if any:

		if (table.hasAlias()){

			sql.append(" AS ");

			/* In case where metadata are known, the alias must always be

			 * written case sensitively in order to ensure a translation

			 * stability (i.e. all references clearly point toward this alias

			 * whatever is their character case). */

			if (table.getDBLink() != null){

				if (table.isCaseSensitive(IdentifierField.ALIAS))

					appendIdentifier(sql, table.getAlias(), true);

				else

					appendIdentifier(sql, table.getAlias().toLowerCase(), true);

			}

			/* Otherwise, just write what is written in ADQL: */

			else

				appendIdentifier(sql, table.getAlias(), table.isCaseSensitive(IdentifierField.ALIAS));

		}
@@ -651,13 +672,16 @@ public abstract class JDBCTranslator implements ADQLTranslator { 
		if (column.getDBLink() != null){

			DBColumn dbCol = column.getDBLink();

			StringBuffer colName = new StringBuffer();

			// Use the table alias if any:

			if (column.getAdqlTable() != null && column.getAdqlTable().hasAlias())

				appendIdentifier(colName, column.getAdqlTable().getAlias(), column.getAdqlTable().isCaseSensitive(IdentifierField.ALIAS)).append('.');



			// Use the DBTable if any:

			else if (dbCol.getTable() != null && dbCol.getTable().getDBName() != null)

			if (dbCol.getTable() != null && dbCol.getTable().getDBName() != null){

				/* Note: if the table is aliased, ensure no schema is prefixing

				 *       this alias thanks to getTableName(..., false). */

				if (dbCol.getTable() instanceof DBTableAlias)

					colName.append(getTableName(dbCol.getTable(), false)).append('.');

				else

					colName.append(getQualifiedTableName(dbCol.getTable())).append('.');

			}



			// Otherwise, use the prefix of the column given in the ADQL query:

			else if (column.getTableName() != null)

test/adql/db/TestSeveralSubQueries.java→test/adql/db/TestSubQueries.java

+24 −2
Original line number Diff line number Diff line
@@ -12,17 +12,18 @@ import org.junit.Test; 


import adql.parser.ADQLParser;

import adql.query.ADQLQuery;

import adql.translator.PostgreSQLTranslator;

import tap.metadata.TAPMetadata;

import tap.metadata.TAPTable;

import tap.metadata.TableSetParser;



public class TestSeveralSubQueries {

public class TestSubQueries {



	@Before

	public void setUp() throws Exception{}



	@Test

	public void test(){

	public void testSeveralSubqueries(){

		try{

			TableSetParser tsParser = new TableSetParser();

			TAPMetadata esaMetaData = tsParser.parse(new File("test/adql/db/subquery_test_tables.xml"));
@@ -41,4 +42,25 @@ public class TestSeveralSubQueries { 
		}

	}



	@Test

	public void testFatherTableAliasIntoSubqueries(){

		try{

			TableSetParser tsParser = new TableSetParser();

			TAPMetadata esaMetaData = tsParser.parse(new File("test/adql/db/subquery_test_tables.xml"));

			ArrayList<DBTable> esaTables = new ArrayList<DBTable>(esaMetaData.getNbTables());

			Iterator<TAPTable> itTables = esaMetaData.getTables();

			while(itTables.hasNext())

				esaTables.add(itTables.next());



			ADQLParser adqlParser = new ADQLParser(new DBChecker(esaTables));



			ADQLQuery query = adqlParser.parseQuery("SELECT oid FROM table1 as MyAlias WHERE oid IN (SELECT oid2 FROM table2 WHERE oid2 = myAlias.oid)");

			System.out.println((new PostgreSQLTranslator()).translate(query));

			assertEquals("SELECT \"myalias\".\"oid\" AS \"oid\"\nFROM \"public\".\"table1\" AS \"myalias\"\nWHERE \"myalias\".\"oid\" IN (SELECT \"public\".\"table2\".\"oid2\" AS \"oid2\"\nFROM \"public\".\"table2\"\nWHERE \"public\".\"table2\".\"oid2\" = \"myalias\".\"oid\")", (new PostgreSQLTranslator()).translate(query));

		}catch(Exception ex){

			ex.printStackTrace(System.err);

			fail("No error expected! (see console for more details)");

		}

	}



}

test/adql/translator/TestSQLServerTranslator.java

+4 −4
Original line number Diff line number Diff line
@@ -46,10 +46,10 @@ public class TestSQLServerTranslator { 
			SQLServerTranslator translator = new SQLServerTranslator();



			// Test the FROM part:

			assertEquals("\"aTable\" AS A INNER JOIN \"anotherTable\" AS B ON A.\"id\"=B.\"id\" AND A.\"name\"=B.\"name\"", translator.translate(query.getFrom()));

			assertEquals("\"aTable\" AS \"a\" INNER JOIN \"anotherTable\" AS \"b\" ON \"a\".\"id\"=\"b\".\"id\" AND \"a\".\"name\"=\"b\".\"name\"", translator.translate(query.getFrom()));



			// Test the SELECT part (in order to ensure the usual common columns (due to NATURAL) are actually translated as columns of the first joined table):

			assertEquals("SELECT A.\"id\" AS \"id\" , A.\"name\" AS \"name\" , A.\"aColumn\" AS \"aColumn\" , B.\"anotherColumn\" AS \"anotherColumn\"", translator.translate(query.getSelect()));

			assertEquals("SELECT \"a\".\"id\" AS \"id\" , \"a\".\"name\" AS \"name\" , \"a\".\"aColumn\" AS \"aColumn\" , \"b\".\"anotherColumn\" AS \"anotherColumn\"", translator.translate(query.getSelect()));



		}catch(ParseException pe){

			pe.printStackTrace();
@@ -69,10 +69,10 @@ public class TestSQLServerTranslator { 
			SQLServerTranslator translator = new SQLServerTranslator();



			// Test the FROM part:

			assertEquals("\"aTable\" AS A INNER JOIN \"anotherTable\" AS B ON A.\"name\"=B.\"name\"", translator.translate(query.getFrom()));

			assertEquals("\"aTable\" AS \"a\" INNER JOIN \"anotherTable\" AS \"b\" ON \"a\".\"name\"=\"b\".\"name\"", translator.translate(query.getFrom()));



			// Test the SELECT part (in order to ensure the usual common columns (due to USING) are actually translated as columns of the first joined table):

			assertEquals("SELECT B.\"id\" AS \"id\" , A.\"name\" AS \"name\" , A.\"aColumn\" AS \"aColumn\" , B.\"anotherColumn\" AS \"anotherColumn\"", translator.translate(query.getSelect()));

			assertEquals("SELECT \"b\".\"id\" AS \"id\" , \"a\".\"name\" AS \"name\" , \"a\".\"aColumn\" AS \"aColumn\" , \"b\".\"anotherColumn\" AS \"anotherColumn\"", translator.translate(query.getSelect()));



		}catch(ParseException pe){

			pe.printStackTrace();