From 61b1a0793330a65f819b140ab982a5545f34841a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9gory=20Mantelet?=
<gregory.mantelet@astro.unistra.fr>
Date: Mon, 26 Aug 2019 15:50:48 +0200
Subject: [PATCH] [ADQL] Improve error report in case of incorrect argument for
numeric functions.
---
src/adql/parser/grammar/ADQLGrammarBase.java | 6 ++++-
src/adql/parser/grammar/adqlGrammar201.jj | 16 +++++++-----
test/adql/parser/TestADQLParser.java | 26 ++++++++++++++++++++
3 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/src/adql/parser/grammar/ADQLGrammarBase.java b/src/adql/parser/grammar/ADQLGrammarBase.java
index eecb0cb..71d1ee5 100644
--- a/src/adql/parser/grammar/ADQLGrammarBase.java
+++ b/src/adql/parser/grammar/ADQLGrammarBase.java
@@ -133,7 +133,11 @@ public abstract class ADQLGrammarBase implements ADQLGrammar {
@Override
public final ParseException generateParseException(Exception ex) {
if (!(ex instanceof ParseException)) {
- ParseException pex = new ParseException("[" + ex.getClass().getName() + "] " + ex.getMessage());
+ ParseException pex;
+ if (ex instanceof IllegalArgumentException)
+ pex = new ParseException("Incorrect argument: " + ex.getMessage());
+ else
+ pex = new ParseException("[" + ex.getClass().getName() + "] " + ex.getMessage());
pex.setStackTrace(ex.getStackTrace());
return pex;
} else
diff --git a/src/adql/parser/grammar/adqlGrammar201.jj b/src/adql/parser/grammar/adqlGrammar201.jj
index 8568988..9567045 100644
--- a/src/adql/parser/grammar/adqlGrammar201.jj
+++ b/src/adql/parser/grammar/adqlGrammar201.jj
@@ -892,14 +892,16 @@ ADQLOperand ValueExpression(): {ADQLOperand valueExpr = null; Token left, right;
| LOOKAHEAD(3) valueExpr=Factor()
/* At this position in this switch, all possibilities (including
- * Column()) have already been tested and failed.
+ * Column() and NumericFunction) have already been tested and failed.
*
- * So, this final choice actually aims to throw an error set with the
+ * So, these final choices actually aim to throw an error set with the
* current token and with an error message implying that a column name
- * was expected (which is generally the case in an ADQL query).
- *
- * Note: This choice will generally be reached if an unexpected ADQL/SQL
- * word is ending the query. */
+ * was expected (which is generally the case in an ADQL query) or that
+ * the parameters of the numeric function are incorrect. */
+ | LOOKAHEAD(2) valueExpr=NumericFunction()
+ /*
+ * Note: Besides, this particular choice will generally be reached if an
+ * unexpected ADQL/SQL word is ending the query. */
| valueExpr=Column() )
{return valueExpr;}
}catch(Exception ex){
@@ -1003,6 +1005,8 @@ ADQLOperand StringExpression(): {ADQLOperand leftOp; ADQLOperand rightOp = null;
if (leftOp instanceof Concatenation){
Concatenation concat = (Concatenation)leftOp;
concat.setPosition(new TextPosition(concat.get(0).getPosition(), concat.get(concat.size()-1).getPosition()));
+ }else if (leftOp instanceof ADQLColumn){
+ ((ADQLColumn)leftOp).setExpectedType('S');
}
return leftOp;
}
diff --git a/test/adql/parser/TestADQLParser.java b/test/adql/parser/TestADQLParser.java
index f46d1e1..6e3a76c 100644
--- a/test/adql/parser/TestADQLParser.java
+++ b/test/adql/parser/TestADQLParser.java
@@ -55,6 +55,32 @@ public class TestADQLParser {
public void tearDown() throws Exception {
}
+ @Test
+ public void testNumericFunctionParams() {
+
+ ADQLParser parser = new ADQLParser(ADQLVersion.V2_1);
+
+ /* CASE: LOWER can only take a string in parameter, but according to the
+ * grammar (and BNF), an unsigned numeric is a string (??).
+ * In such case, an error should be raised: */
+ try {
+ parser.parseQuery("SELECT LOWER(123) FROM foo");
+ fail("LOWER can not take a numeric in parameter.");
+ } catch(Exception ex) {
+ assertEquals(ParseException.class, ex.getClass());
+ assertEquals("Incorrect argument: The ADQL function LOWER must have one parameter of type VARCHAR (i.e. a String)!", ex.getMessage());
+ }
+
+ // CASE: Idem for a second parameter:
+ try {
+ parser.parseQuery("SELECT IN_UNIT(12.3, 123) FROM foo");
+ fail("IN_UNIT can not take a numeric in 2nd parameter.");
+ } catch(Exception ex) {
+ assertEquals(ParseException.class, ex.getClass());
+ assertEquals("Incorrect argument: The 2nd argument of the ADQL function IN_UNIT (i.e. target unit) must be of type VARCHAR (i.e. a string)!", ex.getMessage());
+ }
+ }
+
@Test
public void testOperatorsPrecedence() {
--
GitLab