some cleaning up for pr #702

Author: wumpz

README.md

@@ -51,6 +51,7 @@ Also I would like to know about needed examples or documentation stuff.
 
 ## Extensions in the latest SNAPSHOT version 1.4
 
+* support of **substring(col from 2), position('test' in col), ..** and more (pull request #702)
 * support of db2 **VALUES** syntax (issue #561)
 
 ## Extensions of JSqlParser releases

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt

@@ -2347,17 +2347,17 @@ NamedExpressionList NamedExpressionList1():
     List<String> names = new ArrayList<String>();
     Expression expr1 = null;
     Expression expr2 = null;
-    Expression expr3 = null;
-    Expression expr4 = null;
 	String name = "";
     Token tk1 = null;
     Token tk2 = null;
-    Token tk3 = null;
-    Token tk4 = null;
 }
 {
     ( 
-     expr1=SimpleExpression()  (tk2=<K_FROM>|tk2=<K_IN>|tk2=<K_PLACING>) expr2=SimpleExpression() { expressions.add(expr1); names.add(tk2.image); expressions.add(expr2);}
+		(tk1=<K_BOTH>|tk1=<K_LEADING>|tk1=<K_TRAILING>) { names.add(tk1.image); }
+		expr1=SimpleExpression()  
+		(tk2=<K_FROM>|tk2=<K_IN>|tk2=<K_PLACING>) 
+		expr2=SimpleExpression() 
+		{ expressions.add(expr1); names.add(tk2.image); expressions.add(expr2);}
     )
 
     {
@@ -2374,7 +2374,7 @@ NamedExpressionList NamedExpressionList1():
 // overlay(expr1 placing expr2 from expr3)
 // overlay(expr1 placing expr2 from expr3 for expr4)
 // expr1 has already been consumed
-NamedExpressionList NamedExpressionList2():
+NamedExpressionList NamedExpressionListExprFirst():
 {
     NamedExpressionList retval = new NamedExpressionList();
     List<Expression> expressions = new ArrayList<Expression>();
@@ -2383,18 +2383,28 @@ NamedExpressionList NamedExpressionList2():
     Expression expr2 = null;
     Expression expr3 = null;
     Expression expr4 = null;
-	String name = "";
-    Token tk1 = null;
     Token tk2 = null;
     Token tk3 = null;
     Token tk4 = null;
 }
 {
+	expr1=SimpleExpression() 
+	(tk2=<K_FROM>|tk2=<K_IN>|tk2=<K_PLACING>) 
+	{
+		names.add("");
+		expressions.add(expr1);
+		names.add(tk2.image);
+	}
     ( 
-        expr2=SimpleExpression() { names.add(""); expressions.add(expr2);}
-     ( (tk3=<K_FOR>|tk3=<K_FROM>) expr3=SimpleExpression() {names.add(tk3.image); expressions.add(expr3);} 
-       ( (tk4=<K_FOR>) expr4=SimpleExpression() {names.add(tk4.image); expressions.add(expr4);} )?
-     )?
+        expr2=SimpleExpression() { expressions.add(expr2);}
+		( 
+			(tk3=<K_FOR>|tk3=<K_FROM>) 
+			expr3=SimpleExpression() {names.add(tk3.image); expressions.add(expr3);} 
+			(	
+				(tk4=<K_FOR>) 
+				expr4=SimpleExpression() {names.add(tk4.image); expressions.add(expr4);} 
+			)?
+		)?
     )
 
     {
@@ -3031,34 +3041,25 @@ Function Function() #Function:
 
     [ "." tmp=RelObjectNameExt() { funcName+= "." + tmp; } ["." tmp=RelObjectNameExt() { funcName+= "." + tmp; }]]
     "(" [ [<K_DISTINCT> { retval.setDistinct(true); } | <K_ALL> { retval.setAllColumns(true); }]
-// The addition of functions with named parameters (see NamedExpressionList1, NamedExpressionList2) complicates
-// the parsing of the paramter lists.  JavaCC needs factorization here so that the tokens which indicate branch points
-// are exposed.
         ( LOOKAHEAD(4)
-        ( (tk1=<K_BOTH>|tk1=<K_LEADING>|tk1=<K_TRAILING>) namedExpressionList=NamedExpressionList1() {namedExpressionList.getNames().add(0,tk1.image);})
-
-        | ( expr1=SimpleExpression() ( ((tk2=<K_FROM>|tk2=<K_IN>|tk2=<K_PLACING>) namedExpressionList=NamedExpressionList2() {namedExpressionList.getNames().set(0,tk2.image); namedExpressionList.getNames().add(0, ""); namedExpressionList.getExpressions().add(0, expr1);}) 
-          | ("," expressionList=SimpleExpressionList() {expressionList.getExpressions().add(0,expr1);}  )? ) )
-
-        | "*" { retval.setAllColumns(true); }
-        |  expr = SubSelect() { expr.setUseBrackets(false); expressionList = new ExpressionList(expr); } 
-        )
-        ] ")"
+			namedExpressionList=NamedExpressionList1()
+			| 
+			LOOKAHEAD(NamedExpressionListExprFirst()) namedExpressionList = NamedExpressionListExprFirst()
+			|
+			LOOKAHEAD(3) expressionList=SimpleExpressionList()
+			| 
+			"*" { retval.setAllColumns(true); }
+			|  
+			expr = SubSelect() { expr.setUseBrackets(false); expressionList = new ExpressionList(expr); } 
+        )] 
+	")"
 
     [ "." tmp=RelObjectName() { retval.setAttribute(tmp); }]
 
     [ keep = KeepExpression() ]
 
     ["}"]
     {
-        if(expr1 != null && expressionList==null && namedExpressionList == null){
-// If the function has a single parameter, it is consumed before the choice point between
-// named vs. simple expression list.  Put it in a simple ExpressionList here.
-            expressions = new ArrayList<Expression>();
-            expressions.add(expr1);
-            expressionList = new ExpressionList();
-            expressionList.setExpressions(expressions);
-        }
         retval.setParameters(expressionList);
         retval.setNamedParameters(namedExpressionList);
         retval.setName(funcName);

src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java

@@ -1035,6 +1035,17 @@ public void testEscapedFunctionsIssue647() throws JSQLParserException {
         assertSqlCanBeParsedAndDeparsed("SELECT {fn concat(a, b)} AS COL");
     }
 
+    @Test
+    public void testNamedParametersPR702() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed("SELECT substring(id, 2, 3), substring(id from 2 for 3), substring(id from 2), trim(BOTH ' ' from 'foo bar '), trim(LEADING ' ' from 'foo bar '), trim(TRAILING ' ' from 'foo bar '), trim(' ' from 'foo bar '), position('foo' in 'bar'), overlay('foo' placing 'bar' from 1), overlay('foo' placing 'bar' from 1 for 2) FROM my table");
+    }
+
+    @Test
+    public void testNamedParametersPR702_2() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed("SELECT substring(id, 2, 3) FROM mytable");
+        assertSqlCanBeParsedAndDeparsed("SELECT substring(id from 2 for 3) FROM mytable");
+    }
+
     @Test
     public void testWhere() throws JSQLParserException {
 

src/test/java/net/sf/jsqlparser/statement/select/SpecialOracleTest.java

@@ -18,20 +18,18 @@
  */
 package net.sf.jsqlparser.statement.select;
 
-import net.sf.jsqlparser.JSQLParserException;
-import net.sf.jsqlparser.parser.CCJSqlParserUtil;
-import net.sf.jsqlparser.statement.Statement;
-import org.apache.commons.io.FileUtils;
-import org.junit.ComparisonFailure;
-import org.junit.Test;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-
+import net.sf.jsqlparser.JSQLParserException;
+import net.sf.jsqlparser.parser.CCJSqlParserUtil;
+import net.sf.jsqlparser.statement.Statement;
 import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed;
+import org.apache.commons.io.FileUtils;
 import static org.junit.Assert.assertTrue;
+import org.junit.ComparisonFailure;
+import org.junit.Test;
 
 /**
  * Tries to parse and deparse all statments in net.sf.jsqlparser.test.oracle-tests.
@@ -77,7 +75,7 @@ public void testAllSqlsParseDeparse() throws IOException {
 
         LOG.
                 log(Level.INFO, "tested {0} files. got {1} correct parse results", new Object[]{count, success});
-        assertTrue(success >= 149);
+        assertTrue(success >= 150);
     }
 
     @Test