MaxGekk commented on a change in pull request #28639:
URL: https://github.com/apache/spark/pull/28639#discussion_r430240104



##########
File path: 
sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java
##########
@@ -210,6 +212,17 @@ private static Row createRecordSpark22000Row(Long index) {
     return new GenericRow(values);
   }
 
+  private static String timestampToString(Timestamp ts) {

Review comment:
       I took the code from Spark 2.4.5 
https://github.com/apache/spark/blob/v2.4.5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala#L162-L174

##########
File path: 
sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java
##########
@@ -210,6 +212,17 @@ private static Row createRecordSpark22000Row(Long index) {
     return new GenericRow(values);
   }
 
+  private static String timestampToString(Timestamp ts) {

Review comment:
       We could but if we introduce a bug to the formatter, we could catch it 
by this test.

##########
File path: 
sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java
##########
@@ -210,6 +212,17 @@ private static Row createRecordSpark22000Row(Long index) {
     return new GenericRow(values);
   }
 
+  private static String timestampToString(Timestamp ts) {

Review comment:
       One more thing, if we use the same formatter for expected and actual 
results, checking current timestamps doesn't make so much sense. In that case, 
we could test a concrete timestamp.




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to