You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/08/06 19:54:50 UTC
[GitHub] [phoenix] joshelser commented on a change in pull request #1284: PHOENIX-6486 Phoenix uses inconsistent chronologies internally, breaking pre-Gregorian date handling
joshelser commented on a change in pull request #1284:
URL: https://github.com/apache/phoenix/pull/1284#discussion_r684467588
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java
##########
@@ -1939,6 +1941,99 @@ public void testDateFormatTimeZone()throws Exception {
}
}
+ private String getFormattedDate(List<String> dateList) {
+ return String.join("-", dateList.subList(0, 3)) + " "
+ + String.join(":", dateList.subList(3, 6)) + "." + dateList.get(6);
+ }
+
+ @Test
+ public void testAncientDates() throws Exception {
+
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ Connection conn = DriverManager.getConnection(url, props);
+ Statement stmt = conn.createStatement();
+ String tableName = generateUniqueName();
+
+ List<String> date1list = Arrays.asList("0010", "10", "10", "10", "10", "10", "111");
+ List<String> date2list = Arrays.asList("1000", "02", "03", "04", "05", "06", "000");
+ List<String> date3list = Arrays.asList("0001", "12", "31", "23", "59", "59", "000");
+ List<List<String>> dateLists = Arrays.asList(date1list, date2list, date3list, date2list);
+
+ String date1 = getFormattedDate(date1list); // 0010-10-10 10:10:10.111
+ String date2 = getFormattedDate(date2list); // 1000-02-03 04:05:06.000
+ String date3 = getFormattedDate(date3list); // 0001-12-31 23:59:59.000
+ List<String> dates = Arrays.asList(date1, date2, date3, date2);
+
+
+ //TO_DATE('Sat, 3 Feb 2008 03:05:06 GMT', 'EEE, d MMM yyyy HH:mm:ss z', 'UTC')
+ stmt.execute("CREATE TABLE " + tableName + " ( id INTEGER not null PRIMARY KEY," +
+ " date DATE, time TIME, timestamp TIMESTAMP)");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(1, TO_DATE('" + date1
+ + "'), TO_TIME('" + date1 + "'), TO_TIMESTAMP('" + date1 + "'))");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(2, TO_DATE('" + date2
+ + "'), TO_TIME('" + date2 + "'), TO_TIMESTAMP('" + date2 + "'))");
+
+ String f = " GMT', 'yyyy-MM-dd HH:mm:ss.SSS z', 'UTC";
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(4, TO_DATE('" + date2 + f
+ + "'), TO_TIME('" + date2 + f + "'), TO_TIMESTAMP('" + date2 + f + "'))");
Review comment:
nit: could these be PreparedStatements too?
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java
##########
@@ -1939,6 +1941,99 @@ public void testDateFormatTimeZone()throws Exception {
}
}
+ private String getFormattedDate(List<String> dateList) {
+ return String.join("-", dateList.subList(0, 3)) + " "
+ + String.join(":", dateList.subList(3, 6)) + "." + dateList.get(6);
+ }
+
+ @Test
+ public void testAncientDates() throws Exception {
+
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ Connection conn = DriverManager.getConnection(url, props);
+ Statement stmt = conn.createStatement();
+ String tableName = generateUniqueName();
+
+ List<String> date1list = Arrays.asList("0010", "10", "10", "10", "10", "10", "111");
+ List<String> date2list = Arrays.asList("1000", "02", "03", "04", "05", "06", "000");
+ List<String> date3list = Arrays.asList("0001", "12", "31", "23", "59", "59", "000");
+ List<List<String>> dateLists = Arrays.asList(date1list, date2list, date3list, date2list);
+
+ String date1 = getFormattedDate(date1list); // 0010-10-10 10:10:10.111
+ String date2 = getFormattedDate(date2list); // 1000-02-03 04:05:06.000
+ String date3 = getFormattedDate(date3list); // 0001-12-31 23:59:59.000
+ List<String> dates = Arrays.asList(date1, date2, date3, date2);
+
+
+ //TO_DATE('Sat, 3 Feb 2008 03:05:06 GMT', 'EEE, d MMM yyyy HH:mm:ss z', 'UTC')
+ stmt.execute("CREATE TABLE " + tableName + " ( id INTEGER not null PRIMARY KEY," +
+ " date DATE, time TIME, timestamp TIMESTAMP)");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(1, TO_DATE('" + date1
+ + "'), TO_TIME('" + date1 + "'), TO_TIMESTAMP('" + date1 + "'))");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(2, TO_DATE('" + date2
+ + "'), TO_TIME('" + date2 + "'), TO_TIMESTAMP('" + date2 + "'))");
+
+ String f = " GMT', 'yyyy-MM-dd HH:mm:ss.SSS z', 'UTC";
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(4, TO_DATE('" + date2 + f
+ + "'), TO_TIME('" + date2 + f + "'), TO_TIMESTAMP('" + date2 + f + "'))");
+
+ PreparedStatement pstmt = conn.prepareStatement("UPSERT INTO " + tableName + " values (?, ?, ?, ?)");
+ pstmt.setInt(1, 3);
+ Timestamp t = new Timestamp(DateUtil.parseDate(date3).getTime());
+ pstmt.setDate(2, new Date(t.getTime()));
+ pstmt.setTime(3, new Time(t.getTime()));
+ pstmt.setTimestamp(4, t);
+ pstmt.execute();
+ conn.commit();
+
+ ResultSet rs = stmt.executeQuery("SELECT * FROM " + tableName + " ORDER BY id");
+
+ int assertions = 0;
+ for (int i = 0; i < dates.size(); i++){
+ assertTrue(rs.next());
+ String actualDate = dates.get(i);
+ Timestamp parseTimestamp = DateUtil.parseTimestamp(actualDate);
+
+ assertEquals(i + 1, rs.getInt(1));
+ Date expectedDate = DateUtil.getSQLDateInISOChronology(parseTimestamp);
+ assertEquals(expectedDate, rs.getDate(2));
+ assertEquals(rs.getDate(2), rs.getObject(2));
+ assertEquals(actualDate, rs.getString(2));
+
+ Time expectedTime = DateUtil.getSQLTimeInISOChronology(parseTimestamp);
+ assertEquals(new Timestamp(expectedTime.getTime()), new Timestamp(rs.getTime(3).getTime()));
+ assertEquals(expectedTime, rs.getTime(3));
+ assertEquals(rs.getTime(3), rs.getObject(3));
+ assertEquals(actualDate, rs.getString(3));
+
+ Timestamp expectedTimestamp = DateUtil.getSQLTimestampInISOChronology(parseTimestamp);
+ assertEquals(expectedTimestamp, rs.getTimestamp(4));
+ assertEquals(rs.getTimestamp(4), rs.getObject(4));
+ assertEquals(actualDate, rs.getString(4));
+ assertions++;
+ }
+
+ String query = "SELECT year(timestamp), month(timestamp), dayofmonth(timestamp),"
+ + " hour(timestamp), minute(timestamp), second(timestamp) FROM " + tableName
+ + " ORDER BY id";
+ rs = stmt.executeQuery(query);
+
+ for (int i = 0; i < dates.size(); i++){
+ assertTrue(rs.next());
+ for (int j = 0; j < 6; j++){
+ int expected = Integer.parseInt(dateLists.get(i).get(j));
+ int value = rs.getInt(j + 1);
+ String readFunc = query.split("\\s+")[j + 1];
+ assertTrue("Expected for " + readFunc.substring(0, readFunc.length()-1) + ": " + expected + ", got: " + value,
+ expected == value);
+ assertions++;
+ }
+ }
+ assertEquals(assertions, 28);
Review comment:
nit: make this `dates.size() *7`? Not sure if there's a lot of value directly in this assertion either way.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/DateUtil.java
##########
@@ -190,6 +193,68 @@ private static long parseDateTime(String dateTimeValue) {
return ISODateFormatParser.getInstance().parseDateTime(dateTimeValue);
}
+ private static long getLongInJulianChrono(long ts) {
+ DateTime dt1 = new DateTime(ts, GJChronology.getInstanceUTC());
+ DateTime dt2 = new DateTime(ts, ISOChronology.getInstanceUTC());
+ Date d1 = parseDate(dt1.toString());
+ Date d2 = parseDate(dt2.toString());
+ long l1 = d1.getTime();
+ long l2 = d2.getTime();
+ return (ts + (l1 - l2));
Review comment:
Any chance you could include a description as to how/why this math works?
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java
##########
@@ -1939,6 +1941,99 @@ public void testDateFormatTimeZone()throws Exception {
}
}
+ private String getFormattedDate(List<String> dateList) {
+ return String.join("-", dateList.subList(0, 3)) + " "
+ + String.join(":", dateList.subList(3, 6)) + "." + dateList.get(6);
+ }
+
+ @Test
+ public void testAncientDates() throws Exception {
+
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ Connection conn = DriverManager.getConnection(url, props);
+ Statement stmt = conn.createStatement();
+ String tableName = generateUniqueName();
+
+ List<String> date1list = Arrays.asList("0010", "10", "10", "10", "10", "10", "111");
+ List<String> date2list = Arrays.asList("1000", "02", "03", "04", "05", "06", "000");
+ List<String> date3list = Arrays.asList("0001", "12", "31", "23", "59", "59", "000");
+ List<List<String>> dateLists = Arrays.asList(date1list, date2list, date3list, date2list);
+
+ String date1 = getFormattedDate(date1list); // 0010-10-10 10:10:10.111
+ String date2 = getFormattedDate(date2list); // 1000-02-03 04:05:06.000
+ String date3 = getFormattedDate(date3list); // 0001-12-31 23:59:59.000
+ List<String> dates = Arrays.asList(date1, date2, date3, date2);
+
+
+ //TO_DATE('Sat, 3 Feb 2008 03:05:06 GMT', 'EEE, d MMM yyyy HH:mm:ss z', 'UTC')
+ stmt.execute("CREATE TABLE " + tableName + " ( id INTEGER not null PRIMARY KEY," +
+ " date DATE, time TIME, timestamp TIMESTAMP)");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(1, TO_DATE('" + date1
+ + "'), TO_TIME('" + date1 + "'), TO_TIMESTAMP('" + date1 + "'))");
+
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(2, TO_DATE('" + date2
+ + "'), TO_TIME('" + date2 + "'), TO_TIMESTAMP('" + date2 + "'))");
+
+ String f = " GMT', 'yyyy-MM-dd HH:mm:ss.SSS z', 'UTC";
+ stmt.execute("UPSERT INTO " + tableName + " VALUES(4, TO_DATE('" + date2 + f
+ + "'), TO_TIME('" + date2 + f + "'), TO_TIMESTAMP('" + date2 + f + "'))");
+
+ PreparedStatement pstmt = conn.prepareStatement("UPSERT INTO " + tableName + " values (?, ?, ?, ?)");
+ pstmt.setInt(1, 3);
+ Timestamp t = new Timestamp(DateUtil.parseDate(date3).getTime());
+ pstmt.setDate(2, new Date(t.getTime()));
+ pstmt.setTime(3, new Time(t.getTime()));
+ pstmt.setTimestamp(4, t);
+ pstmt.execute();
+ conn.commit();
+
+ ResultSet rs = stmt.executeQuery("SELECT * FROM " + tableName + " ORDER BY id");
+
+ int assertions = 0;
+ for (int i = 0; i < dates.size(); i++){
+ assertTrue(rs.next());
+ String actualDate = dates.get(i);
+ Timestamp parseTimestamp = DateUtil.parseTimestamp(actualDate);
+
+ assertEquals(i + 1, rs.getInt(1));
+ Date expectedDate = DateUtil.getSQLDateInISOChronology(parseTimestamp);
+ assertEquals(expectedDate, rs.getDate(2));
+ assertEquals(rs.getDate(2), rs.getObject(2));
+ assertEquals(actualDate, rs.getString(2));
+
+ Time expectedTime = DateUtil.getSQLTimeInISOChronology(parseTimestamp);
+ assertEquals(new Timestamp(expectedTime.getTime()), new Timestamp(rs.getTime(3).getTime()));
+ assertEquals(expectedTime, rs.getTime(3));
+ assertEquals(rs.getTime(3), rs.getObject(3));
+ assertEquals(actualDate, rs.getString(3));
+
+ Timestamp expectedTimestamp = DateUtil.getSQLTimestampInISOChronology(parseTimestamp);
+ assertEquals(expectedTimestamp, rs.getTimestamp(4));
+ assertEquals(rs.getTimestamp(4), rs.getObject(4));
+ assertEquals(actualDate, rs.getString(4));
+ assertions++;
+ }
+
+ String query = "SELECT year(timestamp), month(timestamp), dayofmonth(timestamp),"
+ + " hour(timestamp), minute(timestamp), second(timestamp) FROM " + tableName
+ + " ORDER BY id";
+ rs = stmt.executeQuery(query);
+
+ for (int i = 0; i < dates.size(); i++){
+ assertTrue(rs.next());
+ for (int j = 0; j < 6; j++){
+ int expected = Integer.parseInt(dateLists.get(i).get(j));
Review comment:
nit: IMO, would be more readable to add `List<String> dateList = dateList.get(i)` outside the inner-loop and make this `dateList.get(j)`.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/DateUtil.java
##########
@@ -190,6 +193,68 @@ private static long parseDateTime(String dateTimeValue) {
return ISODateFormatParser.getInstance().parseDateTime(dateTimeValue);
}
+ private static long getLongInJulianChrono(long ts) {
+ DateTime dt1 = new DateTime(ts, GJChronology.getInstanceUTC());
+ DateTime dt2 = new DateTime(ts, ISOChronology.getInstanceUTC());
+ Date d1 = parseDate(dt1.toString());
+ Date d2 = parseDate(dt2.toString());
+ long l1 = d1.getTime();
+ long l2 = d2.getTime();
+ return (ts + (l1 - l2));
+ }
+
+ public static Time getSQLTimeInISOChronology(Time time) {
+ return getSQLTimeInISOChronology(time.getTime(), 0);
+ }
+
+ public static Time getSQLTimeInISOChronology(Timestamp ts) {
+ return getSQLTimeInISOChronology(ts.getTime(), ts.getNanos());
+ }
+
+ public static Time getSQLTimeInISOChronology(long ts, long nanos) {
+ DateTime dt1 = new DateTime(ts, GJChronology.getInstanceUTC());
+ DateTime dt2 = new DateTime(ts, ISOChronology.getInstanceUTC());
+ Time d1 = parseTime(dt1.toString());
+ Time d2 = parseTime(dt2.toString());
+ long l1 = d1.getTime();
+ long l2 = d2.getTime();
+ return new Time(ts - (l1 - l2));
Review comment:
An explanation for this one too would be great.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org