GNOME Bugzilla – Bug 587051
Glom can't handle table names containing capital letters
Last modified: 2009-08-26 14:52:43 UTC
When creating a new table in a Glom document, and the table name contains a capital letter, then the new table is not shown correctly in the list view, and when closing and reopening the "Tables" dialog, then the new table is not shown anymore, even though it is shown in the "Tables" menu item. I guess this is because libgda does not list such a table when we query the list of tables in the database. This happens for both PostgreSQL and SQLite. If this is intended, then we should probably prevent the user from entering such table names, or convert them to lowercase before actually creating the table.
This is not intended. We should just fix the bug.
I notice (with pgadmin3, and with our --debug_sql option) that the tables are created correctly. So I think this is a problem with getting the meta data from libgdamm. It is probably a quoting problem. We'll need a simple test case to confirm this.
Created attachment 137573 [details] Confirmation of quotation of table names containing capital letters in the GdaMetaStore Here is a testcase that indeed confirms this: When the table name contains a capital letter, then the table_short_name (which Glom uses) is quoted, otherwise not. So I think we want to unquote this in Glom.
Is this how we get the list of table names in Glom? And sometimes the table name is quoted but sometimes not? That's awful. How can anyone know what is a quote and what is a quote character in the name itself?
The output from the test case: murrayc@murrayc-desktop:~/svn/gnome220$ ./a.out Table Name: WithCapitals table_short_name | table_schema | table_full_name | table_owner | table_comments -----------------+--------------+---------------------+-------------+--------------- "WithCapitals" | main | main."WithCapitals" | NULL | NULL (1 row) murrayc@murrayc-desktop:~/svn/gnome220$ ./a.out Table Name: withnocapitals table_short_name | table_schema | table_full_name | table_owner | table_comments -----------------+--------------+---------------------+-------------+--------------- withnocapitals | main | main.withnocapitals | NULL | NULL (1 row)
(In reply to comment #4) > Is this how we get the list of table names in Glom? And sometimes the table > name is quoted but sometimes not? That's awful. How can anyone know what is a > quote and what is a quote character in the name itself? I guess that if there is a quote character in the table name, then the table name is quoted in the form "foo\"bar". We could just let gda_sql_identifier_remove_quotes() handle that. I'll try this out and see if it fixes the problem.
No, I think escaping is a separate issue. And that function already does to-lowercase conversion, which is very annoying.
According to the documentation, the function only converts to lower case if the name is not quoted. However, if there are capital letters in the name, than the name is quoted anyway, in which case the function does not convert it to lower case. The documentation also says that the string "can contain the delimiter character (the single or double quotes) in the string if every instance of it is preceeded with a backslash character or with the delimiter character itself".
Fixed with this commit: 2009-06-30 Armin Burgmeier <armin@openismus.com> * glom/base_db.cc (get_fields_for_table_from_database): Make this work for table names which need quotation, and return unquoted field names. This fixes bug #587051.
(In reply to comment #9) > Fixed with this commit: > > 2009-06-30 Armin Burgmeier <armin@openismus.com> > > * glom/base_db.cc (get_fields_for_table_from_database): Make this work > for table names which need quotation, and return unquoted field names. > This fixes bug #587051. Thanks. I tried just removing the quotes instead of using that weird gda_sql_identifier_needs_quotes() function, but your way is really the only way to make this work. That's because it ends up using somelowercasetable or "SomeTableWithCapitals", but "somelowercasetable" or SomeTableWithCapitals are not acceptable values. I hate that API so much.
In fact, I now find that get_fields_for_table_from_database() still fails for tables named "CapitalTest", or "CapitalTest2", for instance. I thought I had see Glom showing the fields in the list view, but now I am not sure.
Did you test this after having created a new table called "CapitalTest", or when viewing a table which was created by a version of Glom which did not yet have the fix? In the latter case it won't show the table fields correctly, because the fields are no longer listed in the Glom document due to the previous breakage. However, when creating a new table called "CapitalTest" or "CapitalTest2", everything seems to work for me.
I tried both. I'm doing a make clean all.
Armin, are you using the latest libgdamm from git?
Yes, but there doesn't seem to have been a relevant change recently in libgdamm. Did you maybe forget to push a commit?
Yes, sorry. Pushed. 2009-06-30 Murray Cumming <murrayc@murrayc.com> Fix per-table meta-store retrieval for table names with capital letters. * libgda/src/connection.ccg: update_meta_store_table(): Quote the table name so it works with capital letters, spaces, etc. Vivien Malerba stated on the mailing list that this is necessary. I am not convinced that this function works at all anyway though. However, I guess that may actually be wrong/incomplete.
Created attachment 137672 [details] Testcase for quoting table names to update the meta store Yeah, this change breaks things again. This testcase also suggests that for updating the meta store, the table name should not be quoted.
This specific point is now corrected in git (master and LIBGDA_4.0 branches).
(In reply to comment #18) > This specific point is now corrected in git (master and LIBGDA_4.0 branches). Thanks, but this seems to break this code in libgdamm, making it throw a GError, because GdaMetaContext::size is 0. Should this be correct?: #ifdef GLIBMM_EXCEPTIONS_ENABLED bool Connection::update_meta_store_data_types() { GdaMetaContext mcontext = {(gchar*)"_builtin_data_types", 0, 0, 0 }; GError* gerror = 0; const bool retval = gda_connection_update_meta_store(gobj(), &mcontext, &gerror); ...
Oups, it seems I was a bit too quick in checking if a GdaMetaContext is valid... I'll correct this tonight. Sorry.
This has now been corrected in git master.
(In reply to comment #21) > This has now been corrected in git master. Thanks. Things seem to be working now, though I need to check that we are doing everything that we should be doing now. Could we have a libgda 4.1 tarball release with that, please? It's probably best to keep it out of libgda 4.0 at least for now.
This is finally fixed now, after several bug-fixes and some new API in libgda and adaptation of libgdamm and Glom.