Fix #630286 - Please add handling code for GDate kvp values in SQL, too

If slots table does not include gdate field, it will be added and all current slots will have a NULL value.

Tested on sqlite3 and mysql.  Tested using the example gnucash file referenced from the bug report.  When saved from XML -> sqlite3 -> XML, some timestamps changed their timezone because of a change of timezone (I'm in North America).  I guess this is OK.


git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@19647 57a11ea4-9604-0410-9ed3-97b8803252fd

src/backend/dbi/gnc-backend-dbi.c

@@ -72,30 +72,36 @@ typedef gchar* (*CREATE_TABLE_DDL_FN)( GncSqlConnection* conn,
                                        const gchar* table_name,
                                        const GList* col_info_list );
 typedef GSList* (*GET_TABLE_LIST_FN)( dbi_conn conn, const gchar* dbname );
+typedef void (*APPEND_COLUMN_DEF_FN)( GString* ddl, GncSqlColumnInfo* info );
 typedef struct
 {
     CREATE_TABLE_DDL_FN		create_table_ddl;
     GET_TABLE_LIST_FN		get_table_list;
+    APPEND_COLUMN_DEF_FN    append_col_def;
 } provider_functions_t;
 
 static /*@ null @*/ gchar* conn_create_table_ddl_sqlite3( GncSqlConnection* conn,
         const gchar* table_name,
         const GList* col_info_list );
 static GSList* conn_get_table_list( dbi_conn conn, const gchar* dbname );
+static void append_sqlite3_col_def( GString* ddl, GncSqlColumnInfo* info );
 static provider_functions_t provider_sqlite3 =
 {
     conn_create_table_ddl_sqlite3,
-    conn_get_table_list
+    conn_get_table_list,
+    append_sqlite3_col_def
 };
 #define SQLITE3_TIMESPEC_STR_FORMAT "%04d%02d%02d%02d%02d%02d"
 
 static /*@ null @*/ gchar* conn_create_table_ddl_mysql( GncSqlConnection* conn,
         const gchar* table_name,
         const GList* col_info_list );
+static void append_mysql_col_def( GString* ddl, GncSqlColumnInfo* info );
 static provider_functions_t provider_mysql =
 {
     conn_create_table_ddl_mysql,
-    conn_get_table_list
+    conn_get_table_list,
+    append_mysql_col_def
 };
 #define MYSQL_TIMESPEC_STR_FORMAT "%04d%02d%02d%02d%02d%02d"
 
@@ -103,10 +109,12 @@ static /*@ null @*/ gchar* conn_create_table_ddl_pgsql( GncSqlConnection* conn,
         const gchar* table_name,
         const GList* col_info_list );
 static GSList* conn_get_table_list_pgsql( dbi_conn conn, const gchar* dbname );
+static void append_pgsql_col_def( GString* ddl, GncSqlColumnInfo* info );
 static provider_functions_t provider_pgsql =
 {
     conn_create_table_ddl_pgsql,
-    conn_get_table_list_pgsql
+    conn_get_table_list_pgsql,
+    append_pgsql_col_def
 };
 #define PGSQL_TIMESPEC_STR_FORMAT "%04d%02d%02d %02d%02d%02d"
 
@@ -114,6 +122,9 @@ static /*@ null @*/ gchar* create_index_ddl( GncSqlConnection* conn,
         const gchar* index_name,
         const gchar* table_name,
         const GncSqlColumnTableEntry* col_table );
+static /*@ null @*/ gchar* add_columns_ddl( GncSqlConnection* conn,
+        const gchar* table_name,
+        GList* col_info_list );
 static GncSqlConnection* create_dbi_connection( /*@ observer @*/ provider_functions_t* provider, /*@ observer @*/ QofBackend* qbe, /*@ observer @*/ dbi_conn conn );
 
 #define GNC_DBI_PROVIDER_SQLITE (&provider_sqlite3)
@@ -1652,21 +1663,22 @@ create_index_ddl( GncSqlConnection* conn,
 }
 
 static /*@ null @*/ gchar*
-conn_create_table_ddl_sqlite3( GncSqlConnection* conn,
+add_columns_ddl( GncSqlConnection* conn,
                  const gchar* table_name,
-                               const GList* col_info_list )
+                 GList* col_info_list )
 {
     GString* ddl;
     const GList* list_node;
+    const GncSqlColumnTableEntry* table_row;
     guint col_num;
-    gchar* type_name;
+    GncDbiSqlConnection* dbi_conn = (GncDbiSqlConnection*)conn;
 
     g_return_val_if_fail( conn != NULL, NULL );
     g_return_val_if_fail( table_name != NULL, NULL );
     g_return_val_if_fail( col_info_list != NULL, NULL );
 
     ddl = g_string_new( "" );
-    g_string_printf( ddl, "CREATE TABLE %s (", table_name );
+    g_string_printf( ddl, "ALTER TABLE %s ", table_name );
     for ( list_node = col_info_list, col_num = 0; list_node != NULL;
             list_node = list_node->next, col_num++ )
     {
@@ -1676,6 +1688,20 @@ conn_create_table_ddl_sqlite3( GncSqlConnection* conn,
         {
             (void)g_string_append( ddl, ", " );
         }
+        g_string_append( ddl, "ADD COLUMN " );
+        dbi_conn->provider->append_col_def( ddl, info );
+        g_free( info->name );
+        g_free( info );
+    }
+
+    return g_string_free( ddl, FALSE );
+}
+
+static void
+append_sqlite3_col_def( GString* ddl, GncSqlColumnInfo* info )
+{
+    gchar* type_name;
+
     if ( info->type == BCT_INT )
     {
         type_name = "integer";
@@ -1715,22 +1741,16 @@ conn_create_table_ddl_sqlite3( GncSqlConnection* conn,
     {
         (void)g_string_append( ddl, " NOT NULL" );
     }
-        g_free( info->name );
-        g_free( info );
-    }
-    (void)g_string_append( ddl, ")" );
-
-    return g_string_free( ddl, FALSE );
 }
 
 static /*@ null @*/ gchar*
-conn_create_table_ddl_mysql( GncSqlConnection* conn, const gchar* table_name,
+conn_create_table_ddl_sqlite3( GncSqlConnection* conn,
+                               const gchar* table_name,
                                const GList* col_info_list )
 {
     GString* ddl;
     const GList* list_node;
     guint col_num;
-    gchar* type_name;
 
     g_return_val_if_fail( conn != NULL, NULL );
     g_return_val_if_fail( table_name != NULL, NULL );
@@ -1747,6 +1767,20 @@ conn_create_table_ddl_mysql( GncSqlConnection* conn, const gchar* table_name,
         {
             (void)g_string_append( ddl, ", " );
         }
+        append_sqlite3_col_def( ddl, info );
+        g_free( info->name );
+        g_free( info );
+    }
+    (void)g_string_append( ddl, ")" );
+
+    return g_string_free( ddl, FALSE );
+}
+
+static void
+append_mysql_col_def( GString* ddl, GncSqlColumnInfo* info )
+{
+    gchar* type_name;
+
     if ( info->type == BCT_INT )
     {
         type_name = "integer";
@@ -1799,23 +1833,15 @@ conn_create_table_ddl_mysql( GncSqlConnection* conn, const gchar* table_name,
     {
         (void)g_string_append( ddl, " NOT NULL" );
     }
-        g_free( info->name );
-        g_free( info );
-    }
-    (void)g_string_append( ddl, ")" );
-
-    return g_string_free( ddl, FALSE );
 }
 
 static /*@ null @*/ gchar*
-conn_create_table_ddl_pgsql( GncSqlConnection* conn, const gchar* table_name,
+conn_create_table_ddl_mysql( GncSqlConnection* conn, const gchar* table_name,
                              const GList* col_info_list )
 {
     GString* ddl;
     const GList* list_node;
     guint col_num;
-    gchar* type_name;
-    gboolean is_unicode = FALSE;
 
     g_return_val_if_fail( conn != NULL, NULL );
     g_return_val_if_fail( table_name != NULL, NULL );
@@ -1832,6 +1858,20 @@ conn_create_table_ddl_pgsql( GncSqlConnection* conn, const gchar* table_name,
         {
             (void)g_string_append( ddl, ", " );
         }
+        append_mysql_col_def( ddl, info );
+        g_free( info->name );
+        g_free( info );
+    }
+    (void)g_string_append( ddl, ")" );
+
+    return g_string_free( ddl, FALSE );
+}
+
+static void
+append_pgsql_col_def( GString* ddl, GncSqlColumnInfo* info )
+{
+    gchar* type_name;
+
     if ( info->type == BCT_INT )
     {
         if ( info->is_autoinc )
@@ -1883,6 +1923,33 @@ conn_create_table_ddl_pgsql( GncSqlConnection* conn, const gchar* table_name,
     {
         (void)g_string_append( ddl, " NOT NULL" );
     }
+}
+
+static /*@ null @*/ gchar*
+conn_create_table_ddl_pgsql( GncSqlConnection* conn, const gchar* table_name,
+                             const GList* col_info_list )
+{
+    GString* ddl;
+    const GList* list_node;
+    guint col_num;
+    gboolean is_unicode = FALSE;
+
+    g_return_val_if_fail( conn != NULL, NULL );
+    g_return_val_if_fail( table_name != NULL, NULL );
+    g_return_val_if_fail( col_info_list != NULL, NULL );
+
+    ddl = g_string_new( "" );
+    g_string_printf( ddl, "CREATE TABLE %s (", table_name );
+    for ( list_node = col_info_list, col_num = 0; list_node != NULL;
+            list_node = list_node->next, col_num++ )
+    {
+        GncSqlColumnInfo* info = (GncSqlColumnInfo*)(list_node->data);
+
+        if ( col_num != 0 )
+        {
+            (void)g_string_append( ddl, ", " );
+        }
+        append_pgsql_col_def( ddl, info );
         is_unicode = is_unicode || info->is_unicode;
         g_free( info->name );
         g_free( info );
@@ -1970,6 +2037,42 @@ conn_create_index( /*@ unused @*/ GncSqlConnection* conn, /*@ unused @*/ const g
     return TRUE;
 }
 
+static gboolean
+conn_add_columns_to_table( /*@ unused @*/ GncSqlConnection* conn, /*@ unused @*/ const gchar* table_name,
+                  GList* col_info_list )
+{
+    GncDbiSqlConnection* dbi_conn = (GncDbiSqlConnection*)conn;
+    gchar* ddl;
+    dbi_result result;
+    gint status;
+
+    g_return_val_if_fail( conn != NULL, FALSE );
+    g_return_val_if_fail( table_name != NULL, FALSE );
+    g_return_val_if_fail( col_info_list != NULL, FALSE );
+
+    ddl = add_columns_ddl( conn, table_name, col_info_list );
+    if ( ddl != NULL )
+    {
+        gint status;
+
+        DEBUG( "SQL: %s\n", ddl );
+        result = dbi_conn_query( dbi_conn->conn, ddl );
+        g_free( ddl );
+        status = dbi_result_free( result );
+        if ( status < 0 )
+        {
+            PERR( "Error in dbi_result_free() result\n" );
+            qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
+        }
+    }
+    else
+    {
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
 static /*@ null @*/ gchar*
 conn_quote_string( const GncSqlConnection* conn, gchar* unquoted_str )
 {
@@ -2063,6 +2166,7 @@ create_dbi_connection( /*@ observer @*/ provider_functions_t* provider,
     dbi_conn->base.commitTransaction = conn_commit_transaction;
     dbi_conn->base.createTable = conn_create_table;
     dbi_conn->base.createIndex = conn_create_index;
+    dbi_conn->base.addColumnsToTable = conn_add_columns_to_table;
     dbi_conn->base.quoteString = conn_quote_string;
     dbi_conn->qbe = qbe;
     dbi_conn->conn = conn;

src/backend/sql/gnc-backend-sql.c

@@ -3134,6 +3134,30 @@ gnc_sql_upgrade_table( GncSqlBackend* be, const gchar* table_name,
     g_free( temp_table_name );
 }
 
+/* Adds one or more columns to an existing table. */
+gboolean gnc_sql_add_columns_to_table( GncSqlBackend* be, const gchar* table_name,
+                            const GncSqlColumnTableEntry* new_col_table )
+{
+    GList* col_info_list = NULL;
+    gboolean ok = FALSE;
+
+    g_return_val_if_fail( be != NULL, FALSE );
+    g_return_val_if_fail( table_name != NULL, FALSE );
+    g_return_val_if_fail( new_col_table != NULL, FALSE );
+
+    for ( ; new_col_table->col_name != NULL; new_col_table++ )
+    {
+        GncSqlColumnTypeHandler* pHandler;
+
+        pHandler = get_handler( new_col_table );
+        g_assert( pHandler != NULL );
+        pHandler->add_col_info_to_list_fn( be, new_col_table, &col_info_list );
+    }
+    g_assert( col_info_list != NULL );
+    ok = gnc_sql_connection_add_columns_to_table( be->conn, table_name, col_info_list );
+    return ok;
+}
+
 /* ================================================================= */
 #define VERSION_TABLE_NAME "versions"
 #define MAX_TABLE_NAME_LEN 50

src/backend/sql/gnc-backend-sql.h

@@ -158,6 +158,7 @@ struct GncSqlConnection
     gboolean (*commitTransaction)( GncSqlConnection* ); /**< Returns TRUE if successful, FALSE if error */
     gboolean (*createTable)( GncSqlConnection*, const gchar*, GList* ); /**< Returns TRUE if successful, FALSE if error */
     gboolean (*createIndex)( GncSqlConnection*, const gchar*, const gchar*, const GncSqlColumnTableEntry* ); /**< Returns TRUE if successful, FALSE if error */
+    gboolean (*addColumnsToTable)( GncSqlConnection*, const gchar* table, GList* ); /**< Returns TRUE if successful, FALSE if error */
     gchar* (*quoteString)( const GncSqlConnection*, gchar* );
 };
 #define gnc_sql_connection_dispose(CONN) (CONN)->dispose(CONN)
@@ -179,6 +180,8 @@ struct GncSqlConnection
 		(CONN)->createTable(CONN,NAME,COLLIST)
 #define gnc_sql_connection_create_index(CONN,INDEXNAME,TABLENAME,COLTABLE) \
 		(CONN)->createIndex(CONN,INDEXNAME,TABLENAME,COLTABLE)
+#define gnc_sql_connection_add_columns_to_table(CONN,TABLENAME,COLLIST) \
+		(CONN)->addColumnsToTable(CONN,TABLENAME,COLLIST)
 #define gnc_sql_connection_quote_string(CONN,STR) \
 		(CONN)->quoteString(CONN,STR)
 
@@ -716,6 +719,17 @@ gchar* gnc_sql_convert_timespec_to_string( const GncSqlBackend* be, Timespec ts
 void gnc_sql_upgrade_table( GncSqlBackend* be, const gchar* table_name,
                             const GncSqlColumnTableEntry* col_table );
 
+/**
+ * Adds one or more columns to an existing table.
+ *
+ * @param be SQL backend
+ * @param table_name SQL table name
+ * @param new_col_table Column table for new columns
+ * @return TRUE if successful, FALSE if unsuccessful
+ */
+gboolean gnc_sql_add_columns_to_table( GncSqlBackend* be, const gchar* table_name,
+                            const GncSqlColumnTableEntry* new_col_table );
+
 /**
  * Specifies the load order for a set of objects.  When loading from a database, the
  * objects will be loaded in this order, so that when later objects have references to

src/backend/sql/gnc-slots-sql.c

@@ -44,19 +44,16 @@
 /*@ unused @*/ static QofLogModule log_module = G_LOG_DOMAIN;
 
 #define TABLE_NAME "slots"
-#define TABLE_VERSION 2
+#define TABLE_VERSION 3
 
 typedef struct
 {
     /*@ dependent @*/ GncSqlBackend* be;
-    /*@ dependent @*/
-    const GncGUID* guid;
+    /*@ dependent @*/ const GncGUID* guid;
     gboolean is_ok;
-    /*@ dependent @*/
-    KvpFrame* pKvpFrame;
+    /*@ dependent @*/ KvpFrame* pKvpFrame;
     KvpValueType value_type;
-    /*@ dependent @*/
-    KvpValue* pKvpValue;
+    /*@ dependent @*/ KvpValue* pKvpValue;
     GString* path;
 } slot_info_t;
 
@@ -78,6 +75,8 @@ static /*@ null @*/ gpointer get_guid_val( gpointer pObject );
 static void set_guid_val( gpointer pObject, /*@ null @*/ gpointer pValue );
 static gnc_numeric get_numeric_val( gpointer pObject );
 static void set_numeric_val( gpointer pObject, gnc_numeric value );
+static GDate* get_gdate_val( gpointer pObject );
+static void set_gdate_val( gpointer pObject, GDate* value );
 
 #define SLOT_MAX_PATHNAME_LEN 4096
 #define SLOT_MAX_STRINGVAL_LEN 4096
@@ -122,6 +121,10 @@ static const GncSqlColumnTableEntry col_table[] =
         "numeric_val",  CT_NUMERIC,  0,                     0,        NULL, NULL,
         (QofAccessFunc)get_numeric_val, (QofSetterFunc)set_numeric_val
     },
+    {
+        "gdate_val",    CT_GDATE,    0,                     0,        NULL, NULL,
+        (QofAccessFunc)get_gdate_val, (QofSetterFunc)set_gdate_val
+    },
     { NULL }
     /*@ +full_init_block @*/
 };
@@ -136,6 +139,14 @@ static const GncSqlColumnTableEntry obj_guid_col_table[] =
     /*@ +full_init_block @*/
 };
 
+static const GncSqlColumnTableEntry gdate_col_table[] =
+{
+    /*@ -full_init_block @*/
+    { "gdate_val", CT_GDATE, 0, 0, },
+    { NULL }
+    /*@ +full_init_block @*/
+};
+
 /* ================================================================= */
 
 static /*@ null @*/ gpointer
@@ -376,6 +387,38 @@ set_numeric_val( gpointer pObject, gnc_numeric value )
     }
 }
 
+static GDate*
+get_gdate_val( gpointer pObject )
+{
+    slot_info_t* pInfo = (slot_info_t*)pObject;
+    static GDate date;
+
+    g_return_val_if_fail( pObject != NULL, NULL );
+
+    if ( kvp_value_get_type( pInfo->pKvpValue ) == KVP_TYPE_GDATE )
+    {
+        date = kvp_value_get_gdate( pInfo->pKvpValue );
+        return &date;
+    }
+    else
+    {
+        return NULL;
+    }
+}
+
+static void
+set_gdate_val( gpointer pObject, GDate* value )
+{
+    slot_info_t* pInfo = (slot_info_t*)pObject;
+
+    g_return_if_fail( pObject != NULL );
+
+    if ( pInfo->value_type == KVP_TYPE_GDATE )
+    {
+        kvp_frame_set_gdate( pInfo->pKvpFrame, pInfo->path->str, *value );
+    }
+}
+
 static void
 save_slot( const gchar* key, KvpValue* value, gpointer data )
 {
@@ -713,15 +756,29 @@ create_slots_tables( GncSqlBackend* be )
             PERR( "Unable to create index\n" );
         }
     }
-    else if ( version == 1 )
+    else if ( version < TABLE_VERSION )
+    {
+        /* Upgrade:
+            1->2: 64-bit int values to proper definition, add index
+            2->3: Add gdate field
+        */
+        if ( version == 1 )
         {
-        /* Upgrade 64-bit int values to proper definition */
             gnc_sql_upgrade_table( be, TABLE_NAME, col_table );
             ok = gnc_sql_create_index( be, "slots_guid_index", TABLE_NAME, obj_guid_col_table );
             if ( !ok )
             {
                 PERR( "Unable to create index\n" );
             }
+        }
+        else if ( version == 2 )
+        {
+            ok = gnc_sql_add_columns_to_table( be, TABLE_NAME, gdate_col_table );
+            if ( !ok )
+            {
+                PERR( "Unable to add gdate column\n" );
+            }
+        }
         (void)gnc_sql_set_table_version( be, TABLE_NAME, TABLE_VERSION );
     }
 }