diff --git a/src/adql/parser/ADQLParser.java b/src/adql/parser/ADQLParser.java index 80c3990169bc221aa59dbf115a59314770edf3f4..c98e3d9d0f5cfb7f65a245beb380044581762314 100644 --- a/src/adql/parser/ADQLParser.java +++ b/src/adql/parser/ADQLParser.java @@ -67,7 +67,7 @@ import adql.translator.TranslationException; * @see ADQLQueryFactory * * @author Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de -* @version 1.4 (06/2015) +* @version 1.4 (08/2015) */ public class ADQLParser implements ADQLParserConstants { @@ -1074,12 +1074,12 @@ public class ADQLParser implements ADQLParserConstants { final public ColumnReference ColumnRef() throws ParseException{ trace_call("ColumnRef"); try{ - IdentifierItems identifiers = null; + IdentifierItem identifier = null; Token ind = null; switch((jj_ntk == -1) ? jj_ntk() : jj_ntk){ case DELIMITED_IDENTIFIER: case REGULAR_IDENTIFIER: - identifiers = ColumnName(); + identifier = Identifier(); break; case UNSIGNED_INTEGER: ind = jj_consume_token(UNSIGNED_INTEGER); @@ -1091,8 +1091,8 @@ public class ADQLParser implements ADQLParserConstants { } try{ ColumnReference colRef = null; - if (identifiers != null) - colRef = queryFactory.createColRef(identifiers); + if (identifier != null) + colRef = queryFactory.createColRef(identifier); else colRef = queryFactory.createColRef(Integer.parseInt(ind.image), new TextPosition(ind)); { @@ -1114,12 +1114,12 @@ public class ADQLParser implements ADQLParserConstants { final public ADQLOrder OrderItem() throws ParseException{ trace_call("OrderItem"); try{ - IdentifierItems identifiers = null; + IdentifierItem identifier = null; Token ind = null, desc = null; switch((jj_ntk == -1) ? jj_ntk() : jj_ntk){ case DELIMITED_IDENTIFIER: case REGULAR_IDENTIFIER: - identifiers = ColumnName(); + identifier = Identifier(); break; case UNSIGNED_INTEGER: ind = jj_consume_token(UNSIGNED_INTEGER); @@ -1151,9 +1151,9 @@ public class ADQLParser implements ADQLParserConstants { } try{ ADQLOrder order = null; - if (identifiers != null){ - order = queryFactory.createOrder(identifiers, desc != null); - order.setPosition(identifiers.getPosition()); + if (identifier != null){ + order = queryFactory.createOrder(identifier, desc != null); + order.setPosition(identifier.position); }else{ order = queryFactory.createOrder(Integer.parseInt(ind.image), desc != null); order.setPosition(new TextPosition(ind)); diff --git a/src/adql/parser/ADQLQueryFactory.java b/src/adql/parser/ADQLQueryFactory.java index 4bcfdf2fbfc8828d34bc123e7001e111eb994c93..e10931e422cbd616808b3a18b5e402f2f225432a 100644 --- a/src/adql/parser/ADQLQueryFactory.java +++ b/src/adql/parser/ADQLQueryFactory.java @@ -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 (06/2015) + * @version 1.4 (08/2015) * * @see ADQLParser */ @@ -403,6 +403,17 @@ public class ADQLQueryFactory { return order; } + public ADQLOrder createOrder(final IdentifierItem colName, final boolean desc) throws Exception{ + ADQLOrder order = new ADQLOrder(colName.identifier, desc); + if (order != null) + order.setCaseSensitive(colName.caseSensitivity); + return order; + } + + /** + * @deprecated since 1.4 ; Former version's mistake: an ORDER BY item is either a regular/delimited column name or an integer, not a qualified column name ; Replaced by {@link #createOrder(Identifier, boolean)} ; This function is no longer used by ADQLParser. + */ + @Deprecated public ADQLOrder createOrder(final IdentifierItems idItems, final boolean desc) throws Exception{ ADQLOrder order = new ADQLOrder(idItems.join("."), desc); if (order != null) @@ -410,6 +421,19 @@ public class ADQLQueryFactory { return order; } + public ColumnReference createColRef(final IdentifierItem idItem) throws Exception{ + ColumnReference colRef = new ColumnReference(idItem.identifier); + if (colRef != null){ + colRef.setPosition(idItem.position); + colRef.setCaseSensitive(idItem.caseSensitivity); + } + 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(Identifier)} ; 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 2b771a931d7b31bf2aca158771222fc4be9c99fa..7d4c062fce2958bb383ce02dedb556a84f226f8d 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 (06/2015) +* Version: 1.4 (08/2015) */ /* ########### */ @@ -89,7 +89,7 @@ import adql.translator.TranslationException; * @see ADQLQueryFactory * * @author Grégory Mantelet (CDS;ARI) - gmantele@ari.uni-heidelberg.de -* @version 1.4 (06/2015) +* @version 1.4 (08/2015) */ public class ADQLParser { @@ -918,13 +918,13 @@ ADQLColumn Column(): {IdentifierItems identifiers;} { } } -ColumnReference ColumnRef(): {IdentifierItems identifiers = null; Token ind = null;}{ - ( identifiers=ColumnName() | ind=<UNSIGNED_INTEGER> ) +ColumnReference ColumnRef(): {IdentifierItem identifier = null; Token ind = null;}{ + ( identifier=Identifier() | ind=<UNSIGNED_INTEGER> ) { try{ ColumnReference colRef = null; - if (identifiers != null) - colRef = queryFactory.createColRef(identifiers); + if (identifier != null) + colRef = queryFactory.createColRef(identifier); else colRef = queryFactory.createColRef(Integer.parseInt(ind.image), new TextPosition(ind)); return colRef; @@ -934,14 +934,14 @@ ColumnReference ColumnRef(): {IdentifierItems identifiers = null; Token ind = nu } } -ADQLOrder OrderItem(): {IdentifierItems identifiers = null; Token ind = null, desc = null;}{ - (identifiers=ColumnName() | ind=<UNSIGNED_INTEGER>) (<ASC> | desc=<DESC>)? +ADQLOrder OrderItem(): {IdentifierItem identifier = null; Token ind = null, desc = null;}{ + (identifier=Identifier() | ind=<UNSIGNED_INTEGER>) (<ASC> | desc=<DESC>)? { try{ ADQLOrder order = null; - if (identifiers != null){ - order = queryFactory.createOrder(identifiers, desc!=null); - order.setPosition(identifiers.getPosition()); + if (identifier != null){ + order = queryFactory.createOrder(identifier, desc!=null); + order.setPosition(identifier.position); }else{ order = queryFactory.createOrder(Integer.parseInt(ind.image), desc!=null); order.setPosition(new TextPosition(ind)); diff --git a/test/adql/parser/TestADQLParser.java b/test/adql/parser/TestADQLParser.java index 82366d684db0ebfeda334f14b1c3a4ac3bf0f57b..1272d6792fb9d2c6a384880a055921c4f411d222 100644 --- a/test/adql/parser/TestADQLParser.java +++ b/test/adql/parser/TestADQLParser.java @@ -26,7 +26,74 @@ public class TestADQLParser { @After public void tearDown() throws Exception{} - + + @Test + public void testColumnReference(){ + ADQLParser parser = new ADQLParser(); + try{ + // ORDER BY + parser.parseQuery("SELECT * FROM cat ORDER BY oid;"); + parser.parseQuery("SELECT * FROM cat ORDER BY oid ASC;"); + parser.parseQuery("SELECT * FROM cat ORDER BY oid DESC;"); + parser.parseQuery("SELECT * FROM cat ORDER BY 1;"); + parser.parseQuery("SELECT * FROM cat ORDER BY 1 ASC;"); + 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 1;"); + // JOIN ... USING(...) + parser.parseQuery("SELECT * FROM cat JOIN cat2 USING(oid);"); + }catch(Exception e){ + e.printStackTrace(System.err); + fail("These ADQL queries are strictly correct! No error should have occured. (see stdout for more details)"); + } + + try{ + // ORDER BY + parser.parseQuery("SELECT * FROM cat ORDER BY cat.oid;"); + fail("A qualified column name is forbidden in ORDER BY! This test should have failed."); + }catch(Exception e){ + assertEquals(ParseException.class, e.getClass()); + assertEquals(" Encountered \".\". Was expecting one of: <EOF> \",\" \";\" \"ASC\" \"DESC\" ", e.getMessage()); + } + + // Query reported as in error before the bug correction: + try{ + parser.parseQuery("SELECT TOP 10 browndwarfs.cat.jmag FROM browndwarfs.cat ORDER BY browndwarfs.cat.jmag"); + fail("A qualified column name is forbidden in ORDER BY! This test should have failed."); + }catch(Exception e){ + assertEquals(ParseException.class, e.getClass()); + assertEquals(" Encountered \".\". Was expecting one of: <EOF> \",\" \";\" \"ASC\" \"DESC\" ", e.getMessage()); + } + + try{ + // GROUP BY + parser.parseQuery("SELECT * FROM cat GROUP BY cat.oid;"); + fail("A qualified column name is forbidden in ORDER 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{ + // JOIN ... USING(...) + parser.parseQuery("SELECT * FROM cat JOIN cat2 USING(cat.oid);"); + fail("A qualified column name is forbidden in USING(...)! This test should have failed."); + }catch(Exception e){ + assertEquals(ParseException.class, e.getClass()); + assertEquals(" Encountered \".\". Was expecting one of: \")\" \",\" ", e.getMessage()); + } + + try{ + // JOIN ... USING(...) + parser.parseQuery("SELECT * FROM cat JOIN cat2 USING(1);"); + fail("A column index is forbidden in USING(...)! This test should have failed."); + }catch(Exception e){ + assertEquals(ParseException.class, e.getClass()); + assertEquals(" Encountered \"1\". Was expecting one of: \"\\\"\" <REGULAR_IDENTIFIER> ", e.getMessage()); + } + } + @Test public void testDelimitedIdentifiersWithDot(){ ADQLParser parser = new ADQLParser();