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

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(at)gmx(dot)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();