Comment on attachment 333589 [details] [review]
patch using pragma #if HAVE_LIBDBI_R

It's not right. A correct patch would use time = dbi_result_get_as_longlong() instead of result->rows[row]->field... in the ifdef.

What is the compile error and what version of libdbi do you have?

(In reply to John Ralls from comment #2)
> What is the compile error and what version of libdbi do you have?

Sorry, I should have stated this form the start:
Linux OpenSuse 13.2
libdbi and libdbi-devel version 0.9.0.g33-2.4.1 as package version name from OpenSuse 13.2.

Into the dbi.h I see: 

/* definitions of the libtool library interface versions */
#define LIBDBI_LIB_CURRENT 3
#define LIBDBI_LIB_REVISION 0
#define LIBDBI_LIB_AGE 0

/* definition of the libdbi version */
#define LIBDBI_VERSION ((2 * 10000) + (8 * 100) + (0))

(I get a bit confused with that, but 
#if LIBDBI_VERSION >= 900
#define HAVE_LIBDBI_R 1
static dbi_inst dbi_instance = NULL;
#else
#define HAVE_LIBDBI_R 0
#endif

set HAVE_LIBDBI_R to 1 )

and compile error is: 
....
gnc-backend-dbi.cpp: In function 'const GValue* row_get_value_at_col_name(GncSqlRow*, const gchar*)':

gnc-backend-dbi.cpp:2249:64: error: 'dbi_data_t' has no member named 'd_datetime'
             time64 time = result->rows[row]->field_values[idx].d_datetime;
                                                                ^

(In reply to John Ralls from comment #1)
Thanks for the quick answer, but I do not get your input exactly as

      result->rows[row]->field. 

is already into the code. 
I only added:

 #if HAVE_LIBDBI_R
            (void)g_value_init (value, G_TYPE_INT64);
            g_value_set_int64 (value, dbi_result_get_as_longlong  (dbi_row->result, col_name));
#else

the rest was already there, and I left it there as from comment seemed to be used for older versions of libdbi.

But as I did not know libdbi, my input was the result of quick search in libdbi based on the comment. Most probably I missed something there. 

Regards

No, you didn't miss anything, you just didn't organize the code very well. The minimal patch would be
             dbi_result_t* result = (dbi_result_t*) (dbi_row->result);
             guint64 row = dbi_result_get_currow (result);
             guint idx = dbi_result_get_field_idx (result, col_name) - 1;
+#if LIBDBI_VERSION >= 900
+            time64 time = dbi_result_get_as_longlong (dbi_row->result,
+						       col_name);
+#else
             time64 time = result->rows[row]->field_values[idx].d_datetime;
+#endif
             (void)g_value_init (value, G_TYPE_INT64);
             g_value_set_int64 (value, time);
         }
But since result, row, and idx are needed only for the direct access, and the comment applies only to that, 
@@ -137,9 +137,11 @@ struct QofPGSQLBackendProvider : public QofBackendProvider
 
 #if LIBDBI_VERSION >= 900
 #define HAVE_LIBDBI_R 1
+#define HAVE_LIBDBI_AS_LONGLONG 1
 static dbi_inst dbi_instance = NULL;
 #else
 #define HAVE_LIBDBI_R 0
+#define HAVE_LIBDBI_AS_LONGLONG 0
 #endif
 
 #define GNC_HOST_NAME_MAX 255
@@ -2236,15 +2238,18 @@ row_get_value_at_col_name (GncSqlRow* row, const gchar* col_name)
         }
         else
         {
+#if HAVE_LIBDBI_AS_LONGLONG
+            time64 time = dbi_result_get_as_longlong (dbi_row->result,
+						      col_name);
+#else
             /* A seriously evil hack to work around libdbi bug #15
-             * https://sourceforge.net/p/libdbi/bugs/15/. When libdbi
-             * v0.9 is widely available this can be replaced with
-             * dbi_result_get_as_longlong.
+             * https://sourceforge.net/p/libdbi/bugs/15/.
              */
             dbi_result_t* result = (dbi_result_t*) (dbi_row->result);
             guint64 row = dbi_result_get_currow (result);
             guint idx = dbi_result_get_field_idx (result, col_name) - 1;
             time64 time = result->rows[row]->field_values[idx].d_datetime;
+#endif
             (void)g_value_init (value, G_TYPE_INT64);
             g_value_set_int64 (value, time);
         }
is better still.

Note that I'm not re-using HAVE_LIBDBI_R here because it's a coincidence that libdbi's replacement of dbi_conn_new() with dbi_conn_new_r() and provision of dbi_result_get_as_longlong() happen both to be in version 0.9.0. The "better" fix also defines a new feature macro to make it clear what the required feature is. The libdbi folks could change one of them again and having the same feature macro for both would be a problem -- though there haven't been any commits for almost 2 years, so maybe not.

The reason it broke in the first place is https://sourceforge.net/p/libdbi/libdbi/ci/ac482c2b4e478898d1b78afc62972c78c211d6d9/, which also nicely displays their weird versioning that confused you.

They never saw fit to release that revised code, so the OpenSuSe folks went and grabbed it from their repo. That's kind of naughty of OpenSuSe.

Anyway, I've committed the last version to stable and it will be in the next release. Thanks for the report.

GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=770113. Please update any external references or bookmarks.