Обсуждение: Patch for jdbc escaped functions

Поиск
Список
Период
Сортировка

Patch for jdbc escaped functions

От
Xavier Poinsard
Дата:
Hi,
I saw that escaped functions were on the TODO list.
Here is a patch that prepare for adding escaped functions.
It supports nested escapes.
As example I have coded the concat function.
Tests and metadata updates are included.
If the patch is accepted, I will add more functions.
Any comments are welcome.

Index: jdbc2/AbstractJdbc2DatabaseMetaData.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java,v
retrieving revision 1.16
diff -u -r1.16 AbstractJdbc2DatabaseMetaData.java
--- jdbc2/AbstractJdbc2DatabaseMetaData.java    9 Nov 2004 08:48:31 -0000    1.16
+++ jdbc2/AbstractJdbc2DatabaseMetaData.java    19 Nov 2004 10:17:47 -0000
@@ -475,7 +475,7 @@
     {
         if (Driver.logDebug)
             Driver.debug("getStringFunctions");
-        return "";
+        return EscapedFunctions.CONCAT;
     }

     public String getSystemFunctions() throws SQLException
Index: jdbc2/AbstractJdbc2Statement.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java,v
retrieving revision 1.50
diff -u -r1.50 AbstractJdbc2Statement.java
--- jdbc2/AbstractJdbc2Statement.java    17 Nov 2004 02:43:49 -0000    1.50
+++ jdbc2/AbstractJdbc2Statement.java    19 Nov 2004 10:17:53 -0000
@@ -655,8 +655,9 @@
      *
      * Currently implemented Escape clauses are those mentioned in 11.3
      * in the specification. Basically we look through the sql string for
-     * {d xxx}, {t xxx} or {ts xxx} in non-string sql code. When we find
-     * them, we just strip the escape part leaving only the xxx part.
+     * {d xxx}, {t xxx}, {ts xxx}, {oj xxx} or {fn xxx} in non-string sql
+     * code. When we find them, we just strip the escape part leaving only
+     * the xxx part.
      * So, something like "select * from x where d={d '2001-10-09'}" would
      * return "select * from x where d= '2001-10-09'".
      */
@@ -728,8 +729,33 @@
                     newsql.append(c);
                     break;

-                case ESC_TIMEDATE:
                 case ESC_FUNCTION:
+                    // extract function name
+                    String functionName;
+                    int posArgs = p_sql.indexOf('(',i);
+                    if (posArgs!=-1){
+                        functionName=p_sql.substring(i,posArgs).trim();
+                        // extract arguments
+                        i= posArgs+1;// we start the scan after the first (
+                        int nestedCount=0;
+                        while (i<len && !(p_sql.charAt(i)==')' && 0==nestedCount )){
+                            if ('('==p_sql.charAt(i))
+                                nestedCount++;
+                            else if (')'==p_sql.charAt(i))
+                                nestedCount--;
+                            i++;
+                        }
+                        String args=p_sql.substring(posArgs+1,i);
+                        // translate the function and parse arguments
+                        newsql.append(escapeFunction(functionName,args));
+                    }
+                    // go to the end of the function copying anything found
+                    i++;
+                    while (i<len && p_sql.charAt(i)!='}')
+                        newsql.append(p_sql.charAt(i));
+                    state = IN_SQLCODE; // end of escaped function (or query)
+                    break;
+                case ESC_TIMEDATE:
                 case ESC_OUTERJOIN:
                     if (c == '}')
                         state = IN_SQLCODE;    // end of escape code.
@@ -746,6 +772,54 @@
             return p_sql;
         }
     }
+    /**
+     * generate sql for escaped functions
+     * @param functionName the escaped function name
+     * @param args the arguments for this functin
+     * @return the right postgreSql sql
+     */
+    protected String escapeFunction(String functionName, String args){
+        // parse function arguments
+        int len = args.length();
+        int i=0;
+        ArrayList parsedArgs = new ArrayList();
+        while (i<len){
+            int nestedCount=0;
+            int lastPos=i;
+            while (i<len && !(args.charAt(i)==',' && 0==nestedCount )){
+                if ('('==args.charAt(i))
+                    nestedCount++;
+                else if (')'==args.charAt(i))
+                    nestedCount--;
+                i++;
+            }
+            if (lastPos!=i){
+                parsedArgs.add(replaceProcessing(args.substring(lastPos,i)));
+            }
+            i++;
+        }
+        // we can now tranlate escape functions
+        if (EscapedFunctions.CONCAT.equalsIgnoreCase(functionName)){
+            StringBuffer buf = new StringBuffer();
+            for (int iArg = 0;iArg<parsedArgs.size();iArg++){
+                buf.append(parsedArgs.get(iArg));
+                if (iArg!=(parsedArgs.size()-1))
+                    buf.append(" || ");
+            }
+            return buf.toString();
+        }else{
+            // by default the function name is kept unchanged
+            StringBuffer buf = new StringBuffer();
+            buf.append(functionName).append('(');
+            for (int iArg = 0;iArg<parsedArgs.size();iArg++){
+                buf.append(parsedArgs.get(iArg));
+                if (iArg!=(parsedArgs.size()-1))
+                    buf.append(',');
+            }
+            buf.append(')');
+            return buf.toString();
+        }
+    }

     /*
      *
Index: test/jdbc2/StatementTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/StatementTest.java,v
retrieving revision 1.9
diff -u -r1.9 StatementTest.java
--- test/jdbc2/StatementTest.java    9 Nov 2004 08:55:11 -0000    1.9
+++ test/jdbc2/StatementTest.java    19 Nov 2004 10:17:54 -0000
@@ -39,6 +39,7 @@
     {
         super.tearDown();
         TestUtil.dropTable( con, "test_statement" );
+        TestUtil.dropTable( con, "escapetest" );
         con.close();
     }

@@ -116,10 +117,23 @@
         count = stmt.executeUpdate("insert into escapetest (t) values ({t '00:00:00'})");
         assertEquals(1, count);

-
         ResultSet rs = stmt.executeQuery( "select {fn version()} as version" );
         assertTrue(rs.next());
-
+
+        // check default replacement as-is
+        rs = stmt.executeQuery( "select {fn log(2,3)} as log" );
+        assertTrue(rs.next());
+
+        // check nested and multiple escaped functions
+        rs = stmt.executeQuery( "select {fn version()} as version, {fn log({fn log(3,2)},3)} as log" );
+        assertTrue(rs.next());
+
+        // check concat
+        rs = stmt.executeQuery( "select {fn concat('ab','cd')} as concat" );
+        assertTrue(rs.next());
+        assertEquals(rs.getString(1),"abcd");
+
+
         count = stmt.executeUpdate( "create temp table b (i int)" );
         assertEquals(0, count);

Index: jdbc2/EscapedFunctions.java
===================================================================
RCS file: jdbc2/EscapedFunctions.java
diff -N jdbc2/EscapedFunctions.java
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ jdbc2/EscapedFunctions.java    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,16 @@
+/*-------------------------------------------------------------------------
+*
+* Copyright (c) 2004, PostgreSQL Global Development Group
+*
+* IDENTIFICATION
+*
+*-------------------------------------------------------------------------
+*/
+package org.postgresql.jdbc2;
+
+/**
+ * this class stores supported escaped function
+ */
+public class EscapedFunctions {
+    public final static String CONCAT="concat";
+}


Re: Patch for jdbc escaped functions

От
Kris Jurka
Дата:

On Fri, 19 Nov 2004, Xavier Poinsard wrote:

> Hi,
> I saw that escaped functions were on the TODO list.
> Here is a patch that prepare for adding escaped functions.
> It supports nested escapes.
> As example I have coded the concat function.
> Tests and metadata updates are included.
> If the patch is accepted, I will add more functions.
> Any comments are welcome.
>

This code is not quite right in its determination of function arguments
because it stops checking for literal and identifier markers.
Consider that {fn concat('(', a."(")} should work.

I also don't like the prospect of a giant if/else block that has every
function that must do some kind of mapping/translation.  What about a more
pluggable architecture perhaps along the lines of the following:

public interface StandardFunction {
    public void toSQL(StringBuffer sb, ArrayList args);
}

Then a static HashMap of say lowercase function name -> StandardFunction
implementation can move all of the mapping/translation into a separate
place.  Maybe that's overkill in the opposite direction.  Thoughts?

Kris Jurka

Re: Patch for jdbc escaped functions

От
Xavier Poinsard
Дата:
Kris Jurka wrote:
> This code is not quite right in its determination of function arguments
> because it stops checking for literal and identifier markers.
> Consider that {fn concat('(', a."(")} should work.

Thanks for pointing these defects.
I added this to the parsing and your example to the tests.

>
> I also don't like the prospect of a giant if/else block that has every
> function that must do some kind of mapping/translation.  What about a more
> pluggable architecture perhaps along the lines of the following:
>
> public interface StandardFunction {
>     public void toSQL(StringBuffer sb, ArrayList args);
> }
>
> Then a static HashMap of say lowercase function name -> StandardFunction
> implementation can move all of the mapping/translation into a separate
> place.  Maybe that's overkill in the opposite direction.  Thoughts?

I used reflection to move the translation part to EscapedFunctions class.
Right ?

Index: postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java,v
retrieving revision 1.16
diff -u -r1.16 AbstractJdbc2DatabaseMetaData.java
--- postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java    9 Nov 2004 08:48:31 -0000    1.16
+++ postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java    22 Nov 2004 13:48:22 -0000
@@ -475,7 +475,7 @@
     {
         if (Driver.logDebug)
             Driver.debug("getStringFunctions");
-        return "";
+        return EscapedFunctions.CONCAT;
     }

     public String getSystemFunctions() throws SQLException
Index: postgresql/jdbc2/AbstractJdbc2Statement.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java,v
retrieving revision 1.50
diff -u -r1.50 AbstractJdbc2Statement.java
--- postgresql/jdbc2/AbstractJdbc2Statement.java    17 Nov 2004 02:43:49 -0000    1.50
+++ postgresql/jdbc2/AbstractJdbc2Statement.java    22 Nov 2004 13:48:28 -0000
@@ -11,9 +11,11 @@


 import java.io.*;
+import java.lang.reflect.Method;
 import java.math.*;
 import java.sql.*;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Vector;
 import org.postgresql.Driver;
 import org.postgresql.largeobject.*;
@@ -61,12 +63,12 @@
     // Static variables for parsing SQL when replaceProcessing is true.
     private static final short IN_SQLCODE = 0;
     private static final short IN_STRING = 1;
+    private static final short IN_IDENTIFIER = 6;
     private static final short BACKSLASH = 2;
     private static final short ESC_TIMEDATE = 3;
     private static final short ESC_FUNCTION = 4;
     private static final short ESC_OUTERJOIN = 5;

-
     // Some performance caches
     private StringBuffer sbuf = new StringBuffer(32);

@@ -655,8 +657,9 @@
      *
      * Currently implemented Escape clauses are those mentioned in 11.3
      * in the specification. Basically we look through the sql string for
-     * {d xxx}, {t xxx} or {ts xxx} in non-string sql code. When we find
-     * them, we just strip the escape part leaving only the xxx part.
+     * {d xxx}, {t xxx}, {ts xxx}, {oj xxx} or {fn xxx} in non-string sql
+     * code. When we find them, we just strip the escape part leaving only
+     * the xxx part.
      * So, something like "select * from x where d={d '2001-10-09'}" would
      * return "select * from x where d= '2001-10-09'".
      */
@@ -667,83 +670,172 @@
             // Since escape codes can only appear in SQL CODE, we keep track
             // of if we enter a string or not.
             StringBuffer newsql = new StringBuffer(p_sql.length());
-            short state = IN_SQLCODE;
-
-            int i = -1;
-            int len = p_sql.length();
-            while (++i < len)
-            {
-                char c = p_sql.charAt(i);
-                switch (state)
-                {
-                case IN_SQLCODE:
-                    if (c == '\'')      // start of a string?
-                        state = IN_STRING;
-                    else if (c == '{')     // start of an escape code?
-                        if (i + 1 < len)
+            parseSql(p_sql,0,newsql,false);
+            return newsql.toString();
+        }
+        else
+        {
+            return p_sql;
+        }
+    }
+
+    /**
+     * parse the given sql from index i, appending it to the gven buffer
+     * until we got an ) or end of string or , with the flag stopFirstArg
+     * @param p_sql
+     * @param i
+     * @param newsql
+     * @return
+     */
+    private int parseSql(String p_sql,int i,StringBuffer newsql, boolean stopFirstArg){
+        short state = IN_SQLCODE;
+        // because of the ++i loop
+        i--;
+        int len = p_sql.length();
+        int nestedParenthesis=0;
+        boolean endOfNested=false;
+        while (!endOfNested && ++i < len)
+        {
+            char c = p_sql.charAt(i);
+            switch (state)
+            {
+            case IN_SQLCODE:
+                if (c == '\'')      // start of a string?
+                    state = IN_STRING;
+                else if (c == '"')      // start of a identifer?
+                    state = IN_IDENTIFIER;
+                else if (c=='('){ // begin nested sql
+                    nestedParenthesis++;
+                } else if (c==')'){ // end of nested sql
+                    nestedParenthesis--;
+                    if (nestedParenthesis<0){
+                        endOfNested=true;
+                        break;
+                    }
+                } else if (stopFirstArg && c==',' && nestedParenthesis==0) {
+                    endOfNested=true;
+                    break;
+                }else if (c == '{')     // start of an escape code?
+                    if (i + 1 < len)
+                    {
+                        char next = p_sql.charAt(i + 1);
+                        char nextnext = (i + 2 < len) ? p_sql.charAt(i + 2) : '\0';
+                        if (next == 'd' || next == 'D')
                         {
-                            char next = p_sql.charAt(i + 1);
-                            char nextnext = (i + 2 < len) ? p_sql.charAt(i + 2) : '\0';
-                            if (next == 'd' || next == 'D')
-                            {
-                                state = ESC_TIMEDATE;
-                                i++;
-                                break;
-                            }
-                            else if (next == 't' || next == 'T')
-                            {
-                                state = ESC_TIMEDATE;
-                                i += (nextnext == 's' || nextnext == 'S') ? 2 : 1;
-                                break;
-                            }
-                            else if ( next == 'f' || next == 'F' )
-                            {
-                                state = ESC_FUNCTION;
-                                i += (nextnext == 'n' || nextnext == 'N') ? 2 : 1;
-                                break;
-                            }
-                            else if ( next == 'o' || next == 'O' )
-                            {
-                                state = ESC_OUTERJOIN;
-                                i += (nextnext == 'j' || nextnext == 'J') ? 2 : 1;
-                                break;
-                            }
-
+                            state = ESC_TIMEDATE;
+                            i++;
+                            break;
                         }
-                    newsql.append(c);
-                    break;
-
-                case IN_STRING:
-                    if (c == '\'')       // end of string?
-                        state = IN_SQLCODE;
-                    else if (c == '\\')      // a backslash?
-                        state = BACKSLASH;
+                        else if (next == 't' || next == 'T')
+                        {
+                            state = ESC_TIMEDATE;
+                            i += (nextnext == 's' || nextnext == 'S') ? 2 : 1;
+                            break;
+                        }
+                        else if ( next == 'f' || next == 'F' )
+                        {
+                            state = ESC_FUNCTION;
+                            i += (nextnext == 'n' || nextnext == 'N') ? 2 : 1;
+                            break;
+                        }
+                        else if ( next == 'o' || next == 'O' )
+                        {
+                            state = ESC_OUTERJOIN;
+                            i += (nextnext == 'j' || nextnext == 'J') ? 2 : 1;
+                            break;
+                        }
+                    }
+                newsql.append(c);
+                break;

-                    newsql.append(c);
-                    break;
+            case IN_STRING:
+                if (c == '\'')       // end of string?
+                    state = IN_SQLCODE;
+                else if (c == '\\')      // a backslash?
+                    state = BACKSLASH;

-                case BACKSLASH:
-                    state = IN_STRING;
+                newsql.append(c);
+                break;
+
+            case IN_IDENTIFIER:
+                if (c == '"')       // end of identifier
+                    state = IN_SQLCODE;
+                newsql.append(c);
+                break;

-                    newsql.append(c);
-                    break;
+            case BACKSLASH:
+                state = IN_STRING;

-                case ESC_TIMEDATE:
-                case ESC_FUNCTION:
-                case ESC_OUTERJOIN:
-                    if (c == '}')
-                        state = IN_SQLCODE;    // end of escape code.
-                    else
-                        newsql.append(c);
-                    break;
-                } // end switch
-            }
+                newsql.append(c);
+                break;

-            return newsql.toString();
+            case ESC_FUNCTION:
+                // extract function name
+                String functionName;
+                int posArgs = p_sql.indexOf('(',i);
+                if (posArgs!=-1){
+                    functionName=p_sql.substring(i,posArgs).trim();
+                    // extract arguments
+                    i= posArgs+1;// we start the scan after the first (
+                    StringBuffer args=new StringBuffer();
+                    i = parseSql(p_sql,i,args,false);
+                    // translate the function and parse arguments
+                    newsql.append(escapeFunction(functionName,args.toString()));
+                }
+                // go to the end of the function copying anything found
+                i++;
+                while (i<len && p_sql.charAt(i)!='}')
+                    newsql.append(p_sql.charAt(i));
+                state = IN_SQLCODE; // end of escaped function (or query)
+                break;
+            case ESC_TIMEDATE:
+            case ESC_OUTERJOIN:
+                if (c == '}')
+                    state = IN_SQLCODE;    // end of escape code.
+                else
+                    newsql.append(c);
+                break;
+            } // end switch
         }
-        else
-        {
-            return p_sql;
+        return i;
+    }
+
+    /**
+     * generate sql for escaped functions
+     * @param functionName the escaped function name
+     * @param args the arguments for this functin
+     * @return the right postgreSql sql
+     */
+    protected String escapeFunction(String functionName, String args){
+        // parse function arguments
+        int len = args.length();
+        int i=0;
+        ArrayList parsedArgs = new ArrayList();
+        while (i<len){
+            StringBuffer arg = new StringBuffer();
+            int lastPos=i;
+            i=parseSql(args,i,arg,true);
+            int nestedCount=0;
+            if (lastPos!=i){
+                parsedArgs.add(arg);
+            }
+            i++;
+        }
+        // we can now tranlate escape functions
+        try{
+            Method escapeMethod = EscapedFunctions.class.getDeclaredMethod(functionName.toLowerCase(),new Class [] {
List.class}); 
+            return (String) escapeMethod.invoke(null,new Object[] {parsedArgs});
+        }catch (Exception e){
+            // by default the function name is kept unchanged
+            StringBuffer buf = new StringBuffer();
+            buf.append(functionName).append('(');
+            for (int iArg = 0;iArg<parsedArgs.size();iArg++){
+                buf.append(parsedArgs.get(iArg));
+                if (iArg!=(parsedArgs.size()-1))
+                    buf.append(',');
+            }
+            buf.append(')');
+            return buf.toString();
         }
     }

Index: postgresql/test/jdbc2/StatementTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/StatementTest.java,v
retrieving revision 1.9
diff -u -r1.9 StatementTest.java
--- postgresql/test/jdbc2/StatementTest.java    9 Nov 2004 08:55:11 -0000    1.9
+++ postgresql/test/jdbc2/StatementTest.java    22 Nov 2004 13:48:29 -0000
@@ -32,13 +32,14 @@
         TestUtil.createTempTable(con, "test_statement",
                                  "i int");
         TestUtil.createTempTable(con, "escapetest",
-                                 "ts timestamp, d date, t time");
+                                 "ts timestamp, d date, t time, \")\" varchar(5) ");
     }

     protected void tearDown() throws Exception
     {
         super.tearDown();
         TestUtil.dropTable( con, "test_statement" );
+        TestUtil.dropTable( con, "escapetest" );
         con.close();
     }

@@ -116,10 +117,26 @@
         count = stmt.executeUpdate("insert into escapetest (t) values ({t '00:00:00'})");
         assertEquals(1, count);

-
         ResultSet rs = stmt.executeQuery( "select {fn version()} as version" );
         assertTrue(rs.next());
+
+        // check default replacement as-is
+        rs = stmt.executeQuery( "select {fn log(2,3)} as log" );
+        assertTrue(rs.next());
+
+        // check nested and multiple escaped functions
+        rs = stmt.executeQuery( "select {fn version()} as version, {fn log({fn log(3,2)},3)} as log" );
+        assertTrue(rs.next());
+
+        // check concat
+        rs = stmt.executeQuery( "select {fn concat('ab','cd')} as concat" );
+        assertTrue(rs.next());
+        assertEquals(rs.getString(1),"abcd");

+        // check concat with ( in strings
+        rs = stmt.executeQuery( "select {fn concat(')',escapetest.\")\")} as concat" );
+        assertTrue(rs.next());
+
         count = stmt.executeUpdate( "create temp table b (i int)" );
         assertEquals(0, count);

Index: postgresql/jdbc2/EscapedFunctions.java
===================================================================
RCS file: postgresql/jdbc2/EscapedFunctions.java
diff -N postgresql/jdbc2/EscapedFunctions.java
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ postgresql/jdbc2/EscapedFunctions.java    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,30 @@
+/*-------------------------------------------------------------------------
+*
+* Copyright (c) 2004, PostgreSQL Global Development Group
+*
+* IDENTIFICATION
+*
+*-------------------------------------------------------------------------
+*/
+package org.postgresql.jdbc2;
+
+import java.util.List;
+
+/**
+ * this class stores supported escaped function
+ */
+public class EscapedFunctions {
+    public final static String CONCAT="concat";
+
+    /** concat translation */
+    public static String concat(List parsedArgs){
+        StringBuffer buf = new StringBuffer();
+        buf.append('(');
+        for (int iArg = 0;iArg<parsedArgs.size();iArg++){
+            buf.append(parsedArgs.get(iArg));
+            if (iArg!=(parsedArgs.size()-1))
+                buf.append(" || ");
+        }
+        return buf.append(')').toString();
+    }
+}

Re: Patch for jdbc escaped functions

От
Kris Jurka
Дата:

On Mon, 22 Nov 2004, Xavier Poinsard wrote:

> > I also don't like the prospect of a giant if/else block that has every
> > function that must do some kind of mapping/translation.  What about a more
> > pluggable architecture perhaps along the lines of the following:
> >
> > public interface StandardFunction {
> >     public void toSQL(StringBuffer sb, ArrayList args);
> > }
> >
> > Then a static HashMap of say lowercase function name -> StandardFunction
> > implementation can move all of the mapping/translation into a separate
> > place.  Maybe that's overkill in the opposite direction.  Thoughts?
>
> I used reflection to move the translation part to EscapedFunctions class.
> Right ?
>

I'm not sure why you are using reflection.  The available functions will
be a static list, so I don't see what the purpose of dynamically
inspecting this class is.  Having one class instead of dozens?

Kris Jurka

Re: Patch for jdbc escaped functions

От
Xavier Poinsard
Дата:
Kris Jurka wrote:
>
> On Mon, 22 Nov 2004, Xavier Poinsard wrote:
>
>
>>>I also don't like the prospect of a giant if/else block that has every
>>>function that must do some kind of mapping/translation.  What about a more
>>>pluggable architecture perhaps along the lines of the following:
>>>
>>>public interface StandardFunction {
>>>    public void toSQL(StringBuffer sb, ArrayList args);
>>>}
>>>
>>>Then a static HashMap of say lowercase function name -> StandardFunction
>>>implementation can move all of the mapping/translation into a separate
>>>place.  Maybe that's overkill in the opposite direction.  Thoughts?
>>
>>I used reflection to move the translation part to EscapedFunctions class.
>>Right ?
>>
>
>
> I'm not sure why you are using reflection.  The available functions will
> be a static list, so I don't see what the purpose of dynamically
> inspecting this class is.  Having one class instead of dozens?

Yes, considering the fact that each implementation would only consist of
five lines of codes, it seems overkill to have one class per function.
To avoid inspecting at every call, I could use a static HashMap
containing references to Method objects.

>
> Kris Jurka



Re: Patch for jdbc escaped functions

От
Kris Jurka
Дата:

On Fri, 3 Dec 2004, Xavier Poinsard wrote:

> Here is the (final ?) patch adding escaped functions support.
> - soundex and difference (based on phonetics and I saw nothing like that
> in PostgreSql)

contrib/fuzzystrmatch has something along these lines.  We can't assume
that's installed naturally and dynamically checking for it seems
excessive, but perhaps we could put in the mapping if someone wants to use
it, but just not report it in the DatabaseMetaData function?

> - timestampadd and timestampdiff

Yes, it is unclear what SQL_TSI_* really are.

> Note that the implementations for some functions may cause problem if an
> argument is a prepared statement argument since arguments may be
> duplicated and the order changed, but I don't know if this should be
> implemented for compliance.

This is a serious problem.  Duplicating the argument and erroring out is
bad, and switching argument order and giving incorrect results is worse.
How many functions does this affect.  Remapping could be done, but it
seems like a lot of work.

Another issue is that this doesn't compile with a 1.2 or 1.3 JDK because
of the code in AbstractJdbc2Statement.escapeFunction that calls
Exception.getCause which is a 1.4 addition.

One other minor thing I noticed is that 7.2 servers do not have the
current_database function.  This is a very minor thing, but I'm not sure
what the best way to work around it is because right now the regression
test suite does pass against a 7.2 server and I'd like to keep it that
way.

Kris Jurka

Re: Patch for jdbc escaped functions

От
"Xavier Poinsard"
Дата:
Kris Jurka wrote:
>>- soundex and difference (based on phonetics and I saw nothing like that
>>in PostgreSql)
> contrib/fuzzystrmatch has something along these lines.  We can't assume
> that's installed naturally and dynamically checking for it seems
> excessive, but perhaps we could put in the mapping if someone wants to use
> it, but just not report it in the DatabaseMetaData function?

done.

> This is a serious problem.  Duplicating the argument and erroring out is
> bad, and switching argument order and giving incorrect results is worse.
> How many functions does this affect.  Remapping could be done, but it
> seems like a lot of work.

since only three functions are affected : insert, locate et right
I removed them from the supported list, but left the implementation.
I will document this later.

>
> Another issue is that this doesn't compile with a 1.2 or 1.3 JDK because
> of the code in AbstractJdbc2Statement.escapeFunction that calls
> Exception.getCause which is a 1.4 addition.

Fixed

> One other minor thing I noticed is that 7.2 servers do not have the
> current_database function.  This is a very minor thing, but I'm not sure
> what the best way to work around it is because right now the regression
> test suite does pass against a 7.2 server and I'd like to keep it that
> way.

I only report it as supported for >=7.3 and same for the tests.

Additionnaly I rewrote the messages to reduce translation effort.

Xavier Poinsard.


Вложения

Re: Patch for jdbc escaped functions

От
Kris Jurka
Дата:

On Wed, 8 Dec 2004, Xavier Poinsard wrote:

> [here's a patch to implement standard escaped functions]
>

Applied with some minor modifications.  Notably:

Replace and insert don't work on 7.2 databases.

In junit code when writing assertEquals(x, y).  x should be the expected
value and y is the variable value you are testing.

Kris Jurka