After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 587051 - Glom can't handle table names containing capital letters
Glom can't handle table names containing capital letters
Status: RESOLVED FIXED
Product: glom
Classification: Other
Component: design
git master
Other Linux
: Normal normal
: ---
Assigned To: Murray Cumming
Murray Cumming
Depends on: 589102 589822
Blocks:
 
 
Reported: 2009-06-26 13:35 UTC by Armin Burgmeier
Modified: 2009-08-26 14:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Confirmation of quotation of table names containing capital letters in the GdaMetaStore (1.11 KB, application/octet-stream)
2009-06-29 17:15 UTC, Armin Burgmeier
Details
Testcase for quoting table names to update the meta store (2.28 KB, application/octet-stream)
2009-07-01 08:37 UTC, Armin Burgmeier
Details

Description Armin Burgmeier 2009-06-26 13:35:17 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.
Comment 1 Murray Cumming 2009-06-26 13:39:20 UTC
This is not intended. We should just fix the bug.
Comment 2 Murray Cumming 2009-06-29 13:09:57 UTC
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.
Comment 3 Armin Burgmeier 2009-06-29 17:15:56 UTC
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.
Comment 4 Murray Cumming 2009-06-30 11:48:18 UTC
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?
Comment 5 Murray Cumming 2009-06-30 15:18:24 UTC
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)
Comment 6 Armin Burgmeier 2009-06-30 15:24:39 UTC
(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.
Comment 7 Murray Cumming 2009-06-30 15:34:58 UTC
No, I think escaping is a separate issue. And that function already does to-lowercase conversion, which is very annoying.
Comment 8 Armin Burgmeier 2009-06-30 15:41:28 UTC
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".
Comment 9 Armin Burgmeier 2009-06-30 18:51:10 UTC
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.
Comment 10 Murray Cumming 2009-07-01 07:07:21 UTC
(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.


Comment 11 Murray Cumming 2009-07-01 07:18:49 UTC
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.
Comment 12 Armin Burgmeier 2009-07-01 07:49:57 UTC
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.
Comment 13 Murray Cumming 2009-07-01 07:57:55 UTC
I tried both. I'm doing a make clean all.
Comment 14 Murray Cumming 2009-07-01 08:10:37 UTC
Armin, are you using the latest libgdamm from git?
Comment 15 Armin Burgmeier 2009-07-01 08:16:05 UTC
Yes, but there doesn't seem to have been a relevant change recently in libgdamm. Did you maybe forget to push a commit?
Comment 16 Murray Cumming 2009-07-01 08:19:12 UTC
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.
Comment 17 Armin Burgmeier 2009-07-01 08:37:04 UTC
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.
Comment 18 malerba 2009-07-05 15:53:43 UTC
This specific point is now corrected in git (master and LIBGDA_4.0 branches).
Comment 19 Murray Cumming 2009-07-07 00:00:31 UTC
(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);
  ...




Comment 20 malerba 2009-07-07 06:22:01 UTC
Oups, it seems I was a bit too quick in checking if a GdaMetaContext is valid... I'll correct this tonight. Sorry.
Comment 21 malerba 2009-07-07 19:11:49 UTC
This has now been corrected in git master.
Comment 22 Murray Cumming 2009-07-10 16:17:59 UTC
(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.  

Comment 23 Murray Cumming 2009-08-26 14:52:43 UTC
This is finally fixed now, after several bug-fixes and some new API in libgda and adaptation of libgdamm and Glom.