Merge pull request #201 from bug/REL1_5_STABLE/issue199

Fix a regression in 1.5.1 that was not caught in pre-release testing,
and could leave conversions between PostgreSQL date and java.sql.Date
off by one day in certain timezones and times of the year (#199).

Other issues in date/time conversion have also been uncovered that are
of longer standing, not recent regressions. They are detailed in #200
and are not fixed in this PR, but can be addressed in another, later
release.

pljava-examples/src/main/java/org/postgresql/pljava/example/annotation/PreJSR310.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2018- Tada AB and other contributors, as listed below.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the The BSD 3-Clause License
+ * which accompanies this distribution, and is available at
+ * http://opensource.org/licenses/BSD-3-Clause
+ *
+ * Contributors:
+ *   Chapman Flack
+ */
+package org.postgresql.pljava.example.annotation;
+
+import java.sql.Connection;
+import java.sql.Date;
+import java.sql.DriverManager;
+import java.sql.Statement;
+import java.sql.ResultSet;
+import java.sql.Savepoint;
+import java.sql.SQLException;
+
+import static java.util.logging.Logger.getAnonymousLogger;
+import java.util.TimeZone;
+
+import org.postgresql.pljava.annotation.Function;
+import org.postgresql.pljava.annotation.SQLAction;
+
+/**
+ * Some tests of pre-JSR 310 date/time/timestamp conversions.
+ *<p>
+ * For now, just {@code java.sql.Date}, thanks to issue #199.
+ *<p>
+ * This example relies on {@code implementor} tags reflecting the PostgreSQL
+ * version, set up in the {@link ConditionalDDR} example.
+ */
+@SQLAction(implementor="postgresql_ge_90300", // needs LATERAL
+	requires="issue199", install={
+	"SELECT javatest.issue199()"
+})
+public class PreJSR310
+{
+	private static final String TZPRAGUE = "Europe/Prague";
+
+	/**
+	 * Test for a regression in PG date to/from java.sql.Date conversion
+	 * identified in issue #199.
+	 *<p>
+	 * Checks that two months of consecutive dates in October/November 2018
+	 * are converted correctly in the Europe/Prague timezone. The actual issue
+	 * was by no means limited to that timezone, but this test reproducibly
+	 * detects it.
+	 */
+	@Function(schema="javatest", provides="issue199")
+	public static void issue199() throws SQLException
+	{
+		TimeZone oldZone = TimeZone.getDefault();
+		TimeZone tzPrague = TimeZone.getTimeZone(TZPRAGUE);
+		Connection c = DriverManager.getConnection("jdbc:default:connection");
+		Statement s = c.createStatement();
+		Savepoint svpt = c.setSavepoint();
+		boolean ok = true;
+		try
+		{
+			TimeZone.setDefault(tzPrague);
+			s.execute("SET LOCAL TIME ZONE '" + TZPRAGUE + "'");
+
+			ResultSet rs = s.executeQuery(
+				"SELECT" +
+				"  d, to_char(d, 'YYYY-MM-DD')" +
+				" FROM" +
+				"  generate_series(0, 60) AS s(i)," +
+				"  LATERAL (SELECT date '2018-10-01' + i) AS t(d)");
+			while ( rs.next() )
+			{
+				Date dd = rs.getDate(1);
+				String ds = rs.getString(2);
+				if ( ! ds.equals(dd.toString()) )
+					ok = false;
+			}
+		}
+		finally
+		{
+			TimeZone.setDefault(oldZone);
+			c.rollback(svpt); // restore prior PG timezone
+			s.close();
+			c.close();
+		}
+
+		if ( ok )
+			getAnonymousLogger().info("issue 199 test ok");
+		else
+			getAnonymousLogger().warning("issue 199 test not ok");
+	}
+}

pljava-so/src/main/c/type/Date.c

@@ -95,8 +95,8 @@ static Type _LocalDate_obtain(Oid typeId)
 static jvalue _Date_coerceDatum(Type self, Datum arg)
 {
 	DateADT pgDate = DatumGetDateADT(arg);
-	int64 ts = (int64)pgDate * INT64CONST(86400000000);
-	int   tz = Timestamp_getTimeZone_id(ts);
+	int64 ts = (int64)pgDate * INT64CONST(43200000000);
+	int   tz = Timestamp_getTimeZone_id(ts); /* ts in 2 usec units */
 
 	jlong date = (jlong)(pgDate + EPOCH_DIFF);
 
@@ -109,8 +109,12 @@ static jvalue _Date_coerceDatum(Type self, Datum arg)
 
 static Datum _Date_coerceObject(Type self, jobject date)
 {
-	jlong milliSecs = JNI_callLongMethod(date, s_Date_getTime) - INT64CONST(86400000) * EPOCH_DIFF;
-	jlong secs = milliSecs / 1000 - Timestamp_getTimeZone_id(milliSecs * 1000);
+	jlong milliSecs =
+		JNI_callLongMethod(date, s_Date_getTime)
+		- INT64CONST(86400000) * EPOCH_DIFF;
+	jlong secs =
+		milliSecs / 1000
+		- Timestamp_getTimeZone_id(milliSecs * 500); /* those 2 usec units */
 	return DateADTGetDatum((DateADT)(secs / 86400));
 }
 

src/site/markdown/releasenotes.md.vm

@@ -10,7 +10,34 @@
 #set($ghbug = 'https://github.com/tada/pljava/issues/')
 #set($ghpull = 'https://github.com/tada/pljava/pull/')
 
-$h2 PL/Java 1.5.1
+$h2 PL/Java 1.5.2
+
+A pure bug-fix release, correcting a regression in 1.5.1 that was not caught
+in pre-release testing, and could leave
+[conversions between PostgreSQL `date` and `java.sql.Date`](${ghbug}199) off
+by one day in certain timezones and times of the year.
+
+1.5.1 added support for the newer `java.time` classes from JSR 310 / JDBC 4.2,
+which are [recommended as superior alternatives](use/datetime.html) to the
+older conversions involving `java.sql.Date` and related classes. The new
+versions are superior in part because they do not have hidden timezone
+dependencies.
+
+However, the change to the historical `java.sql.Date` conversion behavior was
+inadvertent, and is fixed in this release.
+
+$h3 Open issues with date/time/timestamp conversions
+
+During preparation of this release, other issues of longer standing were also
+uncovered in the legacy conversions between PG `date`, `time`, and
+`timestamp` classes and the `java.sql` types. They are detailed in
+[issue #200](${ghbug}200). Because they are not regressions but long-established
+behavior, they are left untouched in this release, and will be fixed in
+a future release.
+
+The Java 8 `java.time` conversions are free of these issues as well.
+
+$h2 PL/Java 1.5.1 (17 October 2018)
 
 This release adds support for PostgreSQL 9.6, 10, and 11,
 and plays more nicely with `pg_upgrade`. If a PostgreSQL installation

src/site/markdown/use/datetime.md

@@ -26,6 +26,14 @@ zone`. The conversions of non-zoned values involve a hidden dependency on the
 PostgreSQL session's current setting of `TimeZone`, which can vary from session
 to session at the connecting client's preference.
 
+There are known issues of long standing in PL/Java's conversions to and from
+these types, detailed in [issue #200][issue200]. While these particular issues
+are expected to be fixed in a future PL/Java release, the Java 8 / JDBC 4.2
+mappings described next are the strongly-recommended alternative to the legacy
+mappings, avoiding these issues entirely.
+
+[issue200]: https://github.com/tada/pljava/issues/200
+
 ## Java 8 / JDBC 4.2 date/time mappings
 
 Java 8 introduced the much improved set of date/time classes in the `java.time`