Merge pull request #533 from httpdigest/master

Implement right-padding of CHAR(N) datatype in PostgreSQL mode

h2/src/docsrc/html/changelog.html

@@ -21,7 +21,9 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
-<li>-
+<li>Add padding for CHAR(N) values in PostgreSQL mode 
+</li>
+<li>
 </li>
 </ul>
 

h2/src/main/org/h2/command/dml/Insert.java

@@ -140,7 +140,7 @@ public class Insert extends Prepared implements ResultTarget {
                         // e can be null (DEFAULT)
                         e = e.optimize(session);
                         try {
-                            Value v = c.convert(e.getValue(session));
+                            Value v = c.convert(e.getValue(session), session.getDatabase().getMode());
                             newRow.setValue(index, v);
                         } catch (DbException ex) {
                             throw setRow(ex, x, getSQL(expr));
@@ -186,7 +186,7 @@ public class Insert extends Prepared implements ResultTarget {
             Column c = columns[j];
             int index = c.getColumnId();
             try {
-                Value v = c.convert(values[j]);
+                Value v = c.convert(values[j], session.getDatabase().getMode());
                 newRow.setValue(index, v);
             } catch (DbException ex) {
                 throw setRow(ex, rowNumber, getSQL(values));

h2/src/main/org/h2/engine/Mode.java

@@ -160,6 +160,11 @@ public class Mode {
      */
     public boolean allowAffinityKey;
 
+    /**
+     * Whether to right-pad fixed strings with spaces.
+     */
+    public boolean padFixedLengthStrings;
+
     private final String name;
 
     static {
@@ -256,6 +261,7 @@ public class Mode {
         mode.supportedClientInfoPropertiesRegEx =
                 Pattern.compile("ApplicationName");
         mode.prohibitEmptyInPredicate = true;
+        mode.padFixedLengthStrings = true;
         add(mode);
 
         mode = new Mode("Ignite");

h2/src/main/org/h2/expression/Comparison.java

@@ -13,6 +13,7 @@ import org.h2.index.IndexCondition;
 import org.h2.message.DbException;
 import org.h2.table.ColumnResolver;
 import org.h2.table.TableFilter;
+import org.h2.util.MathUtils;
 import org.h2.util.New;
 import org.h2.value.Value;
 import org.h2.value.ValueBoolean;
@@ -201,7 +202,7 @@ public class Comparison extends Condition {
                     // to constant type, but vise versa, then let's do this here
                     // once.
                     if (constType != resType) {
-                        right = ValueExpression.get(r.convertTo(resType));
+                        right = ValueExpression.get(r.convertTo(resType, MathUtils.convertLongToInt(left.getPrecision()), session.getDatabase().getMode()));
                     }
                 } else if (right instanceof Parameter) {
                     ((Parameter) right).setColumn(

h2/src/main/org/h2/table/Column.java

@@ -163,8 +163,21 @@ public class Column {
      * @return the value
      */
     public Value convert(Value v) {
+        return convert(v, null);
+    }
+
+    /**
+     * Convert a value to this column's type using the given {@link Mode}.
+     * <p>
+     * Use this method in case the conversion is Mode-dependent.
+     *
+     * @param v the value
+     * @param mode the database {@link Mode} to use
+     * @return the value
+     */
+    public Value convert(Value v, Mode mode) {
         try {
-            return v.convertTo(type);
+            return v.convertTo(type, MathUtils.convertLongToInt(precision), mode);
         } catch (DbException e) {
             if (e.getErrorCode() == ErrorCode.DATA_CONVERSION_ERROR_1) {
                 String target = (table == null ? "" : table.getName() + ": ") +

h2/src/main/org/h2/value/Transfer.java

@@ -615,7 +615,7 @@ public class Transfer {
         case Value.STRING_IGNORECASE:
             return ValueStringIgnoreCase.get(readString());
         case Value.STRING_FIXED:
-            return ValueStringFixed.get(readString());
+            return ValueStringFixed.get(readString(), ValueStringFixed.PRECISION_DO_NOT_TRIM, null);
         case Value.BLOB: {
             long length = readLong();
             if (version >= Constants.TCP_PROTOCOL_VERSION_11) {

h2/src/main/org/h2/value/Value.java

@@ -20,6 +20,7 @@ import java.sql.Timestamp;
 import java.sql.Types;
 import org.h2.api.ErrorCode;
 import org.h2.engine.Constants;
+import org.h2.engine.Mode;
 import org.h2.engine.SysProperties;
 import org.h2.message.DbException;
 import org.h2.store.DataHandler;
@@ -541,6 +542,21 @@ public abstract class Value {
      * @return the converted value
      */
     public Value convertTo(int targetType) {
+        // Use -1 to indicate "default behaviour" where value conversion should not 
+        // depend on any datatype precision.
+        return convertTo(targetType, -1, null);
+    }
+
+    /**
+     * Compare a value to the specified type.
+     *
+     * @param targetType the type of the returned value
+     * @param the precision of the column to convert this value to.
+     *        The special constant <code>-1</code> is used to indicate that
+     *        the precision plays no role when converting the value
+     * @return the converted value
+     */
+    public Value convertTo(int targetType, int precision, Mode mode) {
         // converting NULL is done in ValueNull
         // converting BLOB to CLOB and vice versa is done in ValueLob
         if (getType() == targetType) {
@@ -962,7 +978,7 @@ public abstract class Value {
             case STRING_IGNORECASE:
                 return ValueStringIgnoreCase.get(s);
             case STRING_FIXED:
-                return ValueStringFixed.get(s);
+                return ValueStringFixed.get(s, precision, mode);
             case DOUBLE:
                 return ValueDouble.get(Double.parseDouble(s.trim()));
             case FLOAT:

h2/src/main/org/h2/value/ValueGeometry.java

@@ -12,6 +12,8 @@ import java.util.Arrays;
 import com.vividsolutions.jts.geom.CoordinateSequence;
 import com.vividsolutions.jts.geom.CoordinateSequenceFilter;
 import com.vividsolutions.jts.geom.PrecisionModel;
+
+import org.h2.engine.Mode;
 import org.h2.message.DbException;
 import org.h2.util.StringUtils;
 import com.vividsolutions.jts.geom.Envelope;
@@ -272,11 +274,11 @@ public class ValueGeometry extends Value {
     }
 
     @Override
-    public Value convertTo(int targetType) {
+    public Value convertTo(int targetType, int precision, Mode mode) {
         if (targetType == Value.JAVA_OBJECT) {
             return this;
         }
-        return super.convertTo(targetType);
+        return super.convertTo(targetType, precision, mode);
     }
 
     /**

h2/src/main/org/h2/value/ValueLob.java

@@ -14,6 +14,7 @@ import java.io.Reader;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import org.h2.engine.Constants;
+import org.h2.engine.Mode;
 import org.h2.engine.SysProperties;
 import org.h2.message.DbException;
 import org.h2.mvstore.DataUtils;
@@ -443,7 +444,7 @@ public class ValueLob extends Value {
      * @return the converted value
      */
     @Override
-    public Value convertTo(int t) {
+    public Value convertTo(int t, int precision, Mode mode) {
         if (t == type) {
             return this;
         } else if (t == Value.CLOB) {
@@ -453,7 +454,7 @@ public class ValueLob extends Value {
             ValueLob copy = ValueLob.createBlob(getInputStream(), -1, handler);
             return copy;
         }
-        return super.convertTo(t);
+        return super.convertTo(t, precision, mode);
     }
 
     @Override

h2/src/main/org/h2/value/ValueLobDb.java

@@ -14,6 +14,7 @@ import java.io.Reader;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import org.h2.engine.Constants;
+import org.h2.engine.Mode;
 import org.h2.engine.SysProperties;
 import org.h2.message.DbException;
 import org.h2.mvstore.DataUtils;
@@ -184,7 +185,7 @@ public class ValueLobDb extends Value implements Value.ValueClob,
      * @return the converted value
      */
     @Override
-    public Value convertTo(int t) {
+    public Value convertTo(int t, int precision, Mode mode) {
         if (t == type) {
             return this;
         } else if (t == Value.CLOB) {
@@ -204,7 +205,7 @@ public class ValueLobDb extends Value implements Value.ValueClob,
                 return ValueLobDb.createSmallLob(t, small);
             }
         }
-        return super.convertTo(t);
+        return super.convertTo(t, precision, mode);
     }
 
     @Override

h2/src/main/org/h2/value/ValueNull.java

@@ -14,6 +14,7 @@ import java.sql.SQLException;
 import java.sql.Time;
 import java.sql.Timestamp;
 
+import org.h2.engine.Mode;
 import org.h2.message.DbException;
 
 /**
@@ -132,7 +133,7 @@ public class ValueNull extends Value {
     }
 
     @Override
-    public Value convertTo(int type) {
+    public Value convertTo(int type, int precision, Mode mode) {
         return this;
     }
 

h2/src/main/org/h2/value/ValueStringFixed.java

@@ -5,6 +5,9 @@
  */
 package org.h2.value;
 
+import java.util.Arrays;
+
+import org.h2.engine.Mode;
 import org.h2.engine.SysProperties;
 import org.h2.util.StringUtils;
 
@@ -13,6 +16,18 @@ import org.h2.util.StringUtils;
  */
 public class ValueStringFixed extends ValueString {
 
+    /**
+     * Special value for the precision in {@link #get(String, int, Mode)} to indicate that the value
+     * should <i>not</i> be trimmed.
+     */
+    public static final int PRECISION_DO_NOT_TRIM = Integer.MIN_VALUE;
+
+    /**
+     * Special value for the precision in {@link #get(String, int, Mode)} to indicate that the default
+     * behaviour should of trimming the value should apply.
+     */
+    public static final int PRECISION_TRIM = -1;
+
     private static final ValueStringFixed EMPTY = new ValueStringFixed("");
 
     protected ValueStringFixed(String value) {
@@ -20,15 +35,29 @@ public class ValueStringFixed extends ValueString {
     }
 
     private static String trimRight(String s) {
+        return trimRight(s, 0);
+    }
+    private static String trimRight(String s, int minLength) {
         int endIndex = s.length() - 1;
         int i = endIndex;
-        while (i >= 0 && s.charAt(i) == ' ') {
+        while (i >= minLength && s.charAt(i) == ' ') {
             i--;
         }
         s = i == endIndex ? s : s.substring(0, i + 1);
         return s;
     }
 
+    private static String rightPadWithSpaces(String s, int length) {
+        int pad = length - s.length();
+        if (pad <= 0) {
+            return s;
+        }
+        char[] res = new char[length];
+        s.getChars(0, s.length(), res, 0);
+        Arrays.fill(res, s.length(), length, ' ');
+        return new String(res);
+    }
+
     @Override
     public int getType() {
         return Value.STRING_FIXED;
@@ -42,7 +71,42 @@ public class ValueStringFixed extends ValueString {
      * @return the value
      */
     public static ValueStringFixed get(String s) {
-        s = trimRight(s);
+        // Use the special precision constant PRECISION_TRIM to indicate
+        // default H2 behaviour of trimming the value.
+        return get(s, PRECISION_TRIM, null);
+    }
+
+    /**
+     * Get or create a fixed length string value for the given string.
+     * <p>
+     * This method will use a {@link Mode}-specific conversion when <code>mode</code> is not <code>null</code>.
+     * Otherwise it will use the default H2 behaviour of trimming the given string if <code>precision</code>
+     * is not {@link #PRECISION_DO_NOT_TRIM}.
+     *
+     * @param s the string
+     * @param precision if the {@link Mode#padFixedLengthStrings} indicates that strings should be padded, this
+     *        defines the overall length of the (potentially padded) string.
+     *        If the special constant {@link #PRECISION_DO_NOT_TRIM} is used the value will not be trimmed.
+     * @return the value
+     */
+    public static ValueStringFixed get(String s, int precision, Mode mode) {
+        // Should fixed strings be padded?
+        if (mode != null && mode.padFixedLengthStrings) {
+            if (precision == Integer.MAX_VALUE) {
+                // CHAR without a length specification is identical to CHAR(1)
+                precision = 1;
+            }
+            if (s.length() < precision) {
+                // We have to pad
+                s = rightPadWithSpaces(s, precision);
+            } else {
+                // We should trim, because inserting 'A   ' into a CHAR(1) is possible!
+                s = trimRight(s, precision);
+            }
+        } else if (precision != PRECISION_DO_NOT_TRIM) {
+            // Default behaviour of H2
+            s = trimRight(s);
+        }
         if (s.length() == 0) {
             return EMPTY;
         }

h2/src/test/org/h2/test/db/TestCompatibility.java

@@ -245,6 +245,34 @@ public class TestCompatibility extends TestBase {
 
         assertResult("ABC", stat, "SELECT SUBSTRING('ABCDEF' FOR 3)");
         assertResult("ABCD", stat, "SELECT SUBSTRING('0ABCDEF' FROM 2 FOR 4)");
+
+        /* Test right-padding of CHAR(N) at INSERT */
+        stat.execute("CREATE TABLE TEST(CH CHAR(10))");
+        stat.execute("INSERT INTO TEST (CH) VALUES ('Hello')");
+        assertResult("Hello     ", stat, "SELECT CH FROM TEST");
+
+        /* Test that WHERE clauses accept unpadded values and will pad before comparison */
+        assertResult("Hello     ", stat, "SELECT CH FROM TEST WHERE CH = 'Hello'");
+
+        /* Test CHAR which is identical to CHAR(1) */
+        stat.execute("DROP TABLE IF EXISTS TEST");
+        stat.execute("CREATE TABLE TEST(CH CHAR)");
+        stat.execute("INSERT INTO TEST (CH) VALUES ('')");
+        assertResult(" ", stat, "SELECT CH FROM TEST");
+        assertResult(" ", stat, "SELECT CH FROM TEST WHERE CH = ''");
+
+        /* Test that excessive spaces are trimmed */
+        stat.execute("DELETE FROM TEST");
+        stat.execute("INSERT INTO TEST (CH) VALUES ('1   ')");
+        assertResult("1", stat, "SELECT CH FROM TEST");
+        assertResult("1", stat, "SELECT CH FROM TEST WHERE CH = '1      '");
+
+        /* Test that we do not trim too far */
+        stat.execute("DROP TABLE IF EXISTS TEST");
+        stat.execute("CREATE TABLE TEST(CH CHAR(2))");
+        stat.execute("INSERT INTO TEST (CH) VALUES ('1   ')");
+        assertResult("1 ", stat, "SELECT CH FROM TEST");
+        assertResult("1 ", stat, "SELECT CH FROM TEST WHERE CH = '1      '");
     }
 
     private void testMySQL() throws SQLException {