From 7a70c6038cef460ab169682bed391bb5ae1de1e9 Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Mon, 3 Apr 2017 16:31:21 +0200
Subject: [PATCH] [ADQL] Re-Fix GROUP BY's columns handling: a qualified column
name should be allowed, but still no column index should be.
---
src/adql/parser/ADQLParser.java | 24 ++----
src/adql/parser/ADQLQueryFactory.java | 8 +-
src/adql/parser/adqlGrammar.jj | 10 +--
test/adql/parser/TestADQLParser.java | 10 +--
.../translator/TestSQLServerTranslator.java | 83 ++++++++++++++++---
5 files changed, 83 insertions(+), 52 deletions(-)
diff --git a/src/adql/parser/ADQLParser.java b/src/adql/parser/ADQLParser.java
index 40d14d3..8da4035 100644
--- a/src/adql/parser/ADQLParser.java
+++ b/src/adql/parser/ADQLParser.java
@@ -1,19 +1,5 @@
/* 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){
{
diff --git a/src/adql/parser/ADQLQueryFactory.java b/src/adql/parser/ADQLQueryFactory.java
index c6b70b7..b6f8e44 100644
--- a/src/adql/parser/ADQLQueryFactory.java
+++ b/src/adql/parser/ADQLQueryFactory.java
@@ -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){
diff --git a/src/adql/parser/adqlGrammar.jj b/src/adql/parser/adqlGrammar.jj
index c0106f9..857997b 100644
--- a/src/adql/parser/adqlGrammar.jj
+++ b/src/adql/parser/adqlGrammar.jj
@@ -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);
}
diff --git a/test/adql/parser/TestADQLParser.java b/test/adql/parser/TestADQLParser.java
index f7b3889..9e6a016 100644
--- a/test/adql/parser/TestADQLParser.java
+++ b/test/adql/parser/TestADQLParser.java
@@ -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;");
diff --git a/test/adql/translator/TestSQLServerTranslator.java b/test/adql/translator/TestSQLServerTranslator.java
index d3a0290..4ddd1bd 100644
--- a/test/adql/translator/TestSQLServerTranslator.java
+++ b/test/adql/translator/TestSQLServerTranslator.java
@@ -1,6 +1,7 @@
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,20 +10,24 @@ 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 {
-
+
private List<DBTable> tables = null;
@Before
- public void setUp() throws Exception {
+ public void setUp() throws Exception{
tables = new ArrayList<DBTable>(2);
DefaultDBTable t = new DefaultDBTable("aTable");
t.addColumn(new DefaultDBColumn("id", t));
@@ -37,19 +42,19 @@ public class TestSQLServerTranslator {
}
@Test
- public void testNaturalJoin() {
+ public void testNaturalJoin(){
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();
-
+
// Test the FROM part:
assertEquals("\"aTable\" AS A INNER JOIN \"anotherTable\" AS B ON \"aTable\".\"id\"=\"anotherTable\".\"id\" AND \"aTable\".\"name\"=\"anotherTable\".\"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()));
-
+
}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)");
@@ -60,19 +65,71 @@ public class TestSQLServerTranslator {
}
@Test
- public void testJoinWithUSING() {
+ 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);";
-
+
try{
ADQLQuery query = (new ADQLParser(new DBChecker(tables), new SQLServer_ADQLQueryFactory())).parseQuery(adqlquery);
SQLServerTranslator translator = new SQLServerTranslator();
-
+
// Test the FROM part:
assertEquals("\"aTable\" AS A INNER JOIN \"anotherTable\" AS B ON \"aTable\".\"name\"=\"anotherTable\".\"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()));
-
+
}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)");
--
GitLab