Thread: Attempt to clean up ExecSql() in JDBC

Attempt to clean up ExecSql() in JDBC

From
Anders Bengtsson
Date:
Hi,

Attached is my attempt to clean up the horrors of the ExecSQL() method in
the JDBC driver.

I've done this by extracting it into a new method object called
QueryExecutor (should go into org/postgresql/core/) and then taking it
apart into different methods in that class.

A short summary:

* Extracted ExecSQL() from Connection into a method object called
  QueryExecutor.

* Moved ReceiveFields() from Connection to QueryExecutor.

* Extracted parts of the original ExecSQL() method body into smaller
  methods on QueryExecutor.

* Bug fix: The instance variable "pid" in Connection was used in two
  places with different meaning. Both were probably in dead code, but it's
  fixed anyway.

/Anders


PS.: If anyone has any idea what the variable names "fqp" and "hfr" stand
for, please tell me! :)

_____________________________________________________________________
A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
Stockholm, Sweden

Attachment

Re: [PATCHES] Attempt to clean up ExecSql() in JDBC

From
Barry Lind
Date:
I looked at the patch and it looks fine.  As for what fqp and hfr stand
for I don't have a clue.  I was looking through the fe/be protocol
documentation and think I might have a clue about what they are being
used for.

thanks,
--Barry

Anders Bengtsson wrote:
> Hi,
>
> Attached is my attempt to clean up the horrors of the ExecSQL() method in
> the JDBC driver.
>
> I've done this by extracting it into a new method object called
> QueryExecutor (should go into org/postgresql/core/) and then taking it
> apart into different methods in that class.
>
> A short summary:
>
> * Extracted ExecSQL() from Connection into a method object called
>   QueryExecutor.
>
> * Moved ReceiveFields() from Connection to QueryExecutor.
>
> * Extracted parts of the original ExecSQL() method body into smaller
>   methods on QueryExecutor.
>
> * Bug fix: The instance variable "pid" in Connection was used in two
>   places with different meaning. Both were probably in dead code, but it's
>   fixed anyway.
>
> /Anders
>
>
> PS.: If anyone has any idea what the variable names "fqp" and "hfr" stand
> for, please tell me! :)
>
> _____________________________________________________________________
> A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
> Stockholm, Sweden
>
>
> ------------------------------------------------------------------------
>
> Index: src/interfaces/jdbc/org/postgresql/Connection.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/Connection.java,v
> retrieving revision 1.26
> diff -c -r1.26 Connection.java
> *** src/interfaces/jdbc/org/postgresql/Connection.java    2001/08/24 16:50:12    1.26
> --- src/interfaces/jdbc/org/postgresql/Connection.java    2001/08/26 18:33:48
> ***************
> *** 8,14 ****
>   import org.postgresql.fastpath.*;
>   import org.postgresql.largeobject.*;
>   import org.postgresql.util.*;
> ! import org.postgresql.core.Encoding;
>
>   /**
>    * $Id: Connection.java,v 1.26 2001/08/24 16:50:12 momjian Exp $
> --- 8,14 ----
>   import org.postgresql.fastpath.*;
>   import org.postgresql.largeobject.*;
>   import org.postgresql.util.*;
> ! import org.postgresql.core.*;
>
>   /**
>    * $Id: Connection.java,v 1.26 2001/08/24 16:50:12 momjian Exp $
> ***************
> *** 348,513 ****
>        * @return a ResultSet holding the results
>        * @exception SQLException if a database error occurs
>        */
> !     public java.sql.ResultSet ExecSQL(String sql,java.sql.Statement stat) throws SQLException
>       {
> !       // added Jan 30 2001 to correct maxrows per statement
> !       int maxrows=0;
> !       if(stat!=null)
> !         maxrows=stat.getMaxRows();
> !
> !     // added Oct 7 1998 to give us thread safety.
> !     synchronized(pg_stream) {
> !          // Deallocate all resources in the stream associated
> !           // with a previous request.
> !           // This will let the driver reuse byte arrays that has already
> !           // been allocated instead of allocating new ones in order
> !           // to gain performance improvements.
> !           // PM 17/01/01: Commented out due to race bug. See comments in
> !             // PG_Stream
> !             //pg_stream.deallocate();
> !
> !         Field[] fields = null;
> !         Vector tuples = new Vector();
> !         byte[] buf = null;
> !         int fqp = 0;
> !         boolean hfr = false;
> !         String recv_status = null, msg;
> !         int update_count = 1;
> !         int insert_oid = 0;
> !         SQLException final_error = null;
> !
> !         buf = encoding.encode(sql);
> !         try
> !         {
> !             pg_stream.SendChar('Q');
> !             pg_stream.Send(buf);
> !             pg_stream.SendChar(0);
> !             pg_stream.flush();
> !         } catch (IOException e) {
> !             throw new PSQLException("postgresql.con.ioerror",e);
> !         }
> !
> !         while (!hfr || fqp > 0)
> !         {
> !             Object tup=null;    // holds rows as they are recieved
> !
> !             int c = pg_stream.ReceiveChar();
> !
> !             switch (c)
> !             {
> !             case 'A':    // Asynchronous Notify
> !                 pid = pg_stream.ReceiveInteger(4);
> !                             msg = pg_stream.ReceiveString(encoding);
> !                 break;
> !             case 'B':    // Binary Data Transfer
> !                 if (fields == null)
> !                 throw new PSQLException("postgresql.con.tuple");
> !                 tup = pg_stream.ReceiveTuple(fields.length, true);
> !                 // This implements Statement.setMaxRows()
> !                 if(maxrows==0 || tuples.size()<maxrows)
> !                 tuples.addElement(tup);
> !                 break;
> !             case 'C':    // Command Status
> !                             recv_status = pg_stream.ReceiveString(encoding);
> !
> !                 // Now handle the update count correctly.
> !                 if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") ||
recv_status.startsWith("DELETE")|| recv_status.startsWith("MOVE")) { 
> !                     try {
> !                         update_count = Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' ')));
> !                     } catch(NumberFormatException nfe) {
> !                         throw new PSQLException("postgresql.con.fathom",recv_status);
> !                     }
> !                     if(recv_status.startsWith("INSERT")) {
> !                         try {
> !                         insert_oid = Integer.parseInt(recv_status.substring(1+recv_status.indexOf('
'),recv_status.lastIndexOf(''))); 
> !                         } catch(NumberFormatException nfe) {
> !                         throw new PSQLException("postgresql.con.fathom",recv_status);
> !                         }
> !                     }
> !                 }
> !                 if (fields != null)
> !                 hfr = true;
> !                 else
> !                 {
> !                     try
> !                     {
> !                         pg_stream.SendChar('Q');
> !                         pg_stream.SendChar(' ');
> !                         pg_stream.SendChar(0);
> !                         pg_stream.flush();
> !                     } catch (IOException e) {
> !                         throw new PSQLException("postgresql.con.ioerror",e);
> !                     }
> !                     fqp++;
> !                 }
> !                 break;
> !             case 'D':    // Text Data Transfer
> !                 if (fields == null)
> !                 throw new PSQLException("postgresql.con.tuple");
> !                 tup = pg_stream.ReceiveTuple(fields.length, false);
> !                 // This implements Statement.setMaxRows()
> !                 if(maxrows==0 || tuples.size()<maxrows)
> !                 tuples.addElement(tup);
> !                 break;
> !             case 'E':    // Error Message
> !                             msg = pg_stream.ReceiveString(encoding);
> !                 final_error = new SQLException(msg);
> !                 hfr = true;
> !                 break;
> !             case 'I':    // Empty Query
> !                 int t = pg_stream.ReceiveChar();
> !
> !                 if (t != 0)
> !                 throw new PSQLException("postgresql.con.garbled");
> !                 if (fqp > 0)
> !                 fqp--;
> !                 if (fqp == 0)
> !                 hfr = true;
> !                 break;
> !             case 'N':    // Error Notification
> !                             addWarning(pg_stream.ReceiveString(encoding));
> !                 break;
> !             case 'P':    // Portal Name
> !                             String pname = pg_stream.ReceiveString(encoding);
> !                 break;
> !             case 'T':    // MetaData Field Description
> !                 if (fields != null)
> !                 throw new PSQLException("postgresql.con.multres");
> !                 fields = ReceiveFields();
> !                 break;
> !             case 'Z':       // backend ready for query, ignore for now :-)
> !                 break;
> !             default:
> !                 throw new PSQLException("postgresql.con.type",new Character((char)c));
> !             }
> !         }
> !         if (final_error != null)
> !         throw final_error;
> !
> !         return getResultSet(this, stat, fields, tuples, recv_status, update_count, insert_oid);
> !     }
> !     }
> !
> !     /**
> !      * Receive the field descriptions from the back end
> !      *
> !      * @return an array of the Field object describing the fields
> !      * @exception SQLException if a database error occurs
> !      */
> !     private Field[] ReceiveFields() throws SQLException
> !     {
> !     int nf = pg_stream.ReceiveIntegerR(2), i;
> !     Field[] fields = new Field[nf];
> !
> !     for (i = 0 ; i < nf ; ++i)
> !         {
> !                 String typname = pg_stream.ReceiveString(encoding);
> !         int typid = pg_stream.ReceiveIntegerR(4);
> !         int typlen = pg_stream.ReceiveIntegerR(2);
> !         int typmod = pg_stream.ReceiveIntegerR(4);
> !         fields[i] = new Field(this, typname, typid, typlen, typmod);
> !         }
> !     return fields;
>       }
>
>       /**
> --- 348,356 ----
>        * @return a ResultSet holding the results
>        * @exception SQLException if a database error occurs
>        */
> !     public java.sql.ResultSet ExecSQL(String sql, java.sql.Statement stat) throws SQLException
>       {
> !     return new QueryExecutor(sql, stat, pg_stream, this).execute();
>       }
>
>       /**
> ***************
> *** 793,799 ****
>        * This returns a resultset. It must be overridden, so that the correct
>        * version (from jdbc1 or jdbc2) are returned.
>        */
> !     protected abstract java.sql.ResultSet getResultSet(org.postgresql.Connection conn,java.sql.Statement stat,
Field[]fields, Vector tuples, String status, int updateCount,int insertOID) throws SQLException; 
>
>       /**
>        * In some cases, it is desirable to immediately release a Connection's
> --- 636,642 ----
>        * This returns a resultset. It must be overridden, so that the correct
>        * version (from jdbc1 or jdbc2) are returned.
>        */
> !     public abstract java.sql.ResultSet getResultSet(org.postgresql.Connection conn,java.sql.Statement stat, Field[]
fields,Vector tuples, String status, int updateCount,int insertOID) throws SQLException; 
>
>       /**
>        * In some cases, it is desirable to immediately release a Connection's
> Index: src/interfaces/jdbc/org/postgresql/jdbc1/Connection.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc1/Connection.java,v
> retrieving revision 1.8
> diff -c -r1.8 Connection.java
> *** src/interfaces/jdbc/org/postgresql/jdbc1/Connection.java    2001/08/24 16:50:15    1.8
> --- src/interfaces/jdbc/org/postgresql/jdbc1/Connection.java    2001/08/26 18:33:48
> ***************
> *** 131,137 ****
>        * This overides the method in org.postgresql.Connection and returns a
>        * ResultSet.
>        */
> !     protected java.sql.ResultSet getResultSet(org.postgresql.Connection conn,java.sql.Statement stat, Field[]
fields,Vector tuples, String status, int updateCount,int insertOID) throws SQLException 
>       {
>         // in jdbc1 stat is ignored.
>       return new
org.postgresql.jdbc1.ResultSet((org.postgresql.jdbc1.Connection)conn,fields,tuples,status,updateCount,insertOID);
> --- 131,137 ----
>        * This overides the method in org.postgresql.Connection and returns a
>        * ResultSet.
>        */
> !     public java.sql.ResultSet getResultSet(org.postgresql.Connection conn,java.sql.Statement stat, Field[] fields,
Vectortuples, String status, int updateCount,int insertOID) throws SQLException 
>       {
>         // in jdbc1 stat is ignored.
>       return new
org.postgresql.jdbc1.ResultSet((org.postgresql.jdbc1.Connection)conn,fields,tuples,status,updateCount,insertOID);
> Index: src/interfaces/jdbc/org/postgresql/jdbc2/Connection.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/Connection.java,v
> retrieving revision 1.10
> diff -c -r1.10 Connection.java
> *** src/interfaces/jdbc/org/postgresql/jdbc2/Connection.java    2001/08/24 16:50:16    1.10
> --- src/interfaces/jdbc/org/postgresql/jdbc2/Connection.java    2001/08/26 18:33:49
> ***************
> *** 204,210 ****
>        * This overides the method in org.postgresql.Connection and returns a
>        * ResultSet.
>        */
> !     protected java.sql.ResultSet getResultSet(org.postgresql.Connection conn, java.sql.Statement stat,Field[]
fields,Vector tuples, String status, int updateCount, int insertOID) throws SQLException 
>       {
>         // In 7.1 we now test concurrency to see which class to return. If we are not working with a
>         // Statement then default to a normal ResultSet object.
> --- 204,210 ----
>        * This overides the method in org.postgresql.Connection and returns a
>        * ResultSet.
>        */
> !     public java.sql.ResultSet getResultSet(org.postgresql.Connection conn, java.sql.Statement stat,Field[] fields,
Vectortuples, String status, int updateCount, int insertOID) throws SQLException 
>       {
>         // In 7.1 we now test concurrency to see which class to return. If we are not working with a
>         // Statement then default to a normal ResultSet object.
>
>
> ------------------------------------------------------------------------
>
>
> package org.postgresql.core;
>
> import java.util.Vector;
> import java.io.IOException;
> import java.sql.*;
> import org.postgresql.*;
> import org.postgresql.util.PSQLException;
>
> /**
>  * Executes a query on the backend.
>  *
>  * <p>The lifetime of a QueryExecutor object is from sending the query
>  * until the response has been received from the backend.
>  *
>  * $Id$
>  */
>
> public class QueryExecutor {
>
>     private final String sql;
>     private final java.sql.Statement statement;
>     private final PG_Stream pg_stream;
>     private final org.postgresql.Connection connection;
>
>     public QueryExecutor(String sql,
>              java.sql.Statement statement,
>              PG_Stream pg_stream,
>              org.postgresql.Connection connection)
>     throws SQLException
>     {
>     this.sql = sql;
>     this.statement = statement;
>     this.pg_stream = pg_stream;
>     this.connection = connection;
>
>     if (statement != null)
>         maxRows = statement.getMaxRows();
>     else
>         maxRows = 0;
>     }
>
>     private Field[] fields = null;
>     private Vector tuples = new Vector();
>     private String status = null;
>     private int update_count = 1;
>     private int insert_oid = 0;
>     private int maxRows;
>
>     /**
>      * Execute a query on the backend.
>      */
>     public java.sql.ResultSet execute() throws SQLException {
>
>     int fqp = 0;
>     boolean hfr = false;
>
>     synchronized(pg_stream) {
>
>         sendQuery(sql);
>
>         while (!hfr || fqp > 0) {
>         int c = pg_stream.ReceiveChar();
>
>         switch (c)
>             {
>             case 'A':    // Asynchronous Notify
>             int pid = pg_stream.ReceiveInteger(4);
>             String msg = pg_stream.ReceiveString(connection.getEncoding());
>             break;
>             case 'B':    // Binary Data Transfer
>             receiveTuple(true);
>             break;
>             case 'C':    // Command Status
>             receiveCommandStatus();
>
>             if (fields != null)
>                 hfr = true;
>             else {
>                 sendQuery(" ");
>                 fqp++;
>             }
>             break;
>             case 'D':    // Text Data Transfer
>             receiveTuple(false);
>             break;
>             case 'E':    // Error Message
>                 throw new SQLException(pg_stream.ReceiveString(connection.getEncoding()));
>             case 'I':    // Empty Query
>             int t = pg_stream.ReceiveChar();
>             if (t != 0)
>                 throw new PSQLException("postgresql.con.garbled");
>
>             if (fqp > 0)
>                 fqp--;
>             if (fqp == 0)
>                 hfr = true;
>             break;
>             case 'N':    // Error Notification
>             connection.addWarning(pg_stream.ReceiveString(connection.getEncoding()));
>             break;
>             case 'P':    // Portal Name
>             String pname = pg_stream.ReceiveString(connection.getEncoding());
>             break;
>             case 'T':    // MetaData Field Description
>             receiveFields();
>             break;
>             case 'Z':       // backend ready for query, ignore for now :-)
>             break;
>             default:
>             throw new PSQLException("postgresql.con.type",
>                         new Character((char) c));
>             }
>         }
>
>         return connection.getResultSet(connection, statement, fields, tuples, status, update_count, insert_oid);
>     }
>     }
>
>     /**
>      * Send a query to the backend.
>      */
>     private void sendQuery(String query) throws SQLException {
>     try {
>         pg_stream.SendChar('Q');
>         pg_stream.Send(connection.getEncoding().encode(query));
>         pg_stream.SendChar(0);
>         pg_stream.flush();
>
>     } catch (IOException e) {
>         throw new PSQLException("postgresql.con.ioerror", e);
>     }
>     }
>
>     /**
>      * Receive a tuple from the backend.
>      *
>      * @param isBinary set if the tuple should be treated as binary data
>      */
>     private void receiveTuple(boolean isBinary) throws SQLException {
>     if (fields == null)
>         throw new PSQLException("postgresql.con.tuple");
>     Object tuple = pg_stream.ReceiveTuple(fields.length, isBinary);
>     if (maxRows == 0 || tuples.size() < maxRows)
>         tuples.addElement(tuple);
>     }
>
>     /**
>      * Receive command status from the backend.
>      */
>     private void receiveCommandStatus() throws SQLException {
>
>     status = pg_stream.ReceiveString(connection.getEncoding());
>
>     try {
>         // Now handle the update count correctly.
>         if (status.startsWith("INSERT") || status.startsWith("UPDATE") || status.startsWith("DELETE") ||
status.startsWith("MOVE")){ 
>         update_count = Integer.parseInt(status.substring(1 + status.lastIndexOf(' ')));
>         }
>         if (status.startsWith("INSERT")) {
>         insert_oid = Integer.parseInt(status.substring(1 + status.indexOf(' '),
>                                    status.lastIndexOf(' ')));
>         }
>     } catch (NumberFormatException nfe) {
>         throw new PSQLException("postgresql.con.fathom", status);
>     }
>     }
>
>     /**
>      * Receive the field descriptions from the back end.
>      */
>     private void receiveFields() throws SQLException {
>     if (fields != null)
>         throw new PSQLException("postgresql.con.multres");
>
>     int size = pg_stream.ReceiveIntegerR(2);
>     fields = new Field[size];
>
>     for (int i = 0; i < fields.length; i++) {
>         String typeName = pg_stream.ReceiveString(connection.getEncoding());
>         int typeOid = pg_stream.ReceiveIntegerR(4);
>         int typeLength = pg_stream.ReceiveIntegerR(2);
>         int typeModifier = pg_stream.ReceiveIntegerR(4);
>         fields[i] = new Field(connection, typeName, typeOid, typeLength, typeModifier);
>     }
>     }
> }
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>
> execsql.patch
>
> Content-Type:
>
> TEXT/PLAIN
> Content-Encoding:
>
> BASE64
>
>
> ------------------------------------------------------------------------
> QueryExecutor.java
>
> Content-Type:
>
> TEXT/PLAIN
> Content-Encoding:
>
> BASE64
>
>
> ------------------------------------------------------------------------
> Part 1.4
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> binary
>
>



Re: Attempt to clean up ExecSql() in JDBC

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

>
> Hi,
>
> Attached is my attempt to clean up the horrors of the ExecSQL() method in
> the JDBC driver.
>
> I've done this by extracting it into a new method object called
> QueryExecutor (should go into org/postgresql/core/) and then taking it
> apart into different methods in that class.
>
> A short summary:
>
> * Extracted ExecSQL() from Connection into a method object called
>   QueryExecutor.
>
> * Moved ReceiveFields() from Connection to QueryExecutor.
>
> * Extracted parts of the original ExecSQL() method body into smaller
>   methods on QueryExecutor.
>
> * Bug fix: The instance variable "pid" in Connection was used in two
>   places with different meaning. Both were probably in dead code, but it's
>   fixed anyway.
>
> /Anders
>
>
> PS.: If anyone has any idea what the variable names "fqp" and "hfr" stand
> for, please tell me! :)
>
> _____________________________________________________________________
> A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
> Stockholm, Sweden

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Attempt to clean up ExecSql() in JDBC

From
Bruce Momjian
Date:
Patch applied and file added.  Thanks.

>
> Hi,
>
> Attached is my attempt to clean up the horrors of the ExecSQL() method in
> the JDBC driver.
>
> I've done this by extracting it into a new method object called
> QueryExecutor (should go into org/postgresql/core/) and then taking it
> apart into different methods in that class.
>
> A short summary:
>
> * Extracted ExecSQL() from Connection into a method object called
>   QueryExecutor.
>
> * Moved ReceiveFields() from Connection to QueryExecutor.
>
> * Extracted parts of the original ExecSQL() method body into smaller
>   methods on QueryExecutor.
>
> * Bug fix: The instance variable "pid" in Connection was used in two
>   places with different meaning. Both were probably in dead code, but it's
>   fixed anyway.
>
> /Anders
>
>
> PS.: If anyone has any idea what the variable names "fqp" and "hfr" stand
> for, please tell me! :)
>
> _____________________________________________________________________
> A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
> Stockholm, Sweden

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026