leerssej / google-refine

Automatically exported from code.google.com/p/google-refine
Other
0 stars 0 forks source link

Diff for dates fails with "unknown error" always #99

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create a gridworks project with two date columns, possibly by parsing 
explicitly
     string values with something like: toDate(value, "EEE MMM d HH:mm:ss Z yyyy")
     assume the columns are called date1 and date2
2. Add a third column to be the diff between these two date columns:
     diff(cells["date1"]value, cells["date2"].value, "hours")

What is the expected output? What do you see instead?

I expect to see a numeric value indicating the difference between the two dates.
Instead I see emptiness.

What version of the product are you using? On what operating system?

Freebase on Mac OSX 10.6
SVN revision 1024.

Please provide any additional information below.

I believe the issue is that the diff function does not at all expect dates.
For some reason it expects java.util.Calendar objects.  I don't know how
to create such objects in any column.  The "toDate" function produces
java.util.Date objects.  

I suspect this has never worked, because there is a genuine bug in there
as well.  The third parameter is picked up using array index 3 which is
out of bounds.  Array index 2 is the third parameter.

If I change the "Diff" function according to the diff output below everything
works much better for my use case.  Maybe I shouldn't eliminate the mention
of java.util.Calendar altogether, but I don't see how it would ever come into
play.

Index: main/src/com/metaweb/gridworks/expr/functions/strings/Diff.java
===================================================================
--- main/src/com/metaweb/gridworks/expr/functions/strings/Diff.java (revision 
1024)
+++ main/src/com/metaweb/gridworks/expr/functions/strings/Diff.java (working 
copy)
@@ -1,6 +1,6 @@
 package com.metaweb.gridworks.expr.functions.strings;

-import java.util.Calendar;
+import java.util.Date;
 import java.util.Properties;

 import org.apache.commons.lang.StringUtils;
@@ -20,14 +20,14 @@
             if (o1 != null && o2 != null) {
                 if (o1 instanceof String && o2 instanceof String) {
                     return StringUtils.difference((String) o1,(String) o2);
-                } else if (o1 instanceof Calendar && args.length == 3) {
-                    Object o3 = args[3];
+                } else if (o1 instanceof Date && args.length == 3) {
+                    Object o3 = args[2];
                     if (o3 != null && o3 instanceof String) {
                         try {
                             String unit = ((String) o3).toLowerCase();
-                            Calendar c1 = (Calendar) o1;
-                            Calendar c2 = (o2 instanceof Calendar) ? 
(Calendar) o2 : CalendarParser.parse((o2 instanceof String) ? (String) o2 : 
o2.toString());
-                            long delta = (c1.getTimeInMillis() - 
c2.getTimeInMillis()) / 1000;
+                            Date c1 = (Date) o1;
+                            Date c2 = (o2 instanceof Date) ? (Date) o2 : 
CalendarParser.parse((o2 instanceof String) ? (String) o2 : 
o2.toString()).getTime();
+                            long delta = (c1.getTime() - c2.getTime()) / 1000;
                             if ("seconds".equals(unit)) return delta;
                             delta /= 60;
                             if ("minutes".equals(unit)) return delta;

Original issue reported on code.google.com by knut.for...@gmail.com on 23 Jun 2010 at 7:16

GoogleCodeExporter commented 9 years ago
applied to trunk (and retained the ability to work on Calendar as well since 
there can be both)

Original comment by stefa...@google.com on 31 Aug 2010 at 7:13

GoogleCodeExporter commented 9 years ago
Thanks for fixing this, however from what I can tell the array bounds problem 
is still there as of revision 1229
Line 25 says             Object o3 = args[3];
which is outside the array, the intention must be     args[2]   instead 

Original comment by knut.for...@gmail.com on 31 Aug 2010 at 10:51

GoogleCodeExporter commented 9 years ago
d'oh, that's what I get for applying patches by hand.

Fixed in r1250.

Original comment by stefa...@google.com on 31 Aug 2010 at 11:25

GoogleCodeExporter commented 9 years ago

Original comment by tfmorris on 18 Sep 2012 at 2:58