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 411811 - Allow updates from SELECT queries.
Allow updates from SELECT queries.
Status: RESOLVED FIXED
Product: libgda
Classification: Other
Component: general
3.99.x
Other All
: Normal enhancement
: ---
Assigned To: malerba
gnome-db Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-25 06:50 UTC by Yaakov Selkowitz
Modified: 2008-10-25 09:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gda-mdb-provider.c patch (1.44 KB, patch)
2007-02-26 03:17 UTC, Yaakov Selkowitz
committed Details | Review

Description Yaakov Selkowitz 2007-02-25 06:50:12 UTC
The gda_data_model_get_command_text() function, removed from libgda/gda-data-model.c on 2005-09-21 and moved to libgda/gda-data-model-private.h (as a declaration without any code), is still used in 2.99.5 by three providers:

providers/ldap/gda-ldap-recordset.c
providers/msql/gda-msql-recordset.c
providers/oracle/gda-oracle-recordset.c

This makes for undefined symbols in these providers, which prevents linking on Cygwin and must cause runtime errors on other platforms.
Comment 1 Yaakov Selkowitz 2007-02-26 03:17:36 UTC
Created attachment 83348 [details] [review]
gda-mdb-provider.c patch

In addition, providers/mdb/gda-mdb-provider.c still uses gda_handler_bin_new_with_prov() instead of gda_handler_bin_new().  Patch attached.
Comment 2 Murray Cumming 2007-02-28 17:28:36 UTC
Thanks. I have committed that patch.

The problem with the use of the non-existant gda_data_model_get_command_text() remains.
Comment 3 Murray Cumming 2007-02-28 17:40:49 UTC
I see that the latest tarball really is that old. Ubuntu (or maybe Debian) must be packaging code from the mdbtools CVS. If mdbtools is not being maintained then it's probably not a big priority for us.

But maybe some #ifdefing or comments in the code would be useful. 
Comment 4 Yaakov Selkowitz 2007-03-16 19:00:14 UTC
> The problem with the use of the non-existant gda_data_model_get_command_text()
> remains.

I'm not sure how to fix this.
Comment 5 Murray Cumming 2007-03-19 09:31:08 UTC
Please just move the necessary declarations into the relevant public header, adding a comment in the private header about where they are now declared, and documenting them (in the gtk-doc) comments as only for use by provider implementations. I remember seeing an email from Vivien saying that he planned to do this, but it does not seem to have happened.

Please try not to make more functions public than necessary. Some of these private functions do not seem to be used at all.

Thanks.

Comment 6 Yaakov Selkowitz 2007-03-19 14:47:14 UTC
A grep doesn't show any corresponding code for these declarations.
Comment 7 Murray Cumming 2007-03-19 14:49:30 UTC
True. I see no implementation for gda_data_model_get_command_text(). I guess we need to revive it from a previous version.
Comment 9 Murray Cumming 2007-03-19 15:53:37 UTC
So, all we need is this implementation:

const gchar * 	 
gda_data_model_get_command_text (GdaDataModel *model) 	 
{ 	 
	g_return_val_if_fail (GDA_IS_DATA_MODEL (model), NULL); 	 
	  	 
	if (GDA_DATA_MODEL_GET_CLASS (model)->i_get_command) 	 
		return (GDA_DATA_MODEL_GET_CLASS (model)->i_get_command) (model, NULL); 	 
	else 	 
		return NULL; 	 
}

But the i_get_command vfunc no longer exists, so this no longer makes sense.


I am now inclined to just disable the ldap provider, because this is the only one that uses it. I will comment the code to point out where it no longer builds.

Comment 10 Murray Cumming 2007-03-19 15:59:28 UTC
Hmm, unfortunately it's used in the MySQL and Oracle providers too.
Comment 11 Yaakov Selkowitz 2007-03-19 16:18:20 UTC
i_get_command was removed in 
http://svn.gnome.org/viewcvs/libgda/trunk/libgda/gda-data-model.h?r1=2578&r2=2593
Comment 12 malerba 2007-03-19 16:25:13 UTC
This function call (gda_data_model_get_command_text) was removed because it does not make sense to have such a call for all the GdaDataModel implementations (for example for the data model importing data from a CSV file). 

Moreover this function call is used exclusively for code which update a data model returned from a SELECT query, and one of the post 3.0 task is to remove that (non correctly working) feature and replace it with something working. I'll disable that code for the time being and for the 3.0 release.
Comment 13 Yaakov Selkowitz 2007-03-19 16:34:46 UTC
So what does that mean for the three affected providers?
Comment 14 malerba 2007-03-19 17:21:36 UTC
It just means you can't update a GdaDataModel object you got from executing a SELECT command. This feature has never worked correctly, so it's better if it's disabled until it works correctly.
Comment 15 Yaakov Selkowitz 2007-03-19 17:38:26 UTC
So you will patch this?
Comment 16 malerba 2007-03-19 17:42:27 UTC
It's in SVN.
Comment 17 Yaakov Selkowitz 2007-03-19 20:55:03 UTC
Great.  r2870 builds OOTB on Cygwin with all 9 possible providers.

See you all in bug 419063, hopefully the last blocker for 3.0.
Comment 18 Murray Cumming 2007-03-19 20:59:22 UTC
Vivien has changed the code so that it no longer uses the callbacks that use gda_data_model_get_command_text(). I have #ifdef-ed out the definitions of the functions that used gda_data_model_get_command_text(), to prevent linker errors, and added comments pointing back to this bug report.

Interestingly, some of the other providers do have *recordset_append_values() implementations that don't seem to need gda_data_model_get_command_text().
Comment 19 malerba 2007-03-20 08:17:14 UTC
Providers which allow updates to some of the GdaDataModel they created when a SELECT statement was executed need to craft "by hand" the INSERT, DELETE and UPDATE queries they need to run in order to actually update the data model. This feature has been present since the early days of libgda, but its completeness has always varied from provider to provider. I plan to rationalize that to make it available at a low cost to all the providers, but that is post 3.0.
Comment 20 Murray Cumming 2008-03-14 11:01:09 UTC
Please don't forget this now that libgda 4 has been started on svn trunk.
Comment 21 Yaakov Selkowitz 2008-07-08 05:36:00 UTC
> Please don't forget this now that libgda 4 has been started on svn trunk.

Update, anyone?
Comment 22 malerba 2008-09-05 12:21:27 UTC
As of rev#3200, it is functionnal and works with any provider: it's implemented in the GdaDataSelect class which is the base class all database providers are required to use when returning a resultset from the execution of a SELECT statement.

I'm in the process of updating the documentation to make the usage easier.
Comment 23 malerba 2008-10-25 09:21:35 UTC
Closing the bug.