Thread: [PATCH] Cleanup of JDBC character encoding

[PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
Hello,

With this patch I've done an attempt to make the handling of character
encoding in the JDBC driver a little clearer.

* Cleans up the logic to select a JVM encoding for a backend encoding.
* Makes the connection setup code easier to read.
* Gathers character encoding and decoding in a single place.
* Adds unit tests for encoding.
* Introduces a new class, org.postgresql.core.Encoding, and the
corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.

It shouldn't change the external behavior of the driver.

/Anders

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

Attachment

Re: [PATCH] Cleanup of JDBC character encoding

From
Barry Lind
Date:
Anders,

I have some issues with this patch.

1) Please don't require the driver to perform yet another round trip to
the server to get the database encoding.  You changed the following code
which accessed the server once at connection creation:

java.sql.ResultSet initrset = ExecSQL("set datestyle to 'ISO'; " +
        "select case when pg_encoding_to_char(1) = 'SQL_ASCII' then
'UNKNOWN' else getdatabaseencoding() end");

Into two different statements thus requiring two round trips to the
server.  Since connection startup is already expensive adding another
round trip is not a good idea.  The code above was placed there
explicitly to avoid an extra roundtrip.

2) I don't like having test specific code in Encoding.java, specifically
the dummy encoding 'X-UNIT-TESTS'.


thanks,
--Barry



Anders Bengtsson wrote:

> Hello,
>
> With this patch I've done an attempt to make the handling of character
> encoding in the JDBC driver a little clearer.
>
> * Cleans up the logic to select a JVM encoding for a backend encoding.
> * Makes the connection setup code easier to read.
> * Gathers character encoding and decoding in a single place.
> * Adds unit tests for encoding.
> * Introduces a new class, org.postgresql.core.Encoding, and the
> corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.
>
> It shouldn't change the external behavior of the driver.
>
> /Anders
>
> _____________________________________________________________________
> A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
> Stockholm, Sweden
>
>
> ------------------------------------------------------------------------
>
>
> package org.postgresql.test.jdbc2;
>
> import junit.framework.*;
> import org.postgresql.core.Encoding;
>
> /**
>  * Tests for the Encoding class.
>  *
>  * $Id$
>  */
>
>
> public class EncodingTest extends TestCase {
>
>     public EncodingTest(String name) {
>     super(name);
>     }
>
>     public void testCreation() throws Exception {
>     Encoding encoding;
>     encoding = Encoding.forDatabaseEncoding("UNICODE");
>     assertEquals("UTF", encoding.name().substring(0, 3).toUpperCase());
>     encoding = Encoding.forDatabaseEncoding("SQL_ASCII");
>     assert(encoding.name().toUpperCase().indexOf("ASCII") != -1);
>     encoding = Encoding.forDatabaseEncoding("X-UNIT-TESTS");
>     assertEquals("ascii", encoding.name());
>     }
>
>     public void testTransformations() throws Exception {
>     Encoding encoding = Encoding.forDatabaseEncoding("UNICODE");
>     assertEquals("ab", encoding.decode(new byte[] { 97, 98 }));
>
>     assertEquals(2, encoding.encode("ab").length);
>     assertEquals(97, encoding.encode("a")[0]);
>     assertEquals(98, encoding.encode("b")[0]);
>
>     encoding = Encoding.defaultEncoding();
>     assertEquals("a".getBytes()[0], encoding.encode("a")[0]);
>     assertEquals(new String(new byte[] { 97 }),
>              encoding.decode(new byte[] { 97 }));
>     }
> }
>
>
> ------------------------------------------------------------------------
>
> package org.postgresql.core;
>
> import java.io.UnsupportedEncodingException;
> import java.util.*;
> import java.sql.SQLException;
> import org.postgresql.util.*;
>
> /**
>  * Converts to and from the character encoding used by the backend.
>  *
>  * $Id$
>  */
>
> public class Encoding {
>
>     private static final Encoding DEFAULT_ENCODING = new Encoding(null);
>
>     /**
>      * Preferred JVM encodings for backend encodings.
>      */
>     private static final Hashtable encodings = new Hashtable();
>
>     static {
>         encodings.put("SQL_ASCII", new String[] { "ASCII", "us-ascii" });
>         encodings.put("UNICODE", new String[] { "UTF-8", "UTF8" });
>         encodings.put("LATIN1", new String[] { "ISO8859_1" });
>         encodings.put("LATIN2", new String[] { "ISO8859_2" });
>         encodings.put("LATIN3", new String[] { "ISO8859_3" });
>         encodings.put("LATIN4", new String[] { "ISO8859_4" });
>     encodings.put("LATIN5", new String[] { "ISO8859_5" });
>     encodings.put("LATIN6", new String[] { "ISO8859_6" });
>     encodings.put("LATIN7", new String[] { "ISO8859_7" });
>     encodings.put("LATIN8", new String[] { "ISO8859_8" });
>     encodings.put("LATIN9", new String[] { "ISO8859_9" });
>     encodings.put("EUC_JP", new String[] { "EUC_JP" });
>     encodings.put("EUC_CN", new String[] { "EUC_CN" });
>     encodings.put("EUC_KR", new String[] { "EUC_KR" });
>     encodings.put("EUC_TW", new String[] { "EUC_TW" });
>         encodings.put("WIN", new String[] { "Cp1252" });
>     // We prefer KOI8-U, since it is a superset of KOI8-R
>     encodings.put("KOI8", new String[] { "KOI8_U", "KOI8_R" });
>     // Database is not encoding-aware, so we'll settle with the default encoding
>         encodings.put("UNKNOWN", new String[0]);
>     // Data for unit tests
>     encodings.put("X-UNIT-TESTS", new String[] { "dummy1", "dummy2", "ascii" });
>     }
>
>     private final String encoding;
>
>     public Encoding(String encoding) {
>     this.encoding = encoding;
>     }
>
>     /**
>      * Get an Encoding matching the given database encoding.
>      */
>     public static Encoding forDatabaseEncoding(String databaseEncoding) {
>     // If the backend encoding is known and there is a suitable
>     // encoding in the JVM we use that. Otherwise we fall back
>     // to the default encoding of the JVM.
>
>     if (encodings.containsKey(databaseEncoding)) {
>         String[] candidates = (String[]) encodings.get(databaseEncoding);
>         for (int i = 0; i < candidates.length; i++) {
>         if (isAvailable(candidates[i])) {
>             return new Encoding(candidates[i]);
>         }
>         }
>     }
>     return defaultEncoding();
>     }
>
>     /**
>      * Name of the (JVM) encoding used.
>      */
>     public String name() {
>     return encoding;
>     }
>
>     /**
>      * Encode a string to an array of bytes.
>      */
>     public byte[] encode(String s) throws SQLException {
>     try {
>         if (encoding == null) {
>         return s.getBytes();
>         } else {
>         return s.getBytes(encoding);
>         }
>     } catch (UnsupportedEncodingException e) {
>         throw new PSQLException("postgresql.stream.encoding", e);
>     }
>     }
>
>     /**
>      * Decode an array of bytes into a string.
>      */
>     public String decode(byte[] encodedString, int offset, int length) throws SQLException {
>     try {
>         if (encoding == null) {
>         return new String(encodedString, offset, length);
>         } else {
>         return new String(encodedString, offset, length, encoding);
>         }
>     } catch (UnsupportedEncodingException e) {
>         throw new PSQLException("postgresql.stream.encoding", e);
>     }
>     }
>
>     /**
>      * Decode an array of bytes into a string.
>      */
>     public String decode(byte[] encodedString) throws SQLException {
>     return decode(encodedString, 0, encodedString.length);
>     }
>
>     /**
>      * Get an Encoding using the default encoding for the JVM.
>      */
>     public static Encoding defaultEncoding() {
>     return DEFAULT_ENCODING;
>     }
>
>     /**
>      * Test if an encoding is available in the JVM.
>      */
>     public static boolean isAvailable(String encodingName) {
>     try {
>         "DUMMY".getBytes(encodingName);
>         return true;
>     } catch (UnsupportedEncodingException e) {
>         return false;
>     }
>     }
> }
>
>
> ------------------------------------------------------------------------
>
> Index: src/interfaces/jdbc/Implementation
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/Implementation,v
> retrieving revision 1.2
> diff -u -r1.2 Implementation
> --- src/interfaces/jdbc/Implementation    2001/01/18 14:50:14    1.2
> +++ src/interfaces/jdbc/Implementation    2001/07/08 15:00:57
> @@ -151,6 +151,7 @@
>  MemoryPool      Interface for managing MemoryPools. Not used (yet).
>  ObjectPool      Interface for an Object Pool
>  SimpleObjectPool Class that implements ObjectPool and used by BytePoolDim#
> +Encoding        Character encoding logic, mainly for Connection and PG_Stream.
>
>  Package org.postgresql.fastpath
>  ---------------------------
> 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.17
> diff -u -r1.17 Connection.java
> --- src/interfaces/jdbc/org/postgresql/Connection.java    2001/06/07 00:09:32    1.17
> +++ src/interfaces/jdbc/org/postgresql/Connection.java    2001/07/08 15:06:26
> @@ -8,6 +8,7 @@
>  import org.postgresql.fastpath.*;
>  import org.postgresql.largeobject.*;
>  import org.postgresql.util.*;
> +import org.postgresql.core.Encoding;
>
>  /**
>   * $Id: Connection.java,v 1.17 2001/06/07 00:09:32 momjian Exp $
> @@ -33,11 +34,8 @@
>
>    /**
>     *  The encoding to use for this connection.
> -   *  If <b>null</b>, the encoding has not been specified by the
> -   *  user, and the default encoding for the platform should be
> -   *  used.
>     */
> -  private String encoding;
> +  private Encoding encoding = Encoding.defaultEncoding();
>
>    public boolean CONNECTION_OK = true;
>    public boolean CONNECTION_BAD = false;
> @@ -168,7 +166,7 @@
>          // "User authentication failed"
>          //
>          throw new SQLException(pg_stream.ReceiveString
> -                                       (receive_sbuf, 4096, getEncoding()));
> +                                       (receive_sbuf, 4096, encoding));
>
>            case 'R':
>          // Get the type of request
> @@ -239,7 +237,7 @@
>      case 'E':
>      case 'N':
>             throw new SQLException(pg_stream.ReceiveString
> -                                  (receive_sbuf, 4096, getEncoding()));
> +                                  (receive_sbuf, 4096, encoding));
>          default:
>            throw new PSQLException("postgresql.con.setup");
>        }
> @@ -251,110 +249,28 @@
>         break;
>      case 'E':
>      case 'N':
> -           throw new SQLException(pg_stream.ReceiveString(receive_sbuf, 4096, getEncoding()));
> +           throw new SQLException(pg_stream.ReceiveString(receive_sbuf, 4096, encoding));
>          default:
>            throw new PSQLException("postgresql.con.setup");
>        }
>
> -      // Originally we issued a SHOW DATESTYLE statement to find the databases default
> -      // datestyle. However, this caused some problems with timestamps, so in 6.5, we
> -      // went the way of ODBC, and set the connection to ISO.
> -      //
> -      // This may cause some clients to break when they assume anything other than ISO,
> -      // but then - they should be using the proper methods ;-)
> -      //
> -      // We also ask the DB for certain properties (i.e. DatabaseEncoding at this time)
> -      //
>        firstWarning = null;
>
> -      java.sql.ResultSet initrset = ExecSQL("set datestyle to 'ISO'; " +
> -        "select case when pg_encoding_to_char(1) = 'SQL_ASCII' then 'UNKNOWN' else getdatabaseencoding() end");
> +      ExecSQL("set datestyle to 'ISO'");
>
> -      String dbEncoding = null;
> -      //retrieve DB properties
> -      if(initrset.next()) {
> -
> -        //handle DatabaseEncoding
> -        dbEncoding = initrset.getString(1);
> -        //convert from the PostgreSQL name to the Java name
> -        if (dbEncoding.equals("SQL_ASCII")) {
> -          dbEncoding = "ASCII";
> -        } else if (dbEncoding.equals("UNICODE")) {
> -          dbEncoding = "UTF8";
> -        } else if (dbEncoding.equals("LATIN1")) {
> -          dbEncoding = "ISO8859_1";
> -        } else if (dbEncoding.equals("LATIN2")) {
> -          dbEncoding = "ISO8859_2";
> -        } else if (dbEncoding.equals("LATIN3")) {
> -          dbEncoding = "ISO8859_3";
> -        } else if (dbEncoding.equals("LATIN4")) {
> -          dbEncoding = "ISO8859_4";
> -        } else if (dbEncoding.equals("LATIN5")) {
> -          dbEncoding = "ISO8859_5";
> -        } else if (dbEncoding.equals("LATIN6")) {
> -          dbEncoding = "ISO8859_6";
> -        } else if (dbEncoding.equals("LATIN7")) {
> -          dbEncoding = "ISO8859_7";
> -        } else if (dbEncoding.equals("LATIN8")) {
> -          dbEncoding = "ISO8859_8";
> -        } else if (dbEncoding.equals("LATIN9")) {
> -          dbEncoding = "ISO8859_9";
> -        } else if (dbEncoding.equals("EUC_JP")) {
> -          dbEncoding = "EUC_JP";
> -        } else if (dbEncoding.equals("EUC_CN")) {
> -          dbEncoding = "EUC_CN";
> -        } else if (dbEncoding.equals("EUC_KR")) {
> -          dbEncoding = "EUC_KR";
> -        } else if (dbEncoding.equals("EUC_TW")) {
> -          dbEncoding = "EUC_TW";
> -        } else if (dbEncoding.equals("KOI8")) {
> -      // try first if KOI8_U is present, it's a superset of KOI8_R
> -        try {
> -            dbEncoding = "KOI8_U";
> -        "test".getBytes(dbEncoding);
> -        }
> -        catch(UnsupportedEncodingException uee) {
> -        // well, KOI8_U is still not in standard JDK, falling back to KOI8_R :(
> -            dbEncoding = "KOI8_R";
> -        }
> -
> -        } else if (dbEncoding.equals("WIN")) {
> -          dbEncoding = "Cp1252";
> -        } else if (dbEncoding.equals("UNKNOWN")) {
> -          //This isn't a multibyte database so we don't have an encoding to use
> -          //We leave dbEncoding null which will cause the default encoding for the
> -          //JVM to be used
> -          dbEncoding = null;
> -        } else {
> -          dbEncoding = null;
> -        }
> -      }
> -
> -
> -      //Set the encoding for this connection
> -      //Since the encoding could be specified or obtained from the DB we use the
> -      //following order:
> -      //  1.  passed as a property
> -      //  2.  value from DB if supported by current JVM
> -      //  3.  default for JVM (leave encoding null)
> -      String passedEncoding = info.getProperty("charSet");  // could be null
> -
> +      String passedEncoding = info.getProperty("charSet");
>        if (passedEncoding != null) {
> -        encoding = passedEncoding;
> +      if (Encoding.isAvailable(passedEncoding)) {
> +          encoding = new Encoding(passedEncoding);
> +      } else {
> +          encoding = Encoding.defaultEncoding();
> +      }
>        } else {
> -        if (dbEncoding != null) {
> -          //test DB encoding
> -          try {
> -            "TEST".getBytes(dbEncoding);
> -            //no error the encoding is supported by the current JVM
> -            encoding = dbEncoding;
> -          } catch (UnsupportedEncodingException uee) {
> -            //dbEncoding is not supported by the current JVM
> -            encoding = null;
> -          }
> -        } else {
> -          encoding = null;
> -        }
> +      java.sql.ResultSet resultSet = ExecSQL("select case when pg_encoding_to_char(1) = 'SQL_ASCII' then 'UNKNOWN'
elsegetdatabaseencoding() end"); 
> +      if (resultSet.next()) {
> +          String dbEncoding = resultSet.getString(1);
> +          encoding = Encoding.forDatabaseEncoding(dbEncoding);
> +      }
>        }
>
>        // Initialise object handling
> @@ -454,23 +370,8 @@
>          int update_count = 1;
>          int insert_oid = 0;
>          SQLException final_error = null;
> -
> -        // Commented out as the backend can now handle queries
> -        // larger than 8K. Peter June 6 2000
> -        //if (sql.length() > 8192)
> -        //throw new PSQLException("postgresql.con.toolong",sql);
> -
> -        if (getEncoding() == null)
> -            buf = sql.getBytes();
> -        else {
> -            try {
> -                buf = sql.getBytes(getEncoding());
> -            } catch (UnsupportedEncodingException unse) {
> -                 throw new PSQLException("postgresql.con.encoding",
> -                                        unse);
> -            }
> -        }
>
> +        buf = encoding.encode(sql);
>          try
>          {
>              pg_stream.SendChar('Q');
> @@ -491,7 +392,7 @@
>              {
>              case 'A':    // Asynchronous Notify
>                  pid = pg_stream.ReceiveInteger(4);
> -                msg = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
> +                msg = pg_stream.ReceiveString(receive_sbuf,8192,encoding);
>                  break;
>              case 'B':    // Binary Data Transfer
>                  if (fields == null)
> @@ -502,7 +403,7 @@
>                  tuples.addElement(tup);
>                  break;
>              case 'C':    // Command Status
> -                recv_status = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
> +                recv_status = pg_stream.ReceiveString(receive_sbuf,8192,encoding);
>
>                  // Now handle the update count correctly.
>                  if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") ||
recv_status.startsWith("DELETE")|| recv_status.startsWith("MOVE")) { 
> @@ -544,7 +445,7 @@
>                  tuples.addElement(tup);
>                  break;
>              case 'E':    // Error Message
> -                msg = pg_stream.ReceiveString(receive_sbuf,4096,getEncoding());
> +                msg = pg_stream.ReceiveString(receive_sbuf,4096,encoding);
>                  final_error = new SQLException(msg);
>                  hfr = true;
>                  break;
> @@ -559,10 +460,10 @@
>                  hfr = true;
>                  break;
>              case 'N':    // Error Notification
> -                addWarning(pg_stream.ReceiveString(receive_sbuf,4096,getEncoding()));
> +                addWarning(pg_stream.ReceiveString(receive_sbuf,4096,encoding));
>                  break;
>              case 'P':    // Portal Name
> -                String pname = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
> +                String pname = pg_stream.ReceiveString(receive_sbuf,8192,encoding);
>                  break;
>              case 'T':    // MetaData Field Description
>                  if (fields != null)
> @@ -595,7 +496,7 @@
>
>      for (i = 0 ; i < nf ; ++i)
>          {
> -        String typname = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
> +        String typname = pg_stream.ReceiveString(receive_sbuf,8192,encoding);
>          int typid = pg_stream.ReceiveIntegerR(4);
>          int typlen = pg_stream.ReceiveIntegerR(2);
>          int typmod = pg_stream.ReceiveIntegerR(4);
> @@ -665,7 +566,7 @@
>       *  default encoding.
>       */
>      public String getEncoding() throws SQLException {
> -        return encoding;
> +        return encoding.name();
>      }
>
>      /**
> Index: src/interfaces/jdbc/org/postgresql/PG_Stream.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/PG_Stream.java,v
> retrieving revision 1.8
> diff -u -r1.8 PG_Stream.java
> --- src/interfaces/jdbc/org/postgresql/PG_Stream.java    2001/07/06 18:01:22    1.8
> +++ src/interfaces/jdbc/org/postgresql/PG_Stream.java    2001/07/08 15:06:30
> @@ -10,7 +10,7 @@
>  import org.postgresql.util.*;
>
>  /**
> - * @version 1.0 15-APR-1997
> + * $Id$
>   *
>   * This class is used by Connection & PGlobj for communicating with the
>   * backend.
> @@ -211,7 +211,7 @@
>    public String ReceiveString(int maxsiz) throws SQLException
>    {
>      byte[] rst = bytePoolDim1.allocByte(maxsiz);
> -    return ReceiveString(rst, maxsiz, null);
> +    return ReceiveString(rst, maxsiz, Encoding.defaultEncoding());
>    }
>
>    /**
> @@ -225,7 +225,7 @@
>     * @return string from back end
>     * @exception SQLException if an I/O error occurs
>     */
> -  public String ReceiveString(int maxsiz, String encoding) throws SQLException
> +  public String ReceiveString(int maxsiz, Encoding encoding) throws SQLException
>    {
>      byte[] rst = bytePoolDim1.allocByte(maxsiz);
>      return ReceiveString(rst, maxsiz, encoding);
> @@ -243,9 +243,12 @@
>     * @return string from back end
>     * @exception SQLException if an I/O error occurs
>     */
> -  public String ReceiveString(byte rst[], int maxsiz, String encoding)
> +  public String ReceiveString(byte rst[], int maxsiz, Encoding encoding)
>        throws SQLException
>    {
> +    if (encoding == null)
> +      encoding = Encoding.defaultEncoding();
> +
>      int s = 0;
>
>      try
> @@ -265,18 +268,8 @@
>        throw new PSQLException("postgresql.stream.toomuch");
>        } catch (IOException e) {
>      throw new PSQLException("postgresql.stream.ioerror",e);
> -      }
> -      String v = null;
> -      if (encoding == null)
> -          v = new String(rst, 0, s);
> -      else {
> -          try {
> -              v = new String(rst, 0, s, encoding);
> -          } catch (UnsupportedEncodingException unse) {
> -              throw new PSQLException("postgresql.stream.encoding", unse);
> -          }
>        }
> -      return v;
> +      return encoding.decode(rst, 0, s);
>    }
>
>    /**
> Index: src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java,v
> retrieving revision 1.3
> diff -u -r1.3 JDBC2Tests.java
> --- src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java    2001/02/14 17:45:17    1.3
> +++ src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java    2001/07/08 15:06:31
> @@ -195,6 +195,7 @@
>      suite.addTestSuite(DriverTest.class);
>      suite.addTestSuite(ConnectionTest.class);
>      suite.addTestSuite(DatabaseMetaDataTest.class);
> +    suite.addTestSuite(EncodingTest.class);
>
>      // Connectivity/Protocols
>
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>
> EncodingTest.java
>
> Content-Type:
>
> TEXT/PLAIN
> Content-Encoding:
>
> BASE64
>
>
> ------------------------------------------------------------------------
> Encoding.java
>
> Content-Type:
>
> TEXT/PLAIN
> Content-Encoding:
>
> BASE64
>
>
> ------------------------------------------------------------------------
> encoding.patch
>
> Content-Type:
>
> TEXT/PLAIN
> Content-Encoding:
>
> BASE64
>
>
> ------------------------------------------------------------------------
> Part 1.5
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> binary
>
>



Re: [PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
On Mon, 9 Jul 2001, Barry Lind wrote:

> Anders,
>
> I have some issues with this patch.
>
> 1) Please don't require the driver to perform yet another round trip to
> the server to get the database encoding.  You changed the following code
> which accessed the server once at connection creation:
[snip]
> 2) I don't like having test specific code in Encoding.java, specifically
> the dummy encoding 'X-UNIT-TESTS'.

Thanks Barry, I'll fix these things and return with a new patch.

I think I'll be able to get pretty good test coverage even without the
extra test data.

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


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


Re: Re: [PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
> > I have some issues with this patch.
> >
> > 1) Please don't require the driver to perform yet another round trip to
> > the server to get the database encoding.  You changed the following code
> > which accessed the server once at connection creation:
> [snip]
> > 2) I don't like having test specific code in Encoding.java, specifically
> > the dummy encoding 'X-UNIT-TESTS'.

Ok, now I've made the changes, and added a comment about why the code for
(1) should stay optimized. :)

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

Attachment

Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
Oops, patch superceeded.  Removing from queue.

> Hello,
>
> With this patch I've done an attempt to make the handling of character
> encoding in the JDBC driver a little clearer.
>
> * Cleans up the logic to select a JVM encoding for a backend encoding.
> * Makes the connection setup code easier to read.
> * Gathers character encoding and decoding in a single place.
> * Adds unit tests for encoding.
> * Introduces a new class, org.postgresql.core.Encoding, and the
> corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.
>
> It shouldn't change the external behavior of the driver.
>
> /Anders
>
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
  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: [JDBC] Re: Re: [PATCH] Cleanup of JDBC character encoding

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.

>
> > > I have some issues with this patch.
> > >
> > > 1) Please don't require the driver to perform yet another round trip to
> > > the server to get the database encoding.  You changed the following code
> > > which accessed the server once at connection creation:
> > [snip]
> > > 2) I don't like having test specific code in Encoding.java, specifically
> > > the dummy encoding 'X-UNIT-TESTS'.
>
> Ok, now I've made the changes, and added a comment about why the code for
> (1) should stay optimized. :)
>
> /Anders
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

--
  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: Re: [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
Here is the right one:

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.

> On Mon, 9 Jul 2001, Barry Lind wrote:
>
> > Anders,
> >
> > I have some issues with this patch.
> >
> > 1) Please don't require the driver to perform yet another round trip to
> > the server to get the database encoding.  You changed the following code
> > which accessed the server once at connection creation:
> [snip]
> > 2) I don't like having test specific code in Encoding.java, specifically
> > the dummy encoding 'X-UNIT-TESTS'.
>
> Thanks Barry, I'll fix these things and return with a new patch.
>
> I think I'll be able to get pretty good test coverage even without the
> extra test data.
>
> /Anders
> _____________________________________________________________________
> A n d e r s  B e n g t s s o n                   ndrsbngtssn@yahoo.se
> Stockholm, Sweden
>
>
> _________________________________________________________
> Do You Yahoo!?
> Get your free @yahoo.com address at http://mail.yahoo.com
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>

--
  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: [JDBC] [PATCH] Cleanup of JDBC character encoding

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.

> Hello,
>
> With this patch I've done an attempt to make the handling of character
> encoding in the JDBC driver a little clearer.
>
> * Cleans up the logic to select a JVM encoding for a backend encoding.
> * Makes the connection setup code easier to read.
> * Gathers character encoding and decoding in a single place.
> * Adds unit tests for encoding.
> * Introduces a new class, org.postgresql.core.Encoding, and the
> corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.
>
> It shouldn't change the external behavior of the driver.
>
> /Anders
>
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
  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: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
On Thu, 12 Jul 2001, Bruce Momjian wrote:

> Your patch has been added to the PostgreSQL unapplied patches list at:

Here is a new version of that patch, with improvements from some feedback
I got from Barry Lind.
More of the encoding-related logic is moved into the Encoding class and
I've added som clarifying comments.

/Anders

> > With this patch I've done an attempt to make the handling of character
> > encoding in the JDBC driver a little clearer.
> >
> > * Cleans up the logic to select a JVM encoding for a backend encoding.
> > * Makes the connection setup code easier to read.
> > * Gathers character encoding and decoding in a single place.
> > * Adds unit tests for encoding.
> > * Introduces a new class, org.postgresql.core.Encoding, and the
> > corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.


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

Attachment

Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
Old removed.

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.

> On Thu, 12 Jul 2001, Bruce Momjian wrote:
>
> > Your patch has been added to the PostgreSQL unapplied patches list at:
>
> Here is a new version of that patch, with improvements from some feedback
> I got from Barry Lind.
> More of the encoding-related logic is moved into the Encoding class and
> I've added som clarifying comments.
>
> /Anders
>
> > > With this patch I've done an attempt to make the handling of character
> > > encoding in the JDBC driver a little clearer.
> > >
> > > * Cleans up the logic to select a JVM encoding for a backend encoding.
> > > * Makes the connection setup code easier to read.
> > > * Gathers character encoding and decoding in a single place.
> > > * Adds unit tests for encoding.
> > > * Introduces a new class, org.postgresql.core.Encoding, and the
> > > corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.
>
>
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

--
  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: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
I am having trouble applying this patch to the current CVS tree.  The
majority of changes fail to apply.  Can you give me a patch against
current CVS or can someone manually merge the changes into CVS and send
me a patch?  Thanks.


> On Thu, 12 Jul 2001, Bruce Momjian wrote:
>
> > Your patch has been added to the PostgreSQL unapplied patches list at:
>
> Here is a new version of that patch, with improvements from some feedback
> I got from Barry Lind.
> More of the encoding-related logic is moved into the Encoding class and
> I've added som clarifying comments.
>
> /Anders
>
> > > With this patch I've done an attempt to make the handling of character
> > > encoding in the JDBC driver a little clearer.
> > >
> > > * Cleans up the logic to select a JVM encoding for a backend encoding.
> > > * Makes the connection setup code easier to read.
> > > * Gathers character encoding and decoding in a single place.
> > > * Adds unit tests for encoding.
> > > * Introduces a new class, org.postgresql.core.Encoding, and the
> > > corresponding unit test class, org.postgresql.test.jdbc2.EncodingTest.
>
>
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

--
  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: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
On Sun, 15 Jul 2001, Bruce Momjian wrote:

> I am having trouble applying this patch to the current CVS tree.  The
> majority of changes fail to apply.  Can you give me a patch against
> current CVS or can someone manually merge the changes into CVS and send
> me a patch?  Thanks.

Here's a patch against the current CVS. The changes from the previous
patch are mostly related to the changed interface for PG_Stream.

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

Attachment

Re: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

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.

> On Sun, 15 Jul 2001, Bruce Momjian wrote:
>
> > I am having trouble applying this patch to the current CVS tree.  The
> > majority of changes fail to apply.  Can you give me a patch against
> > current CVS or can someone manually merge the changes into CVS and send
> > me a patch?  Thanks.
>
> Here's a patch against the current CVS. The changes from the previous
> patch are mostly related to the changed interface for PG_Stream.
>
> /Anders
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

--
  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: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
OK, patch applied.  I put the new java files in /core and /test as
indicated in the java package header in the source code.  Patch applied
too.

> On Sun, 15 Jul 2001, Bruce Momjian wrote:
>
> > I am having trouble applying this patch to the current CVS tree.  The
> > majority of changes fail to apply.  Can you give me a patch against
> > current CVS or can someone manually merge the changes into CVS and send
> > me a patch?  Thanks.
>
> Here's a patch against the current CVS. The changes from the previous
> patch are mostly related to the changed interface for PG_Stream.
>
> /Anders
> _____________________________________________________________________
> 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... ]

Content-Description:

[ Attachment, skipping... ]

--
  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: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Anders Bengtsson
Date:
On Sat, 21 Jul 2001, Bruce Momjian wrote:

> OK, patch applied.  I put the new java files in /core and /test as
> indicated in the java package header in the source code.  Patch applied
> too.

Almost right. :-) EncodingTest.java should go in
"org/postgresql/test/jdbc2/", not in "org/postgresql/test/". In it's
current state the JDBC driver won't compile.

(But I agree that test/ would be a more natural place for EncodingTest,
however that would require bigger changes to the test setup, something
that can wait).

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


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


Re: Re: [JDBC] [PATCH] Cleanup of JDBC character encoding

From
Bruce Momjian
Date:
> On Sat, 21 Jul 2001, Bruce Momjian wrote:
>
> > OK, patch applied.  I put the new java files in /core and /test as
> > indicated in the java package header in the source code.  Patch applied
> > too.
>
> Almost right. :-) EncodingTest.java should go in
> "org/postgresql/test/jdbc2/", not in "org/postgresql/test/". In it's
> current state the JDBC driver won't compile.
>
> (But I agree that test/ would be a more natural place for EncodingTest,
> however that would require bigger changes to the test setup, something
> that can wait).

Moved.  Funny, it compiled here.  My guess is that is wouldn't have
worked properly.

Thanks.
--
  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