Re: [pgsql-jdbc] dollar-quoted CREATE FUNCTION statement fails - Mailing list pgsql-jdbc

Michael,

I'm only looking at this from a performance standpoint.

1) testing for array.length in a loop is fairly expensive compared to
testing for 0, not a big deal but it adds up.
2) I'm wondering what the cost of a switch is for two cases
( candidly I don't know the answer to this,and it's quite likely that
modern compilers will turn it into an if else anyway.)

Dave

On 1-Nov-06, at 4:47 PM, Michael Paesold wrote:

> Michael Paesold wrote:
>> Kris Jurka wrote:
>>> On Fri, 6 Oct 2006, Michael Paesold wrote:
>>>
>>>> I thought I'd keep you posted on my progress. Here is an updated
>>>> work-in-progress patch implementing dollar-quotes, -- and /* */
>>>> quotes (also with SQL compliant nesting, i.e. /* /* */ */). It
>>>> replaces my last one.
>>>
>>> This looks good to me.  Have you done anything further?
>> Yeah, I have started extracting the code for parsing specific
>> parts of the query (single-quotes, dollar-quotes, comments, etc.)
>> into a separate class so the code can be reused in the V2Query
>> code. I was also looking at supporting standard_conforming_strings
>> as a patch on top of that work.
>
> Attached is a new version of the patch. From my side it looks
> pretty complete now, but I am of course willing to improve it based
> on further comments.
>
> The parsing code is now split into several methods in a new class
> org.postgresql.core.Parser. These methods are used in the v3
> QueryExecutorImpl as well as the V2Query class. Therefore, both
> support dollar-quotes and comments, now.
>
> If you are OK with the approach, I will start coding the support
> for standard_conforming_strings.
>
> Best Regards,
> Michael Paesold
> diff -crN pgjdbc.862ca68b9ea5/org/postgresql/core/Parser.java
> pgjdbc.323e7d139a7b/org/postgresql/core/Parser.java
> *** pgjdbc.862ca68b9ea5/org/postgresql/core/Parser.java    1970-01-01
> 01:00:00.000000000 +0100
> --- pgjdbc.323e7d139a7b/org/postgresql/core/Parser.java    2006-11-01
> 22:41:41.000000000 +0100
> ***************
> *** 0 ****
> --- 1,199 ----
> + /
> *---------------------------------------------------------------------
> ----
> + *
> + * Copyright (c) 2006, PostgreSQL Global Development Group
> + *
> + * IDENTIFICATION
> + *   $PostgreSQL$
> + *
> +
> *---------------------------------------------------------------------
> ----
> + */
> + package org.postgresql.core;
> +
> + /**
> +  * Basic query parser infrastructure.
> +  *
> +  * @author Michael Paesold (mpaesold@gmx.at)
> +  */
> + public class Parser {
> +
> +     /**
> +      * Find the end of the single-quoted string starting at the
> given offset.
> +      */
> +     public static int parseSingleQuotes(final char[] query, int
> offset) {
> +         while (++offset < query.length)
> +         {
> +             switch (query[offset])
> +             {
> +             case '\\':
> +                 ++offset;
> +                 break;
> +             case '\'':
> +                 return offset;
> +             }
> +         }
> +         return query.length;
> +     }
> +
> +     /**
> +      * Find the end of the double-quoted string starting at the
> given offset.
> +      */
> +     public static int parseDoubleQuotes(final char[] query, int
> offset) {
> +         while (++offset < query.length && query[offset] != '"') ;
> +         return offset;
> +     }
> +
> +     /**
> +      * Test if the dollar character (<tt>$</tt>) at the given
> offset starts
> +      * a dollar-quoted string and return the offset of the ending
> dollar
> +      * character.
> +      */
> +     public static int parseDollarQuotes(final char[] query, int
> offset) {
> +         if (offset + 1 < query.length)
> +         {
> +             int endIdx = -1;
> +             if (query[offset + 1] == '$')
> +                 endIdx = offset + 1;
> +             else if (isDollarQuoteStartChar(query[offset + 1]))
> +             {
> +                 for (int d = offset + 2; d < query.length; ++d)
> +                 {
> +                     if (query[d] == '$')
> +                     {
> +                         endIdx = d;
> +                         break;
> +                     }
> +                     else if (!isDollarQuoteContChar(query[d]))
> +                         break;
> +                 }
> +             }
> +             if (endIdx > 0)
> +             {
> +                 // found; note: tag includes start and end $
> character
> +                 int tagIdx = offset, tagLen = endIdx - offset + 1;
> +                 offset = endIdx; // loop continues at endIdx + 1
> +                 for (++offset; offset < query.length; ++offset)
> +                 {
> +                     if (query[offset] == '$' &&
> +                         subArraysEqual(query, tagIdx, offset,
> tagLen))
> +                     {
> +                         offset += tagLen - 1;
> +                         break;
> +                     }
> +                 }
> +             }
> +         }
> +         return offset;
> +     }
> +
> +     /**
> +      * Test if the <tt>-</tt> character at <tt>offset</tt> starts a
> +      * <tt>--</tt> style line comment, and return the position of
> the first
> +      * <tt>\r</tt> or <tt>\n</tt> character.
> +      */
> +     public static int parseLineComment(final char[] query, int
> offset) {
> +         if (offset + 1 < query.length && query[offset + 1] == '-')
> +         {
> +             while (++offset < query.length)
> +             {
> +                 if (query[offset] == '\r' || query[offset] == '\n')
> +                     break;
> +             }
> +         }
> +         return offset;
> +     }
> +
> +     /**
> +      * Test if the <tt>/</tt> character at <tt>offset</tt> starts
> a block
> +      * comment, and return the position of the last <tt>/</tt>
> character.
> +      */
> +     public static int parseBlockComment(final char[] query, int
> offset) {
> +         if (offset + 1 < query.length && query[offset + 1] == '*')
> +         {
> +             // /* /* */ */ nest, according to SQL spec
> +             int level = 1;
> +             for (offset += 2; offset < query.length; ++offset)
> +             {
> +                 switch (query[offset-1]) {
> +                 case '*':
> +                     if (query[offset] == '/')
> +                     {
> +                         --level;
> +                         ++offset; // don't parse / in */* twice
> +                     }
> +                     break;
> +                 case '/':
> +                     if (query[offset] == '*')
> +                     {
> +                         ++level;
> +                         ++offset; // don't parse * in /*/ twice
> +                     }
> +                     break;
> +                 }
> +                 if (level == 0)
> +                 {
> +                     --offset; // reset position to last '/' char
> +                     break;
> +                 }
> +             }
> +         }
> +         return offset;
> +     }
> +
> +     /**
> +      * Checks if a character is valid as the start of a dollar
> quoting tag.
> +      *
> +      * @param c the character to check
> +      * @return true if valid as first character of a dollar
> quoting tag; false if not
> +      */
> +     public static boolean isDollarQuoteStartChar(char c) {
> +         /*
> +          * The allowed dollar quote start and continuation
> characters
> +          * must stay in sync with what the backend defines in
> +          * pgsql/src/backend/parser/scan.l
> +          */
> +         return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')
> +                 || c == '_' || c > 127;
> +     }
> +
> +     /**
> +      * Checks if a character is valid as the second or latter
> character of a
> +      * dollar quoting tag.
> +      *
> +      * @param c the character to check
> +      * @return true if valid as second or later character of a
> dollar quoting tag;
> +      *         false if not
> +      */
> +     public static boolean isDollarQuoteContChar(char c) {
> +         return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')
> +                 || c == '_' || c > 127
> +                 || (c >= '0' && c <= '9');
> +     }
> +
> +     /**
> +      * Compares two sub-arrays of the given character array for
> equalness.
> +      * If the length is zero, the result is true, if and only if
> the offsets
> +      * are within the bounds of the array.
> +      *
> +      * @param arr  a char array
> +      * @param offA first sub-array start offset
> +      * @param offB second sub-array start offset
> +      * @param len  length of the sub arrays to compare
> +      * @return     true if the sub-arrays are equal; false if not
> +      */
> +     private static boolean subArraysEqual(final char[] arr,
> +                                           final int offA, final
> int offB,
> +                                           final int len) {
> +         if (offA < 0 || offB < 0
> +                 || offA >= arr.length || offB >= arr.length
> +                 || offA + len > arr.length || offB + len >
> arr.length)
> +             return false;
> +
> +         for (int i = 0; i < len; ++i)
> +         {
> +             if (arr[offA + i] != arr[offB + i])
> +                 return false;
> +         }
> +
> +         return true;
> +     }
> + }
> diff -crN pgjdbc.862ca68b9ea5/org/postgresql/core/v2/V2Query.java
> pgjdbc.323e7d139a7b/org/postgresql/core/v2/V2Query.java
> *** pgjdbc.862ca68b9ea5/org/postgresql/core/v2/V2Query.java
> 2006-11-01 22:41:41.000000000 +0100
> --- pgjdbc.323e7d139a7b/org/postgresql/core/v2/V2Query.java
> 2006-11-01 22:41:41.000000000 +0100
> ***************
> *** 29,62 ****
>           Vector v = new Vector();
>           int lastParmEnd = 0;
>
> !         boolean inSingleQuotes = false;
> !         boolean inDoubleQuotes = false;
>
> !         for (int i = 0; i < query.length(); ++i)
>           {
> !             char c = query.charAt(i);
> !
> !             switch (c)
>               {
> !             case '\\':
> !                 if (inSingleQuotes)
> !                     ++i; // Skip one character.
>                   break;
>
> !             case '\'':
> !                 inSingleQuotes = !inDoubleQuotes && !inSingleQuotes;
>                   break;
>
> !             case '"':
> !                 inDoubleQuotes = !inSingleQuotes && !inDoubleQuotes;
>                   break;
>
>               case '?':
> !                 if (!inSingleQuotes && !inDoubleQuotes)
> !                 {
> !                     v.addElement(query.substring (lastParmEnd, i));
> !                     lastParmEnd = i + 1;
> !                 }
>                   break;
>
>               default:
> --- 29,63 ----
>           Vector v = new Vector();
>           int lastParmEnd = 0;
>
> !         char []aChars = query.toCharArray();
>
> !         for (int i = 0; i < aChars.length; ++i)
>           {
> !             switch (aChars[i])
>               {
> !             case '\'': // single-quotes
> !                 i = Parser.parseSingleQuotes(aChars, i);
> !                 break;
> !
> !             case '"': // double-quotes
> !                 i = Parser.parseDoubleQuotes(aChars, i);
>                   break;
>
> !             case '-': // possibly -- style comment
> !                 i = Parser.parseLineComment(aChars, i);
>                   break;
>
> !             case '/': // possibly /* */ style comment
> !                 i = Parser.parseBlockComment(aChars, i);
> !                 break;
> !
> !             case '$': // possibly dollar quote start
> !                 i = Parser.parseDollarQuotes(aChars, i);
>                   break;
>
>               case '?':
> !                 v.addElement(query.substring (lastParmEnd, i));
> !                 lastParmEnd = i + 1;
>                   break;
>
>               default:
> diff -crN pgjdbc.862ca68b9ea5/org/postgresql/core/v3/
> QueryExecutorImpl.java pgjdbc.323e7d139a7b/org/postgresql/core/v3/
> QueryExecutorImpl.java
> *** pgjdbc.862ca68b9ea5/org/postgresql/core/v3/
> QueryExecutorImpl.java    2006-11-01 22:41:41.000000000 +0100
> --- pgjdbc.323e7d139a7b/org/postgresql/core/v3/
> QueryExecutorImpl.java    2006-11-01 22:41:41.000000000 +0100
> ***************
> *** 63,116 ****
>           ArrayList statementList = new ArrayList();
>           ArrayList fragmentList = new ArrayList(15);
>
> -         boolean inQuotes = false;
>           int fragmentStart = 0;
> -
> -         boolean inSingleQuotes = false;
> -         boolean inDoubleQuotes = false;
>           int inParen = 0;
> !
>           char []aChars = query.toCharArray();
> !
>           for (int i = 0; i < aChars.length; ++i)
>           {
> !             char c = aChars[i];
> !
> !             switch (c)
>               {
> !             case '\\':
> !                 if (inSingleQuotes)
> !                     ++i; // Skip one character.
>                   break;
>
> !             case '\'':
> !                 inSingleQuotes = !inDoubleQuotes && !inSingleQuotes;
>                   break;
>
> !             case '"':
> !                 inDoubleQuotes = !inSingleQuotes && !inDoubleQuotes;
>                   break;
>
> !             case '?':
> !                 if (withParameters && !inSingleQuotes && !
> inDoubleQuotes)
> !                 {
> !                     fragmentList.add(query.substring
> (fragmentStart, i));
> !                     fragmentStart = i + 1;
> !                 }
>                   break;
>
>               case '(':
> !                 if (!inSingleQuotes && !inDoubleQuotes)
> !                         inParen++;
>                   break;
>
>               case ')':
> !                 if (!inSingleQuotes && !inDoubleQuotes)
> !                         inParen--;
>                   break;
>
>               case ';':
> !                 if (!inSingleQuotes && !inDoubleQuotes && inParen
> == 0)
>                   {
>                       fragmentList.add(query.substring
> (fragmentStart, i));
>                       fragmentStart = i + 1;
> --- 63,115 ----
>           ArrayList statementList = new ArrayList();
>           ArrayList fragmentList = new ArrayList(15);
>
>           int fragmentStart = 0;
>           int inParen = 0;
> !
>           char []aChars = query.toCharArray();
> !
>           for (int i = 0; i < aChars.length; ++i)
>           {
> !             switch (aChars[i])
>               {
> !             case '\'': // single-quotes
> !                 i = Parser.parseSingleQuotes(aChars, i);
>                   break;
>
> !             case '"': // double-quotes
> !                 i = Parser.parseDoubleQuotes(aChars, i);
>                   break;
>
> !             case '-': // possibly -- style comment
> !                 i = Parser.parseLineComment(aChars, i);
>                   break;
>
> !             case '/': // possibly /* */ style comment
> !                 i = Parser.parseBlockComment(aChars, i);
> !                 break;
> !
> !             case '$': // possibly dollar quote start
> !                 i = Parser.parseDollarQuotes(aChars, i);
>                   break;
>
>               case '(':
> !                 inParen++;
>                   break;
>
>               case ')':
> !                 inParen--;
> !                 break;
> !
> !             case '?':
> !                 if (withParameters)
> !                 {
> !                     fragmentList.add(query.substring
> (fragmentStart, i));
> !                     fragmentStart = i + 1;
> !                 }
>                   break;
>
>               case ';':
> !                 if (inParen == 0)
>                   {
>                       fragmentList.add(query.substring
> (fragmentStart, i));
>                       fragmentStart = i + 1;
> diff -crN pgjdbc.862ca68b9ea5/org/postgresql/test/jdbc2/
> PreparedStatementTest.java pgjdbc.323e7d139a7b/org/postgresql/test/
> jdbc2/PreparedStatementTest.java
> *** pgjdbc.862ca68b9ea5/org/postgresql/test/jdbc2/
> PreparedStatementTest.java    2006-11-01 22:41:41.000000000 +0100
> --- pgjdbc.323e7d139a7b/org/postgresql/test/jdbc2/
> PreparedStatementTest.java    2006-11-01 22:41:41.000000000 +0100
> ***************
> *** 304,309 ****
> --- 304,374 ----
>               pstmt.close();
>           }
>       }
> +
> +     public void testDollarQuotes() throws SQLException {
> +         // dollar-quotes are supported in the backend since
> version 8.0
> +         if (!TestUtil.haveMinimumServerVersion(conn, "8.0"))
> +             return;
> +
> +         PreparedStatement st;
> +         ResultSet rs;
> +
> +         st = conn.prepareStatement("SELECT $$;$$ WHERE $x$?$x$=$_0
> $?$_0$ AND $$?$$=?");
> +         st.setString(1, "?");
> +         rs = st.executeQuery();
> +         assertTrue(rs.next());
> +         assertEquals(";", rs.getString(1));
> +         assertFalse(rs.next());
> +         st.close();
> +
> +         st = conn.prepareStatement(
> +                   "SELECT $__$;$__$ WHERE ''''=$q_1$'$q_1$ AND
> ';'=?;"
> +                 + "SELECT $x$$a$;$x $a$$x$ WHERE $$;$$=? OR ''=$c
> $c$;$c$;"
> +                 + "SELECT ?");
> +         st.setString(1, ";");
> +         st.setString(2, ";");
> +         st.setString(3, "$a$ $a$");
> +
> +         assertTrue(st.execute());
> +         rs = st.getResultSet();
> +         assertTrue(rs.next());
> +         assertEquals(";", rs.getString(1));
> +         assertFalse(rs.next());
> +
> +         assertTrue(st.getMoreResults());
> +         rs = st.getResultSet();
> +         assertTrue(rs.next());
> +         assertEquals("$a$;$x $a$", rs.getString(1));
> +         assertFalse(rs.next());
> +
> +         assertTrue(st.getMoreResults());
> +         rs = st.getResultSet();
> +         assertTrue(rs.next());
> +         assertEquals("$a$ $a$", rs.getString(1));
> +         assertFalse(rs.next());
> +         st.close();
> +     }
> +
> +     public void testComments() throws SQLException {
> +         PreparedStatement st;
> +         ResultSet rs;
> +
> +         st = conn.prepareStatement("SELECT /*?*/ /*/*/*/**/*/*/*/
> 1;SELECT ?;--SELECT ?");
> +         st.setString(1, "a");
> +         assertTrue(st.execute());
> +         assertTrue(st.getMoreResults());
> +         assertFalse(st.getMoreResults());
> +         st.close();
> +
> +         st = conn.prepareStatement("SELECT /**/'?'/*/**/*/ WHERE
> '?'=/*/*/*?*/*/*/--?\n?");
> +         st.setString(1, "?");
> +         rs = st.executeQuery();
> +         assertTrue(rs.next());
> +         assertEquals("?", rs.getString(1));
> +         assertFalse(rs.next());
> +         st.close();
> +     }
> +
>       public void testDouble() throws SQLException
>       {
>           PreparedStatement pstmt = conn.prepareStatement("CREATE
> TEMP TABLE double_tab (max_double float, min_double float,
> null_value float)");
> diff -crN pgjdbc.862ca68b9ea5/org/postgresql/test/jdbc2/
> StatementTest.java pgjdbc.323e7d139a7b/org/postgresql/test/jdbc2/
> StatementTest.java
> *** pgjdbc.862ca68b9ea5/org/postgresql/test/jdbc2/
> StatementTest.java    2006-11-01 22:41:41.000000000 +0100
> --- pgjdbc.323e7d139a7b/org/postgresql/test/jdbc2/
> StatementTest.java    2006-11-01 22:41:41.000000000 +0100
> ***************
> *** 390,395 ****
> --- 390,442 ----
>           assertTrue(!rs.next());
>       }
>
> +     public void testParsingDollarQuotes() throws SQLException
> +     {
> +         // dollar-quotes are supported in the backend since
> version 8.0
> +         if (!TestUtil.haveMinimumServerVersion(con, "8.0"))
> +             return;
> +
> +         Statement st = con.createStatement();
> +         ResultSet rs;
> +
> +         rs = st.executeQuery("SELECT '$a$ ; $a$'");
> +         assertTrue(rs.next());
> +         assertEquals("$a$ ; $a$", rs.getObject(1));
> +         rs.close();
> +
> +         rs = st.executeQuery("SELECT $$;$$");
> +         assertTrue(rs.next());
> +         assertEquals(";", rs.getObject(1));
> +         rs.close();
> +
> +         rs = st.executeQuery("SELECT $OR$$a$'$b$a$$OR$ WHERE '$a
> $''$b$a$'=$OR$$a$'$b$a$$OR$OR ';'=''");
> +         assertTrue(rs.next());
> +         assertEquals("$a$'$b$a$", rs.getObject(1));
> +         assertFalse(rs.next());
> +         rs.close();
> +
> +         rs = st.executeQuery("SELECT $B$;$b$B$");
> +         assertTrue(rs.next());
> +         assertEquals(";$b", rs.getObject(1));
> +         rs.close();
> +
> +         rs = st.executeQuery("SELECT $c$c$;$c$");
> +         assertTrue(rs.next());
> +         assertEquals("c$;", rs.getObject(1));
> +         rs.close();
> +
> +         rs = st.executeQuery("SELECT $A0$;$A0$ WHERE ''=$t$t$t$
> OR ';$t$'=';$t$'");
> +         assertTrue(rs.next());
> +         assertEquals(";", rs.getObject(1));
> +         assertFalse(rs.next());
> +         rs.close();
> +
> +         st.executeQuery("SELECT /* */$$;$$/**//*;*/").close();
> +         st.executeQuery("SELECT /* */--;\n$$a$$/**/--\n--;
> \n").close();
> +
> +         st.close();
> +     }
> +
>       public void testUnbalancedParensParseError() throws SQLException
>       {
>           Statement stmt = con.createStatement();