Thread: Add casts & related fixes

Add casts & related fixes

From
Kim Ho
Date:
Problem:
 - This improves the handling of numbers by adding the casts required by
the backend.  As a result, many JDBC compliance tests now pass.

Fix:
 - Use m_bindTypes to add a '::<cast>' when we send the query in
QueryExecutor.
 - Changes a lot of the casts, especially for numbers, to ensure that
all the proper types are being cast.


Additional patches that are related:
SetObjectNumbers:
 - Due to the addcasts patch, this one deals with the fact that
different numbers are dealt with differently now.

SetObjectBinary:
 - Calls the setByte[] instead of setObject (which used to default to
string I believe). If not, attempt to cast the string to byte[].

Feedback and suggestions welcome.

Cheers,

Kim

Attachment

Re: Add casts & related fixes

From
Barry Lind
Date:
Kim,

Can you provide some examples of the type of problem you are trying to
solve here.  I am wondering if there is a different way to solve it than
adding these casts.  This patch will break backward compatibility with
some existing code.  That may be necessary, but I would like to avoid it
if possible.

Specifically statements along the lines of:

select foo from ?

will no longer work as this would now get translated into:

select foo from table::text

There are also other subtle ways that adding the cast will cause
problems.  We added casts in the past for int8 and int2 binds in order
to get them to use indexes, but ended up removing the casts because of
problems with breaking existing apps.

thanks,
--Barry


Kim Ho wrote:
> Problem:
>  - This improves the handling of numbers by adding the casts required by
> the backend.  As a result, many JDBC compliance tests now pass.
>
> Fix:
>  - Use m_bindTypes to add a '::<cast>' when we send the query in
> QueryExecutor.
>  - Changes a lot of the casts, especially for numbers, to ensure that
> all the proper types are being cast.
>
>
> Additional patches that are related:
> SetObjectNumbers:
>  - Due to the addcasts patch, this one deals with the fact that
> different numbers are dealt with differently now.
>
> SetObjectBinary:
>  - Calls the setByte[] instead of setObject (which used to default to
> string I believe). If not, attempt to cast the string to byte[].
>
> Feedback and suggestions welcome.
>
> Cheers,
>
> Kim
>
>
> ------------------------------------------------------------------------
>
> Index: org/postgresql/core/QueryExecutor.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/core/QueryExecutor.java,v
> retrieving revision 1.22
> diff -c -p -r1.22 QueryExecutor.java
> *** org/postgresql/core/QueryExecutor.java    29 May 2003 03:21:32 -0000    1.22
> --- org/postgresql/core/QueryExecutor.java    8 Jul 2003 14:19:21 -0000
> *************** public class QueryExecutor
> *** 24,35 ****
> --- 24,37 ----
>       //creates a new one for the results of the query
>       public static BaseResultSet execute(String[] p_sqlFrags,
>                       Object[] p_binds,
> +                     String[] p_bindTypes,
>                       BaseStatement statement)
>       throws SQLException
>       {
>           QueryExecutor qe = new QueryExecutor();
>           qe.m_sqlFrags = p_sqlFrags;
>           qe.m_binds = p_binds;
> +         qe.m_bindTypes = p_bindTypes;
>           qe.statement = statement;
>           if (statement != null)
>               qe.maxRows = statement.getMaxRows();
> *************** public class QueryExecutor
> *** 47,58 ****
> --- 49,62 ----
>       //more results are fetched
>       public static void execute(String[] p_sqlFrags,
>                       Object[] p_binds,
> +                     String[] p_bindTypes,
>                       BaseResultSet rs)
>       throws SQLException
>       {
>           QueryExecutor qe = new QueryExecutor();
>           qe.m_sqlFrags = p_sqlFrags;
>           qe.m_binds = p_binds;
> +         qe.m_bindTypes = p_bindTypes;
>           qe.rs = rs;
>           qe.statement = qe.rs.getPGStatement();
>           if (qe.statement != null)
> *************** public class QueryExecutor
> *** 73,78 ****
> --- 77,83 ----
>
>          private String[] m_sqlFrags;
>       private Object[] m_binds;
> +     private String[] m_bindTypes;
>       private BaseStatement statement;
>       private BaseResultSet rs;
>
> *************** public class QueryExecutor
> *** 325,330 ****
> --- 330,341 ----
>                   l_parts[j] = l_encoding.encode(m_binds[i].toString());
>                   l_msgSize += l_parts[j].length;
>                   j++;
> +                 if (m_bindTypes!=null && !(m_bindTypes[i].equals("boolean")) && m_binds[i].toString().length() > 0)
> +                 {
> +                     pgStream.Send(connection.getEncoding().encode("::"+m_bindTypes[i]));
> +                     l_msgSize += l_parts[j].length;
> +                     j++;
> +                 }
>               }
>               l_parts[j] = l_encoding.encode(m_sqlFrags[m_binds.length]);
>               l_msgSize += l_parts[j].length;
> *************** public class QueryExecutor
> *** 358,363 ****
> --- 369,376 ----
>               {
>                   pgStream.Send(connection.getEncoding().encode(m_sqlFrags[i]));
>                   pgStream.Send(connection.getEncoding().encode(m_binds[i].toString()));
> +                 if (m_bindTypes!=null && !(m_bindTypes[i].equals("boolean")) && m_binds[i].toString().length() > 0)
> +                     pgStream.Send(connection.getEncoding().encode("::"+m_bindTypes[i]));
>               }
>
>               pgStream.Send(connection.getEncoding().encode(m_sqlFrags[m_binds.length]));
> Index: org/postgresql/jdbc1/AbstractJdbc1Connection.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Connection.java,v
> retrieving revision 1.21
> diff -c -p -r1.21 AbstractJdbc1Connection.java
> *** org/postgresql/jdbc1/AbstractJdbc1Connection.java    30 Jun 2003 21:10:55 -0000    1.21
> --- org/postgresql/jdbc1/AbstractJdbc1Connection.java    8 Jul 2003 14:19:22 -0000
> *************** public abstract class AbstractJdbc1Conne
> *** 863,869 ****
>           final Object[] nullarr = new Object[0];
>           BaseStatement stat = (BaseStatement) createStatement();
>           return QueryExecutor.execute(new String[] { s },
> !                                      nullarr,
>                                        stat);
>       }
>
> --- 863,870 ----
>           final Object[] nullarr = new Object[0];
>           BaseStatement stat = (BaseStatement) createStatement();
>           return QueryExecutor.execute(new String[] { s },
> !                                      nullarr,
> !                                      null,
>                                        stat);
>       }
>
> Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> retrieving revision 1.13
> diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
> --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    8 Jul 2003 14:19:23 -0000
> *************** public abstract class AbstractJdbc1Resul
> *** 134,139 ****
> --- 134,140 ----
>               sql[0] = "FETCH FORWARD " + fetchSize + " FROM " + cursorName;
>               QueryExecutor.execute(sql,
>                                     binds,
> +                                   null,
>                                     this);
>
>               // Test the new rows array.
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.26
> diff -c -p -r1.26 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java    30 Jun 2003 21:10:55 -0000    1.26
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java    8 Jul 2003 14:19:25 -0000
> *************** public abstract class AbstractJdbc1State
> *** 436,441 ****
> --- 436,442 ----
>           // New in 7.1, pass Statement so that ExecSQL can customise to it
>           result = QueryExecutor.execute(m_sqlFragments,
>                                                  m_binds,
> +                            m_bindTypes,
>                                                  this);
>
>           //If we are executing a callable statement function set the return data
> Index: org/postgresql/jdbc1/Jdbc1RefCursorResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/Jdbc1RefCursorResultSet.java,v
> retrieving revision 1.1
> diff -c -p -r1.1 Jdbc1RefCursorResultSet.java
> *** org/postgresql/jdbc1/Jdbc1RefCursorResultSet.java    3 May 2003 20:40:45 -0000    1.1
> --- org/postgresql/jdbc1/Jdbc1RefCursorResultSet.java    8 Jul 2003 14:19:25 -0000
> *************** public class Jdbc1RefCursorResultSet ext
> *** 37,43 ****
>                           return super.next();
>                   // Initialize this res set with the rows from the cursor.
>                   String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], this);
>                   isInitialized = true;
>                   return super.next();
>           }
> --- 37,43 ----
>                           return super.next();
>                   // Initialize this res set with the rows from the cursor.
>                   String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], null, this);
>                   isInitialized = true;
>                   return super.next();
>           }
> Index: org/postgresql/jdbc2/Jdbc2RefCursorResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc2/Jdbc2RefCursorResultSet.java,v
> retrieving revision 1.1
> diff -c -p -r1.1 Jdbc2RefCursorResultSet.java
> *** org/postgresql/jdbc2/Jdbc2RefCursorResultSet.java    3 May 2003 20:40:45 -0000    1.1
> --- org/postgresql/jdbc2/Jdbc2RefCursorResultSet.java    8 Jul 2003 14:19:25 -0000
> *************** public class Jdbc2RefCursorResultSet ext
> *** 36,42 ****
>                           return super.next();
>                   // Initialize this res set with the rows from the cursor.
>                   String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], this);
>                   isInitialized = true;
>                   return super.next();
>           }
> --- 36,42 ----
>                           return super.next();
>                   // Initialize this res set with the rows from the cursor.
>                   String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], null, this);
>                   isInitialized = true;
>                   return super.next();
>           }
> Index: org/postgresql/jdbc3/Jdbc3RefCursorResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc3/Jdbc3RefCursorResultSet.java,v
> retrieving revision 1.2
> diff -c -p -r1.2 Jdbc3RefCursorResultSet.java
> *** org/postgresql/jdbc3/Jdbc3RefCursorResultSet.java    29 May 2003 04:39:50 -0000    1.2
> --- org/postgresql/jdbc3/Jdbc3RefCursorResultSet.java    8 Jul 2003 14:19:25 -0000
> *************** public class Jdbc3RefCursorResultSet ext
> *** 39,45 ****
>               return super.next();
>           // Initialize this res set with the rows from the cursor.
>           String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], this);
>           isInitialized = true;
>           return super.next();
>       }
> --- 39,45 ----
>               return super.next();
>           // Initialize this res set with the rows from the cursor.
>           String[] toExec = { "FETCH ALL IN \"" + refCursorHandle + "\";" };
> !                 QueryExecutor.execute(toExec, new String[0], null, this);
>           isInitialized = true;
>           return super.next();
>       }
>
>
> ------------------------------------------------------------------------
>
> ? cloudscape.LOG
> ? temp.diff
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.26
> diff -c -p -c -p -r1.26 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java    30 Jun 2003 21:10:55 -0000    1.26
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java    3 Jul 2003 16:22:36 -0000
> *************** public abstract class AbstractJdbc1State
> *** 1464,1486 ****
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
> !                 if (x instanceof Boolean)
> !                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> !                 else
> !                     bind(parameterIndex, x.toString(), PG_INTEGER);
>                   break;
>               case Types.TINYINT:
>               case Types.SMALLINT:
>               case Types.BIGINT:
>               case Types.REAL:
>               case Types.FLOAT:
>               case Types.DOUBLE:
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 if (x instanceof Boolean)
> !                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> !                 else
> !                     bind(parameterIndex, x.toString(), PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> --- 1464,1491 ----
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
> !                 x = removeRadix(x, Types.INTEGER);
> !                 bindNumber(parameterIndex,x,PG_INTEGER);
>                   break;
>               case Types.TINYINT:
>               case Types.SMALLINT:
> +                 x = removeRadix(x,Types.SMALLINT);
> +                 bindNumber(parameterIndex,x,PG_INT2);
> +                 break;
>               case Types.BIGINT:
> +                 x = removeRadix(x,Types.BIGINT);
> +                 bindNumber(parameterIndex,x,PG_INT8);
> +                 break;
>               case Types.REAL:
>               case Types.FLOAT:
> +                 bindNumber(parameterIndex,x,PG_FLOAT);
> +                 break;
>               case Types.DOUBLE:
> +                 bindNumber(parameterIndex,x,PG_DOUBLE);
> +                 break;
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 bindNumber(parameterIndex,x,PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> *************** public abstract class AbstractJdbc1State
> *** 2031,2036 ****
> --- 2036,2068 ----
>       {
>           return m_useServerPrepare;
>       }
> +
> +     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> +     {
> +         if (x instanceof Boolean)
> +             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> +         else
> +             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> +     }
> +
> +
> +     private Object removeRadix(Object x, int sqlType)
> +     {
> +         if (x.toString().indexOf(".")>0)
> +         {
> +             switch (sqlType)
> +             {
> +                 case Types.BIGINT:
> +                     x = String.valueOf(new Double(x.toString()).longValue());
> +                     break;
> +                 default:
> +                     x = String.valueOf(new Float(x.toString()).intValue());
> +                     break;
> +             }
> +         }
> +         return x;
> +     }
> +
>
>
>       private static final String PG_TEXT = "text";
>
>
> ------------------------------------------------------------------------
>
> Index: org/postgresql/errors.properties
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/errors.properties,v
> retrieving revision 1.21
> diff -c -p -c -p -r1.21 errors.properties
> *** org/postgresql/errors.properties    30 Jun 2003 16:38:30 -0000    1.21
> --- org/postgresql/errors.properties    3 Jul 2003 18:55:35 -0000
> *************** postgresql.call.wrongrtntype:A CallableS
> *** 100,102 ****
> --- 100,103 ----
>   postgresql.input.fetch.gt0:Fetch size must be a value greater than or equal to 0.
>   postgresql.input.query.gt0:Query Timeout must be a value greater than or equal to 0.
>   postgresql.input.rows.gt0:Maximum number of rows must be a value greater than or equal to 0.
> + postgresql.badcast.bytes:setObject cannot be used with this Object type and
java.sql.Types.BINARY/VARBINARY/LONGVARBINARY.
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.26
> diff -c -p -c -p -r1.26 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java    30 Jun 2003 21:10:55 -0000    1.26
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java    3 Jul 2003 18:55:36 -0000
> *************** public abstract class AbstractJdbc1State
> *** 1513,1519 ****
>               case Types.BINARY:
>               case Types.VARBINARY:
>               case Types.LONGVARBINARY:
> !                 setObject(parameterIndex, x);
>                   break;
>               case Types.OTHER:
>                   setString(parameterIndex, ((PGobject)x).getValue(), PG_TEXT);
> --- 1513,1528 ----
>               case Types.BINARY:
>               case Types.VARBINARY:
>               case Types.LONGVARBINARY:
> !                 if (x instanceof byte[])
> !                 {
> !                     setBytes(parameterIndex,(byte[])x);
> !                 }
> !                 else if (x instanceof String)
> !                 {
> !                     setBytes(parameterIndex,((String)x).getBytes());
> !                 }
> !                 else
> !                     throw new PSQLException("postgresql.badcast.bytes");
>                   break;
>               case Types.OTHER:
>                   setString(parameterIndex, ((PGobject)x).getValue(), PG_TEXT);
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org




Re: Add casts & related fixes

From
Kim Ho
Date:
Hm, you're right. Maybe there's a better way to do this then. Perhaps
it'll be something I can change in the registerOutParam patch to get it
to work.

I'll have to run CTS to find a specific case.

Cheers,

Kim

On Tue, 2003-07-08 at 11:46, Barry Lind wrote:
> Kim,
>
> Can you provide some examples of the type of problem you are trying to
> solve here.  I am wondering if there is a different way to solve it than
> adding these casts.  This patch will break backward compatibility with
> some existing code.  That may be necessary, but I would like to avoid it
> if possible.
>
> Specifically statements along the lines of:
>
> select foo from ?
>
> will no longer work as this would now get translated into:
>
> select foo from table::text
>
> There are also other subtle ways that adding the cast will cause
> problems.  We added casts in the past for int8 and int2 binds in order
> to get them to use indexes, but ended up removing the casts because of
> problems with breaking existing apps.
>
> thanks,
> --Barry
>