Re: [gnome-db] Libgda Firebird provider patch

[Jeronimo Albi <amenofisvii yahoo com ar>]

Rodrigo Moya wrote:

> > +		{ "double precision", "", "Double precision number", GDA_VALUE_TYPE_DOUBLE }
> >
> >    
> is "double precision" the string you use in SQL commands? This string is used in
> some places (Mergeant IIRC) to create SQL commands for creating tables, so the string
> needs to be exactly what you would use in those SQL commands.
>
>  
 Yes the string for this data type is "DOUBLE PRECISION" unfortunatelly 
:-(.

> > +static GdaDataModel *
> > +fb_get_tables (GdaConnection *cnc,
> > +		GdaParameterList *params,
> > +		gboolean show_views)
> > +{
> > +	GList *reclist, *value_list;
> > +	GdaDataModel *recset = NULL;
> > +	GdaTransaction *transaction;
> > +	GdaCommand *command;
> > +	GdaValue *value;
> > +	GdaRow *row;
> > +	GdaParameter *par = NULL;
> > +	gchar *sql, *sql_cond, *table_name;
> > +	gint i;
> > +	gboolean systables = FALSE;
> > +
> > +	g_return_val_if_fail (GDA_IS_CONNECTION (cnc), NULL);
> > +        
> > +	transaction = gda_transaction_new ("temp_transaction");
> > +	if (gda_connection_begin_transaction (cnc, transaction)) {
> >
> >    
> why do we need a transaction for reading the tables?
Yes Interbase/Firebird is a transactional database, so for almost
any statement it needs an already started transaction. I know it might
be annoying sometimes.

> > gda_firebird_provider_drop_database (GdaServerProvider *provider,
> > -				      GdaConnection *cnc,
> > -				      const gchar *name)
> > +				     GdaConnection *cnc,
> > +				     const gchar *name)
> > {
> > +	GdaFirebirdConnection *fcnc;
> > +    
> > +	g_return_val_if_fail (GDA_IS_FIREBIRD_PROVIDER (provider), FALSE);
> > +	g_return_val_if_fail (GDA_IS_CONNECTION (cnc), FALSE);
> > +
> > +	fcnc = g_object_get_data (G_OBJECT (cnc), CONNECTION_DATA);
> > +	if (!fcnc) {
> > +		gda_connection_add_error_string (cnc, _("Invalid Firebird handle"));
> > +		return FALSE;
> > +	}
> > +    
> > +	/* Connection must be open for database drop */	
> > +	if (gda_connection_is_open (cnc)) {
> > +  		if (isc_drop_database (fcnc->status, &(fcnc->handle))) {
> >
> >    
> this is wrong, you should not drop the current database, but the database specified
> by the 'name' argument. So, usually, you'll just use a DROP DATABASE SQL command.
>  
Here we've got an important issue :-) !! Whe have 2 methods, one is to use
API function "isc_drop_database" an the other is actually with a "DROP
DATABASE" statement. In the first case function need a handle, already
initialized by making a connection to the database we want to drop. In
the second case we need to execute a "DROP DATABASE;" statement on
the current connection. This statement hasnt database name as parameter :-),
the command drops actual database, unlike other engines that accepts a
database name as parameter.

> > +static GdaFieldAttributes *
> > +gda_firebird_recordset_describe_column (GdaDataModel *model, 
> > +					gint col)
> > +{
> > +	return NULL;
> > +}
> > +
> >    
> this function is mandatory, so you should really implement it.
>  
So it'll be a priority :-).

All other issues are corrected.
If you want me to arrange other syntax issues, just tell me.

What do you want me to do with drop_database ? I'll send
patch with correction after you tell me what to do.

Cheer,
Jeronimo