[OPENJPA-2971] Make PostgreSQL StoreCharsAsNumbers default overridable and release-note it

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java

@@ -315,7 +315,18 @@ public class DBDictionary
     // types
     public boolean storageLimitationsFatal = false;
     public boolean storeLargeNumbersAsStrings = false;
+    /**
+     * Whether Java <code>char</code> fields are stored in numeric columns
+     * rather than as <code>CHAR</code> values. Dictionaries may set their own
+     * default and may adjust it in {@link #connectedConfiguration(Connection)}
+     * based on the detected database version; use
+     * {@link #setStoreCharsAsNumbers(boolean)} to configure a value that is
+     * guaranteed to be left untouched (OPENJPA-2971).
+     */
     public boolean storeCharsAsNumbers = true;
+    // true once the user explicitly configured StoreCharsAsNumbers; dictionaries
+    // must not auto-detect over such a value (OPENJPA-2971)
+    private boolean storeCharsAsNumbersExplicit = false;
     public boolean trimStringColumns = false;
     public boolean useGetBytesForBlobs = false;
     public boolean useSetBytesForBlobs = false;
@@ -6013,6 +6024,44 @@ public void setSupportsDelimitedIdentifiers(boolean supportsDelimitedIds) {
         supportsDelimitedIdentifiers = supportsDelimitedIds;
     }
 
+    /**
+     * Whether Java <code>char</code> fields are stored in numeric columns
+     * rather than as <code>CHAR</code> values.
+     *
+     * @see #setStoreCharsAsNumbers(boolean)
+     */
+    public boolean getStoreCharsAsNumbers() {
+        return storeCharsAsNumbers;
+    }
+
+    /**
+     * Explicitly configure whether Java <code>char</code> fields are stored
+     * in numeric columns rather than as <code>CHAR</code> values. User
+     * configuration such as
+     * <code>openjpa.jdbc.DBDictionary=postgres(StoreCharsAsNumbers=true)</code>
+     * goes through this setter. A value set this way is treated as an explicit
+     * user choice: dictionaries that auto-detect a database specific default
+     * in {@link #connectedConfiguration(Connection)} must not override it
+     * (see {@link #isStoreCharsAsNumbersExplicit()}). Programmatic
+     * configuration and subclasses should prefer this setter over assigning
+     * the public {@link #storeCharsAsNumbers} field, which dictionaries are
+     * free to adjust for the detected database version.
+     */
+    public void setStoreCharsAsNumbers(boolean storeCharsAsNumbers) {
+        this.storeCharsAsNumbers = storeCharsAsNumbers;
+        this.storeCharsAsNumbersExplicit = true;
+    }
+
+    /**
+     * Whether {@link #setStoreCharsAsNumbers(boolean)} was called, i.e.
+     * whether the user explicitly configured <code>StoreCharsAsNumbers</code>.
+     * Dictionaries auto-detecting a database specific default must respect
+     * an explicit user value.
+     */
+    protected boolean isStoreCharsAsNumbersExplicit() {
+        return storeCharsAsNumbersExplicit;
+    }
+
     /**
      * @param metaData the DatabaseMetaData to use to determine whether delimiters can be supported
      */

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java

@@ -143,6 +143,12 @@ public PostgresDictionary() {
         maxAutoAssignNameLength = 63;
         schemaCase = SCHEMA_CASE_LOWER;
         rangePosition = RANGE_POST_LOCK;
+        // Modern PostgreSQL stores Java char fields natively as CHAR; only
+        // PostgreSQL < 9 falls back to numeric storage in
+        // connectedConfiguration. Being the dictionary default, a value
+        // assigned by a subclass constructor or via the public field before
+        // connecting is never overwritten on PostgreSQL 9+ (OPENJPA-2971).
+        storeCharsAsNumbers = false;
         requiresAliasForSubselect = true;
         allowsAliasInBulkClause = false;
 
@@ -1056,11 +1062,16 @@ public void connectedConfiguration(Connection conn) throws SQLException {
             searchStringEscape = "\\\\";
         }
 
-        // Modern PostgreSQL supports native CHAR storage; only the older
-        // releases required the DBDictionary default (chars stored as
-        // numeric values).
-        if (maj >= 9) {
-            storeCharsAsNumbers = false;
+        // Only PostgreSQL releases before 9 (or an undetectable version)
+        // require the generic DBDictionary behaviour of storing chars as
+        // numeric values; the dictionary default (see constructor) is native
+        // CHAR storage. Never touch an explicit user setting (OPENJPA-2971):
+        // schemas created by earlier OpenJPA releases (INTEGER columns for
+        // char fields) keep working with StoreCharsAsNumbers=true, and a
+        // value assigned to the public field is only ever changed here on
+        // PostgreSQL < 9.
+        if (maj < 9 && !isStoreCharsAsNumbersExplicit()) {
+            storeCharsAsNumbers = true;
         }
     }
 

openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/sql/TestPostgresDictionary.java

@@ -0,0 +1,256 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.openjpa.jdbc.sql;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+
+import javax.sql.DataSource;
+
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl;
+import org.apache.openjpa.lib.conf.Configurations;
+import org.junit.Test;
+
+/**
+ * Tests for {@link PostgresDictionary}, in particular the version dependent
+ * <code>StoreCharsAsNumbers</code> default and that an explicit user setting
+ * survives {@link DBDictionary#connectedConfiguration(Connection)} (OPENJPA-2971).
+ */
+public class TestPostgresDictionary {
+
+    @Test
+    public void testStoreCharsAsNumbersDefaultsToFalseOnModernPostgres() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary(null);
+        dict.connectedConfiguration(connection(16, 0));
+        assertFalse(dict.storeCharsAsNumbers);
+        assertFalse(dict.getStoreCharsAsNumbers());
+    }
+
+    @Test
+    public void testStoreCharsAsNumbersDefaultsToTrueOnLegacyPostgres() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary(null);
+        dict.connectedConfiguration(connection(8, 4));
+        assertTrue(dict.storeCharsAsNumbers);
+        assertTrue(dict.getStoreCharsAsNumbers());
+    }
+
+    @Test
+    public void testExplicitStoreCharsAsNumbersSurvivesConnectedConfiguration() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary("StoreCharsAsNumbers=true");
+        assertTrue(dict.storeCharsAsNumbers);
+        dict.connectedConfiguration(connection(16, 0));
+        assertTrue("explicit StoreCharsAsNumbers=true must not be overridden on PostgreSQL 16",
+            dict.storeCharsAsNumbers);
+    }
+
+    @Test
+    public void testExplicitStoreCharsAsNumbersFalseSurvivesOnLegacyPostgres() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary("StoreCharsAsNumbers=false");
+        assertFalse(dict.storeCharsAsNumbers);
+        dict.connectedConfiguration(connection(8, 4));
+        assertFalse("explicit StoreCharsAsNumbers=false must not be overridden on PostgreSQL 8.4",
+            dict.storeCharsAsNumbers);
+    }
+
+    @Test
+    public void testSetterMarksValueAsExplicit() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary(null);
+        assertFalse(dict.isStoreCharsAsNumbersExplicit());
+        dict.setStoreCharsAsNumbers(true);
+        assertTrue(dict.isStoreCharsAsNumbersExplicit());
+        dict.connectedConfiguration(connection(16, 0));
+        assertTrue(dict.storeCharsAsNumbers);
+    }
+
+    @Test
+    public void testDictionaryDefaultIsNativeCharStorageBeforeConnecting() {
+        assertFalse(new PostgresDictionary().storeCharsAsNumbers);
+        assertFalse(new PostgresDictionary().isStoreCharsAsNumbersExplicit());
+    }
+
+    @Test
+    public void testProgrammaticPublicFieldAssignmentSurvivesOnModernPostgres() throws Exception {
+        PostgresDictionary dict = newConfiguredDictionary(null);
+        dict.storeCharsAsNumbers = true;
+        dict.connectedConfiguration(connection(16, 0));
+        assertTrue("storeCharsAsNumbers=true assigned via the public field must survive on PostgreSQL 16",
+            dict.storeCharsAsNumbers);
+    }
+
+    @Test
+    public void testSubclassConstructorAssignmentSurvivesOnModernPostgres() throws Exception {
+        PostgresDictionary dict = new PostgresDictionary() {
+            {
+                storeCharsAsNumbers = true;
+            }
+        };
+        Configurations.configureInstance(dict, new JDBCConfigurationImpl(), (String) null, "DBDictionary");
+        dict.connectedConfiguration(connection(16, 0));
+        assertTrue("storeCharsAsNumbers=true assigned by a subclass constructor must survive on PostgreSQL 16",
+            dict.storeCharsAsNumbers);
+    }
+
+    @Test
+    public void testExplicitStoreCharsAsNumbersThroughDBDictionaryFactory() throws Exception {
+        JDBCConfiguration conf = new JDBCConfigurationImpl();
+        DataSource ds = dataSource(connection(16, 0));
+
+        DBDictionary explicit = DBDictionaryFactory.newDBDictionary(conf, ds, "StoreCharsAsNumbers=true");
+        assertTrue(explicit instanceof PostgresDictionary);
+        assertTrue("explicit StoreCharsAsNumbers=true must survive DBDictionaryFactory", explicit.storeCharsAsNumbers);
+
+        DBDictionary auto = DBDictionaryFactory.newDBDictionary(conf, ds, null);
+        assertTrue(auto instanceof PostgresDictionary);
+        assertFalse("PostgreSQL 16 must default to native CHAR storage", auto.storeCharsAsNumbers);
+    }
+
+    /**
+     * Creates a dictionary configured the same way user supplied
+     * <code>openjpa.jdbc.DBDictionary=postgres(...)</code> plugin properties are applied.
+     */
+    private static PostgresDictionary newConfiguredDictionary(String props) {
+        PostgresDictionary dict = new PostgresDictionary();
+        Configurations.configureInstance(dict, new JDBCConfigurationImpl(), props, "DBDictionary");
+        return dict;
+    }
+
+    private static DataSource dataSource(final Connection conn) {
+        return (DataSource) Proxy.newProxyInstance(TestPostgresDictionary.class.getClassLoader(),
+            new Class<?>[] { DataSource.class }, new DefaultHandler() {
+                @Override
+                protected Object handle(Method method, Object[] args) {
+                    if ("getConnection".equals(method.getName())) {
+                        return conn;
+                    }
+                    return null;
+                }
+            });
+    }
+
+    /**
+     * A {@link Connection} whose {@link DatabaseMetaData} reports the given
+     * PostgreSQL version and sensible defaults for everything else.
+     */
+    private static Connection connection(final int major, final int minor) {
+        final DatabaseMetaData metaData = (DatabaseMetaData) Proxy.newProxyInstance(
+            TestPostgresDictionary.class.getClassLoader(), new Class<?>[] { DatabaseMetaData.class },
+            new DefaultHandler() {
+                @Override
+                protected Object handle(Method method, Object[] args) {
+                    switch (method.getName()) {
+                        case "getDatabaseProductName":
+                            return "PostgreSQL";
+                        case "getDatabaseProductVersion":
+                            return major + "." + minor;
+                        case "getDatabaseMajorVersion":
+                            return major;
+                        case "getDatabaseMinorVersion":
+                            return minor;
+                        case "getJDBCMajorVersion":
+                            return 4;
+                        case "getJDBCMinorVersion":
+                            return 2;
+                        case "getDriverName":
+                            return "PostgreSQL JDBC Driver";
+                        case "getDriverVersion":
+                            return "42.7";
+                        case "getIdentifierQuoteString":
+                            return "\"";
+                        case "getURL":
+                            return "jdbc:postgresql://localhost/test";
+                        default:
+                            return null;
+                    }
+                }
+            });
+        return (Connection) Proxy.newProxyInstance(TestPostgresDictionary.class.getClassLoader(),
+            new Class<?>[] { Connection.class }, new DefaultHandler() {
+                @Override
+                protected Object handle(Method method, Object[] args) {
+                    if ("getMetaData".equals(method.getName())) {
+                        return metaData;
+                    }
+                    return null;
+                }
+            });
+    }
+
+    /**
+     * Invocation handler returning type appropriate defaults (false / 0 / "" / null)
+     * for every method unless {@link #handle(Method, Object[])} provides a value.
+     */
+    private abstract static class DefaultHandler implements InvocationHandler {
+
+        @Override
+        public Object invoke(Object proxy, Method method, Object[] args) {
+            switch (method.getName()) {
+                case "toString":
+                    return "proxy";
+                case "hashCode":
+                    return System.identityHashCode(proxy);
+                case "equals":
+                    return proxy == args[0];
+                default:
+                    break;
+            }
+            Object value = handle(method, args);
+            return value != null ? value : defaultValue(method.getReturnType());
+        }
+
+        protected abstract Object handle(Method method, Object[] args);
+
+        private static Object defaultValue(Class<?> type) {
+            if (type == boolean.class) {
+                return Boolean.FALSE;
+            }
+            if (type == int.class) {
+                return 0;
+            }
+            if (type == long.class) {
+                return 0L;
+            }
+            if (type == short.class) {
+                return (short) 0;
+            }
+            if (type == byte.class) {
+                return (byte) 0;
+            }
+            if (type == float.class) {
+                return 0f;
+            }
+            if (type == double.class) {
+                return 0d;
+            }
+            if (type == char.class) {
+                return (char) 0;
+            }
+            if (type == String.class) {
+                return "";
+            }
+            return null;
+        }
+    }
+}

openjpa-project/src/doc/manual/migration_considerations.xml

@@ -647,6 +647,25 @@
                     <literal>DBDictionary.supportsQueryTimeout</literal>
                 </para>
             </section>
+            <section id="jpa_3.2_PostgreSqlStoreCharsAsNumbers">
+                <title>PostgreSQL stores char fields as CHAR by default</title>
+                <para>
+                    Previous releases stored Java <literal>char</literal> fields in numeric
+                    (<literal>INTEGER</literal>) columns on PostgreSQL, following the generic
+                    <literal>DBDictionary.StoreCharsAsNumbers=true</literal> default.
+                    Starting with this release, <literal>StoreCharsAsNumbers</literal> defaults to
+                    <literal>false</literal> on PostgreSQL 9 and later, so <literal>char</literal>
+                    fields are mapped to <literal>CHAR</literal> columns.
+                </para>
+                <para>
+                    Existing schemas created by earlier OpenJPA releases must either migrate the
+                    affected columns to <literal>CHAR</literal> or restore the previous behaviour by
+                    setting the property explicitly, e.g.
+                    <literal>openjpa.jdbc.DBDictionary=postgres(StoreCharsAsNumbers=true)</literal>.
+                    An explicitly configured value is always respected and never overridden by the
+                    PostgreSQL version detection.
+                </para>
+            </section>
         </section>
     </section>
 

openjpa-project/src/doc/manual/ref_guide_dbsetup.xml

@@ -2577,7 +2577,12 @@ a warning.
                     </indexterm>
 <literal>StoreCharsAsNumbers</literal>: Set this property to <literal>false
 </literal> to store Java <literal>char</literal> fields as <literal>CHAR
-</literal> values rather than numbers. Defaults to <literal>true</literal>.
+</literal> values rather than numbers. Defaults to <literal>true</literal>,
+except on DB2 and on PostgreSQL 9 and later, where the default is
+<literal>false</literal> (native <literal>CHAR</literal> storage). An
+explicitly configured value is always respected. Schemas created by earlier
+OpenJPA releases on PostgreSQL store <literal>char</literal> fields in numeric
+columns and require <literal>StoreCharsAsNumbers=true</literal>.
                     </para>
                 </listitem>
                 <listitem id="DBDictionary.StoreLargeNumbersAsStrings">