[glom] libglom: Use escape_sql_id() for column add/crop/change.

From: Murray Cumming <murrayc src gnome org>
To: commits-list gnome org
Cc:
Subject: [glom] libglom: Use escape_sql_id() for column add/crop/change.
Date: Tue, 8 Nov 2011 13:07:41 +0000 (UTC)
commit 1d381de4ba0fc2d6991651f1f8f89366cd4e3e94
Author: Murray Cumming <murrayc murrayc com>
Date:   Tue Nov 8 14:06:36 2011 +0100

    libglom: Use escape_sql_id() for column add/crop/change.
    
    	* glom/libglom/connectionpool.cc: add_column(), drop_column(),
    
    	* glom/libglom/connectionpool_backends/backend.[h|cc]:
    	* glom/libglom/connectionpool_backends/postgres.cc
    	* glom/libglom/connectionpool_backends/sqlite.[h|cc]:
    	add_column(), drop_column(): Return a bool to indicate failure.
    	change_columns(): Report false return values from these.
    
    	* Makefile_tests.am
    	* tests/test_selfhosting_new_then_change_columns.cc: Add a simple
    	test, though we do not yet test that the expected change really
    	happened.

 ChangeLog                                         |   17 +++
 Makefile_tests.am                                 |    6 +
 glom/libglom/connectionpool.cc                    |    8 +-
 glom/libglom/connectionpool_backends/backend.cc   |    8 +-
 glom/libglom/connectionpool_backends/backend.h    |    4 +-
 glom/libglom/connectionpool_backends/postgres.cc  |   31 +++--
 glom/libglom/connectionpool_backends/sqlite.cc    |   55 ++++----
 glom/libglom/connectionpool_backends/sqlite.h     |    4 +-
 tests/test_selfhosting_new_then_change_columns.cc |  146 +++++++++++++++++++++
 9 files changed, 229 insertions(+), 50 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 0fe1f65..0f63ff1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2011-11-08  Murray Cumming  <murrayc murrayc com>
 
+	libglom: Use escape_sql_id() for column add/crop/change.
+
+	* glom/libglom/connectionpool.cc: add_column(), drop_column(),
+	
+	* glom/libglom/connectionpool_backends/backend.[h|cc]:
+	* glom/libglom/connectionpool_backends/postgres.cc
+	* glom/libglom/connectionpool_backends/sqlite.[h|cc]:
+	add_column(), drop_column(): Return a bool to indicate failure.
+	change_columns(): Report false return values from these.
+
+	* Makefile_tests.am
+	* tests/test_selfhosting_new_then_change_columns.cc: Add a simple
+	test, though we do not yet test that the expected change really
+	happened.
+
+2011-11-08  Murray Cumming  <murrayc murrayc com>
+
 	libglom: Add escape_sql_id() and use it for table add/drop/rename.
 
 	* glom/libglom/db_utils.[h|cc]: Add escape_sql_id(), using 
diff --git a/Makefile_tests.am b/Makefile_tests.am
index ecea00b..a554577 100644
--- a/Makefile_tests.am
+++ b/Makefile_tests.am
@@ -32,6 +32,7 @@ check_PROGRAMS =						\
 	tests/test_selfhosting_new_then_backup_restore \
 	tests/test_selfhosting_new_then_get_privs \
 	tests/test_selfhosting_new_then_alter_table \
+	tests/test_selfhosting_new_then_change_columns \
 	tests/test_selfhosting_sqlinjection \
 	tests/import/test_parsing \
 	tests/import/test_signals
@@ -52,6 +53,7 @@ TESTS =	tests/test_document_load	\
 	tests/test_selfhosting_new_then_image \
 	tests/test_selfhosting_new_then_get_privs \
 	tests/test_selfhosting_new_then_alter_table \
+	tests/test_selfhosting_new_then_change_columns \
 	tests/test_selfhosting_sqlinjection \
 	tests/import/test_parsing \
 	tests/import/test_signals
@@ -166,6 +168,10 @@ tests_test_selfhosting_new_then_alter_table_SOURCES = tests/test_selfhosting_new
 tests_test_selfhosting_new_then_alter_table_LDADD = $(tests_ldadd)
 tests_test_selfhosting_new_then_alter_table_CPPFLAGS = $(tests_cppflags)
 
+tests_test_selfhosting_new_then_change_columns_SOURCES = tests/test_selfhosting_new_then_change_columns.cc $(sources_test_selfhosting_utils)
+tests_test_selfhosting_new_then_change_columns_LDADD = $(tests_ldadd)
+tests_test_selfhosting_new_then_change_columns_CPPFLAGS = $(tests_cppflags)
+
 tests_test_selfhosting_sqlinjection_SOURCES = tests/test_selfhosting_sqlinjection.cc $(sources_test_selfhosting_utils)
 tests_test_selfhosting_sqlinjection_LDADD = $(tests_ldadd)
 tests_test_selfhosting_sqlinjection_CPPFLAGS = $(tests_cppflags)
diff --git a/glom/libglom/connectionpool.cc b/glom/libglom/connectionpool.cc
index 0685c54..dae8533 100644
--- a/glom/libglom/connectionpool.cc
+++ b/glom/libglom/connectionpool.cc
@@ -676,9 +676,9 @@ bool ConnectionPool::add_column(const Glib::ustring& table_name, const sharedptr
 
   try
   {
-    m_backend->add_column(m_refGdaConnection, table_name, field);
+    const bool result = m_backend->add_column(m_refGdaConnection, table_name, field);
     m_refGdaConnection->update_meta_store_table(table_name, m_backend->get_public_schema_name());
-    return true;
+    return result;
   }
   catch(const Glib::Error& ex)
   {
@@ -701,9 +701,9 @@ bool ConnectionPool::drop_column(const Glib::ustring& table_name, const Glib::us
 
   try
   {
-    m_backend->drop_column(m_refGdaConnection, table_name, field_name);
+    const bool result = m_backend->drop_column(m_refGdaConnection, table_name, field_name);
     m_refGdaConnection->update_meta_store_table(table_name, m_backend->get_public_schema_name());
-    return true;
+    return result;
   }
   catch(const Glib::Error& ex)
   {
diff --git a/glom/libglom/connectionpool_backends/backend.cc b/glom/libglom/connectionpool_backends/backend.cc
index a90bcd5..9058bf5 100644
--- a/glom/libglom/connectionpool_backends/backend.cc
+++ b/glom/libglom/connectionpool_backends/backend.cc
@@ -65,7 +65,7 @@ bool Backend::set_network_shared(const SlotProgress& /* slot_progress */, bool /
   return true; //Success at doing nothing.
 }
 
-void Backend::add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field)
+bool Backend::add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field)
 {
   Glib::RefPtr<Gnome::Gda::ServerProvider> provider = connection->get_provider();
   Glib::RefPtr<Gnome::Gda::ServerOperation> operation = provider->create_operation(connection, Gnome::Gda::SERVER_OPERATION_ADD_COLUMN);
@@ -77,10 +77,10 @@ void Backend::add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection,
   operation->set_value_at("/COLUMN_DEF_P/COLUMN_PKEY", field->get_primary_key());
   operation->set_value_at("/COLUMN_DEF_P/COLUMN_UNIQUE", field->get_unique_key());
 
-  provider->perform_operation(connection, operation);
+  return provider->perform_operation(connection, operation);
 }
 
-void Backend::drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name)
+bool Backend::drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name)
 {
   Glib::RefPtr<Gnome::Gda::ServerProvider> provider = connection->get_provider();
   Glib::RefPtr<Gnome::Gda::ServerOperation> operation = provider->create_operation(connection, Gnome::Gda::SERVER_OPERATION_DROP_COLUMN);
@@ -88,7 +88,7 @@ void Backend::drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection
   //TODO: Quote table name and column name?
   operation->set_value_at("/COLUMN_DESC_P/TABLE_NAME", table_name);
   operation->set_value_at("/COLUMN_DESC_P/COLUMN_NAME", field_name);
-  provider->perform_operation(connection, operation);
+  return provider->perform_operation(connection, operation);
 }
 
 void Backend::set_database_directory_uri(const std::string& directory_uri)
diff --git a/glom/libglom/connectionpool_backends/backend.h b/glom/libglom/connectionpool_backends/backend.h
index 787a2c9..46a5666 100644
--- a/glom/libglom/connectionpool_backends/backend.h
+++ b/glom/libglom/connectionpool_backends/backend.h
@@ -177,11 +177,11 @@ protected:
 
   /** @throws Glib::Error (from libgdamm)
    */
-  virtual void add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field);
+  virtual bool add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field);
 
   /** @throws Glib::Error (from libgdamm)
    */
-  virtual void drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name);
+  virtual bool drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name);
 
   virtual bool change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const type_vec_const_fields& old_fields, const type_vec_const_fields& new_fields) throw() = 0;
 
diff --git a/glom/libglom/connectionpool_backends/postgres.cc b/glom/libglom/connectionpool_backends/postgres.cc
index 5572dd6..b2baa7a 100644
--- a/glom/libglom/connectionpool_backends/postgres.cc
+++ b/glom/libglom/connectionpool_backends/postgres.cc
@@ -24,6 +24,7 @@
 #include <libglom/connectionpool_backends/postgres.h>
 #include <libglom/spawn_with_feedback.h>
 #include <libglom/utils.h>
+#include <libglom/db_utils.h>
 #include <libgdamm/config.h>
 #include <giomm/file.h>
 #include <glibmm/convert.h>
@@ -176,10 +177,15 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 		    // primary key columns.
 		    temp_field->set_primary_key(false);
 
-		    add_column(connection, table_name, temp_field);
+		    const bool added = add_column(connection, table_name, temp_field);
+                    if(!added)
+                    {
+                      std::cerr << G_STRFUNC << ": add_column() failed." << std::endl;
+                      //TODO: Stop the transaction and return?
+                    }
 
 		    Glib::ustring conversion_command;
-		    const Glib::ustring field_name_old_quoted = "\"" + old_fields[i]->get_name() + "\"";
+		    const Glib::ustring field_name_old_quoted = DbUtils::escape_sql_id(old_fields[i]->get_name());
 		    const Field::glom_field_type old_field_type = old_fields[i]->get_glom_type();
 
 		    if(Field::get_conversion_possible(old_fields[i]->get_glom_type(), new_fields[i]->get_glom_type()))
@@ -263,7 +269,8 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 		        }
 		      }
 
-		      connection->statement_execute_non_select("UPDATE \"" + table_name + "\" SET \"" + TEMP_COLUMN_NAME + "\" = " + conversion_command);
+                      //TODO: Use SqlBuilder here?
+		      connection->statement_execute_non_select("UPDATE " + DbUtils::escape_sql_id(table_name) + " SET " + DbUtils::escape_sql_id(TEMP_COLUMN_NAME) + " = " + conversion_command);
 		    }
 		    else
 		    {
@@ -272,12 +279,12 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 
 		    drop_column(connection, table_name, old_fields[i]->get_name());
 
-		    connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" RENAME COLUMN \"" + TEMP_COLUMN_NAME + "\" TO \"" + new_fields[i]->get_name() + "\"");
+		    connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " RENAME COLUMN " + DbUtils::escape_sql_id(TEMP_COLUMN_NAME) + " TO " + DbUtils::escape_sql_id(new_fields[i]->get_name()));
 
 		    // Read primary key constraint
 		    if(new_fields[i]->get_primary_key())
 		    {
-		      connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" ADD PRIMARY KEY (\"" + new_fields[i]->get_name() + "\")");
+		      connection->statement_execute_non_select("ALTER TABLE  " + DbUtils::escape_sql_id(table_name) + " ADD PRIMARY KEY (" + DbUtils::escape_sql_id(new_fields[i]->get_name()) + ")");
 		    }
 		  }
 		  else
@@ -298,19 +305,19 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 		        primary_key_was_set = true;
 
 		        // Primary key was added
-		       connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" ADD PRIMARY KEY (\"" + old_fields[i]->get_name() + "\")");
+		       connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " ADD PRIMARY KEY (" + DbUtils::escape_sql_id(old_fields[i]->get_name()) + ")");
 
 		        // Remove unique key constraint, because this is already implied in
 		        // the field being primary key.
 		        if(old_fields[i]->get_unique_key())
-		          connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" DROP CONSTRAINT \"" + old_fields[i]->get_name() + "_key");
+		          connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " DROP CONSTRAINT " + DbUtils::escape_sql_id(old_fields[i]->get_name() + "_key"));
 		      }
 		      else
 		      {
 		        primary_key_was_unset = true;
 
 		        // Primary key was removed
-		        connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" DROP CONSTRAINT \"" + table_name + "_pkey\"");
+		        connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " DROP CONSTRAINT " + DbUtils::escape_sql_id(table_name + "_pkey"));
 		      }
 		    }
 
@@ -321,11 +328,11 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 		      // to do that separately if we already made it a primary key
 		      if(!primary_key_was_set && new_fields[i]->get_unique_key())
 		      {
-		        connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" ADD CONSTRAINT \"" + old_fields[i]->get_name() + "_key\" UNIQUE (\"" + old_fields[i]->get_name() + "\")");
+		        connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " ADD CONSTRAINT " + DbUtils::escape_sql_id(old_fields[i]->get_name() + "_key") + " UNIQUE (" + DbUtils::escape_sql_id(old_fields[i]->get_name()) + ")");
 		      }
 		      else if(!primary_key_was_unset && !new_fields[i]->get_unique_key() && !new_fields[i]->get_primary_key())
 		      {
-		        connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" DROP CONSTRAINT \"" + old_fields[i]->get_name() + "_key\"");
+		        connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " DROP CONSTRAINT " + DbUtils::escape_sql_id(old_fields[i]->get_name() + "_key"));
 		      }
 		    }
 
@@ -333,13 +340,13 @@ bool Postgres::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connec
 		    {
 		      if(old_fields[i]->get_default_value() != new_fields[i]->get_default_value())
 		      {
-		        connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" ALTER COLUMN \"" + old_fields[i]->get_name() + "\" SET DEFAULT " + new_fields[i]->sql(new_fields[i]->get_default_value(), connection));
+		        connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " ALTER COLUMN " + DbUtils::escape_sql_id(old_fields[i]->get_name()) + " SET DEFAULT " + new_fields[i]->sql(new_fields[i]->get_default_value(), connection));
 		      }
 		    }
 
 		    if(old_fields[i]->get_name() != new_fields[i]->get_name())
 		    {
-		      connection->statement_execute_non_select("ALTER TABLE \"" + table_name + "\" RENAME COLUMN \"" + old_fields[i]->get_name() + "\" TO \"" + new_fields[i]->get_name() + "\"");
+		      connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(table_name) + " RENAME COLUMN " + DbUtils::escape_sql_id(old_fields[i]->get_name()) + " TO " + DbUtils::escape_sql_id(new_fields[i]->get_name()));
 		    }
 		  }
 		}
diff --git a/glom/libglom/connectionpool_backends/sqlite.cc b/glom/libglom/connectionpool_backends/sqlite.cc
index 12e8470..fa4eff1 100644
--- a/glom/libglom/connectionpool_backends/sqlite.cc
+++ b/glom/libglom/connectionpool_backends/sqlite.cc
@@ -21,6 +21,7 @@
 #include <libglom/libglom_config.h>
 #include <libglom/connectionpool_backends/sqlite.h>
 #include <libglom/utils.h>
+#include <libglom/db_utils.h>
 #include <giomm/file.h>
 #include <libgdamm/metastore.h>
 
@@ -204,35 +205,35 @@ bool Sqlite::recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connecti
       {
       case Field::TYPE_TEXT:
         if(column->gtype == G_TYPE_BOOLEAN)
-	  trans_fields += Glib::ustring("(CASE WHEN \"") + column->column_name + "\" = 1 THEN 'true' "
-                                              "WHEN \""  + column->column_name + "\" = 0 THEN 'false' "
-                                              "WHEN \""  + column->column_name + "\" IS NULL THEN 'false' END)";
+	  trans_fields += "(CASE WHEN "+ DbUtils::escape_sql_id(column->column_name) + " = 1 THEN 'true' "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " = 0 THEN 'false' "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " IS NULL THEN 'false' END)";
 	else if(column->gtype == GDA_TYPE_BLOB)
 	  trans_fields += "''";
 	else
           // Make sure we don't insert NULL strings, as we ignore that concept in Glom.
-          trans_fields += Glib::ustring("(CASE WHEN \"") + column->column_name + "\" IS NULL THEN '' "
-                                              "WHEN \""  + column->column_name + "\" IS NOT NULL THEN \"" + column->column_name + "\" END)";
+          trans_fields += "(CASE WHEN "+ DbUtils::escape_sql_id(column->column_name) + " IS NULL THEN '' "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " IS NOT NULL THEN " + DbUtils::escape_sql_id(column->column_name) + " END)";
 	break;
       case Field::TYPE_NUMERIC:
         if(column->gtype == G_TYPE_BOOLEAN)
-          trans_fields += Glib::ustring("(CASE WHEN \"") + column->column_name + "\" = 0 THEN 0 "
-                                              "WHEN \""  + column->column_name + "\" != 0 THEN 1 "
-                                              "WHEN \""  + column->column_name + "\" IS NULL THEN 0 END)";
+          trans_fields += "(CASE WHEN "+ DbUtils::escape_sql_id(column->column_name) + " = 0 THEN 0 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " != 0 THEN 1 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " IS NULL THEN 0 END)";
         else if(column->gtype == GDA_TYPE_BLOB || column->gtype == G_TYPE_DATE || column->gtype == GDA_TYPE_TIME)
           trans_fields += '0';
         else
-          trans_fields += Glib::ustring("CAST(") + column->column_name + " AS real)";
+          trans_fields += Glib::ustring("CAST(")+ DbUtils::escape_sql_id(column->column_name) + " AS real)";
         break;
       case Field::TYPE_BOOLEAN:
         if(column->gtype == G_TYPE_STRING)
-          trans_fields += Glib::ustring("(CASE WHEN \"") + column->column_name + "\" = 'true' THEN 1 "
-                                              "WHEN \""  + column->column_name + "\" = 'false' THEN 0 "
-                                              "WHEN \""  + column->column_name + "\" IS NULL THEN 0 END)";
+          trans_fields += "(CASE WHEN "+ DbUtils::escape_sql_id(column->column_name) + " = 'true' THEN 1 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " = 'false' THEN 0 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " IS NULL THEN 0 END)";
         else if(column->gtype == G_TYPE_DOUBLE)
-          trans_fields += Glib::ustring("(CASE WHEN \"") + column->column_name + "\" = 0 THEN 0 "
-                                              "WHEN \""  + column->column_name + "\" != 0 THEN 1 "
-                                              "WHEN \""  + column->column_name + "\" IS NULL THEN 0 END)";
+          trans_fields += "(CASE WHEN "+ DbUtils::escape_sql_id(column->column_name) + " = 0 THEN 0 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " != 0 THEN 1 "
+                                              "WHEN " + DbUtils::escape_sql_id(column->column_name) + " IS NULL THEN 0 END)";
         else if(column->gtype == G_TYPE_BOOLEAN)
           trans_fields += column->column_name;
         else
@@ -242,13 +243,13 @@ bool Sqlite::recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connecti
         if(column->gtype == G_TYPE_BOOLEAN || column->gtype == GDA_TYPE_BLOB || column->gtype == G_TYPE_DOUBLE)
           trans_fields += "NULL";
         else
-          trans_fields += Glib::ustring("date(") + column->column_name + ')';
+          trans_fields += Glib::ustring("date(")+ DbUtils::escape_sql_id(column->column_name) + ')';
         break;
       case Field::TYPE_TIME:
         if(column->gtype == G_TYPE_BOOLEAN || column->gtype == GDA_TYPE_BLOB || column->gtype == G_TYPE_DOUBLE)
           trans_fields += "NULL";
         else
-          trans_fields += Glib::ustring("time(") + column->column_name + ')';
+          trans_fields += Glib::ustring("time(")+ DbUtils::escape_sql_id(column->column_name) + ')';
         break;
       case Field::TYPE_IMAGE:
         if(column->gtype == GDA_TYPE_BLOB)
@@ -282,7 +283,7 @@ bool Sqlite::recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connecti
 
       if(!trans_fields.empty())
         trans_fields += ',';
-      Gnome::Gda::Value default_value = field->get_default_value();
+      const Gnome::Gda::Value default_value = field->get_default_value();
       if(default_value.get_value_type() != G_TYPE_NONE && !default_value.is_null())
         trans_fields += field->sql(default_value, connection);
       else
@@ -324,9 +325,11 @@ bool Sqlite::recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connecti
 
     if(!trans_fields.empty())
     {
-      connection->statement_execute_non_select(Glib::ustring("INSERT INTO \"") + TEMPORARY_TABLE_NAME + "\" SELECT " + trans_fields + " FROM \"" + table_name + "\"");
-      connection->statement_execute_non_select("DROP TABLE " + table_name);
-      connection->statement_execute_non_select(Glib::ustring("ALTER TABLE \"") + TEMPORARY_TABLE_NAME + "\" RENAME TO \"" + table_name + "\"");
+      const Glib::ustring query_insert = "INSERT INTO " + DbUtils::escape_sql_id(TEMPORARY_TABLE_NAME) + " SELECT " + trans_fields + " FROM " + DbUtils::escape_sql_id(table_name);
+      //std::cout << "debug: query_insert=" << query_insert << std::endl;
+      connection->statement_execute_non_select(query_insert);
+      connection->statement_execute_non_select("DROP TABLE " + DbUtils::escape_sql_id(table_name));
+      connection->statement_execute_non_select("ALTER TABLE " + DbUtils::escape_sql_id(TEMPORARY_TABLE_NAME) + " RENAME TO " + DbUtils::escape_sql_id(table_name));
 
       connection->commit_transaction(TRANSACTION_NAME);
 
@@ -351,23 +354,23 @@ bool Sqlite::recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connecti
   return false;
 }
 
-void Sqlite::add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field)
+bool Sqlite::add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field)
 {
   // Sqlite does not support adding primary key columns. So recreate the table
   // in that case.
   if(!field->get_primary_key())
   {
-    Backend::add_column(connection, table_name, field);
+    return Backend::add_column(connection, table_name, field);
   }
   else
   {
-    recreate_table(connection, table_name, type_vec_strings(), type_vec_const_fields(1, field), type_mapFieldChanges());
+    return recreate_table(connection, table_name, type_vec_strings(), type_vec_const_fields(1, field), type_mapFieldChanges());
   }
 }
 
-void Sqlite::drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name)
+bool Sqlite::drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name)
 {
-  recreate_table(connection, table_name, type_vec_strings(1, field_name), type_vec_const_fields(), type_mapFieldChanges());
+  return recreate_table(connection, table_name, type_vec_strings(1, field_name), type_vec_const_fields(), type_mapFieldChanges());
 }
 
 bool Sqlite::change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const type_vec_const_fields& old_fields, const type_vec_const_fields& new_fields) throw()
diff --git a/glom/libglom/connectionpool_backends/sqlite.h b/glom/libglom/connectionpool_backends/sqlite.h
index 3917874..52c0850 100644
--- a/glom/libglom/connectionpool_backends/sqlite.h
+++ b/glom/libglom/connectionpool_backends/sqlite.h
@@ -52,8 +52,8 @@ private:
 
   bool recreate_table(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const type_vec_strings& fields_removed, const type_vec_const_fields& fields_added, const type_mapFieldChanges& fields_changed) throw();
 
-  virtual void add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field);
-  virtual void drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name);
+  virtual bool add_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const sharedptr<const Field>& field);
+  virtual bool drop_column(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const Glib::ustring& field_name);
   virtual bool change_columns(const Glib::RefPtr<Gnome::Gda::Connection>& connection, const Glib::ustring& table_name, const type_vec_const_fields& old_fields, const type_vec_const_fields& new_fields) throw();
 
   virtual Glib::RefPtr<Gnome::Gda::Connection> connect(const Glib::ustring& database, const Glib::ustring& username, const Glib::ustring& password);
diff --git a/tests/test_selfhosting_new_then_change_columns.cc b/tests/test_selfhosting_new_then_change_columns.cc
new file mode 100644
index 0000000..3a7bd5f
--- /dev/null
+++ b/tests/test_selfhosting_new_then_change_columns.cc
@@ -0,0 +1,146 @@
+/* Glom
+ *
+ * Copyright (C) 2010 Openismus GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+71 * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#include "tests/test_selfhosting_utils.h"
+#include <libglom/init.h>
+#include <libglom/utils.h>
+#include <libglom/db_utils.h>
+#include <libglom/connectionpool.h>
+#include <glibmm/fileutils.h>
+#include <glibmm/miscutils.h>
+#include <libgda/gda-blob-op.h>
+#include <glib.h> //For g_assert()
+#include <iostream>
+#include <cstdlib> //For EXIT_SUCCESS and EXIT_FAILURE
+#include <cstring> //For memcmp().
+
+static bool test(Glom::Document::HostingMode hosting_mode)
+{
+  Glom::Document document;
+  const bool recreated = 
+    test_create_and_selfhost_from_example("example_smallbusiness.glom", document, hosting_mode);
+  if(!recreated)
+  {
+    std::cerr << "Recreation failed." << std::endl;
+    return false;
+  }
+  
+  const Glib::ustring table_name = "contacts";
+  const Glom::sharedptr<const Glom::Field> field = document.get_field(table_name, "date_of_birth");
+  if(!field)
+  {
+    std::cerr << "Failure: Could not get field." << std::endl;
+    return false;
+  }
+
+  Glom::sharedptr<Glom::Field> field_new = Glom::glom_sharedptr_clone(field);
+  if(!field_new)
+  {
+    std::cerr << "Failure: field_new is null." << std::endl;
+    return false;
+  }
+  field_new->set_glom_type(Glom::Field::TYPE_TEXT);
+
+  Glom::ConnectionPool* connection_pool = Glom::ConnectionPool::get_instance();
+  if(!connection_pool)
+  {
+    std::cerr << "Failure: connection_pool is null." << std::endl;
+    return false;
+  }
+
+  //Test that change_column() does not fail horribly:
+  //TODO: Start with some data that can be converted meaningfully,
+  //and check that the result is as expected:
+  try
+  {
+    const bool test = connection_pool->change_column(table_name, field, field_new);
+    if(!test)
+    {
+      std::cerr << "Failure: change_column() failed." << std::endl;
+      return false;
+    }
+  }
+  catch(const Glib::Error& ex)
+  { 
+    std::cerr << "Failure: change_column() threw an exception: " << ex.what() << std::endl;
+    return false;
+  }
+
+  //Try another change:
+  field_new->set_glom_type(Glom::Field::TYPE_NUMERIC);
+  try
+  {
+    const bool test = connection_pool->change_column(table_name, field, field_new);
+    if(!test)
+    {
+      std::cerr << "Failure: change_column() failed." << std::endl;
+      return false;
+    }
+  }
+  catch(const Glib::Error& ex)
+  { 
+    std::cerr << "Failure: change_column() threw an exception: " << ex.what() << std::endl;
+    return false;
+  }
+
+  //Try another change:
+  field_new->set_name("somenewfieldname");
+  try
+  {
+    const bool test = connection_pool->change_column(table_name, field, field_new);
+    if(!test)
+    {
+      std::cerr << "Failure: change_column() failed." << std::endl;
+      return false;
+    }
+  }
+  catch(const Glib::Error& ex)
+  { 
+    std::cerr << "Failure: change_column() threw an exception: " << ex.what() << std::endl;
+    return false;
+  }
+
+  test_selfhosting_cleanup();
+ 
+  return true; 
+}
+
+int main()
+{
+  Glom::libglom_init();
+
+  if(!test(Glom::Document::HOSTING_MODE_POSTGRES_SELF))
+  {
+    std::cerr << "Failed with PostgreSQL" << std::endl;
+    test_selfhosting_cleanup();
+    return EXIT_FAILURE;
+  }
+  
+  if(!test(Glom::Document::HOSTING_MODE_SQLITE))
+  {
+    std::cerr << "Failed with SQLite" << std::endl;
+    test_selfhosting_cleanup();
+    return EXIT_FAILURE;
+  }
+
+  Glom::libglom_deinit();
+
+  return EXIT_SUCCESS;
+}
[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]