Re: bk commit into 5.0 tree (antony:1.2492) BUG#29019

Hi Antony,

antony@mysql.com wrote:
...
> ChangeSet@1.2492, 2007-06-19 16:58:27-07:00, antony@ppcg5.local +4 -0
> Bug#29019
> "REPLACE/INSERT IGNORE/UPDATE IGNORE doesn't work"
> Federated does not record neccessary HA_EXTRA flags in order to
> support REPLACE/INSERT IGNORE/UPDATE IGNORE.
> Implement ::extra() to capture flags neccessary for functionality.
> New function append_ident() to better escape identifiers.
...
> +/**
> + @brief Append identifiers to the string.
> + @param string The target string.
> + @param name Identifier name
> + @param length Length of identifier name in bytes
> + @param quote_char Quote char to use for quoting identifier.
> + @return FALSE if successful
> + @note This function is based upon the append_identifier() function
> + in sql_show.cc except that quoting always occurs.
> +*/
> +
> +static bool append_ident(String *string, const char *name, uint length,
> + const char quote_char= '`')

Small problems with the function comment. Empty lines between sections. @param miss [in], [out], or [in,out] qualifiers. @retval missing.

You don't use the parameter 'quote_char' anywhere. And this is good. If we want federated to be able to connect to a non-MySQL DBMS one day, we would have a single place where we might need to change the quote char depending on the DBMS.

I suggest to remove the parameter and make it a local variable.

You did not change the appends of field_name in update_row() and delete_row(). Just miss them, or is there a reason?

...
> - query.append(FEDERATED_BTICK);
> - escaped_table_name_length=
> - escape_string_for_mysql(&my_charset_bin, (char*)escaped_table_name,
> - sizeof(escaped_table_name),
> - share->table_name,
> - share->table_name_length);
> - query.append(escaped_table_name, escaped_table_name_length);
> - query.append(FEDERATED_BTICK);
> + append_ident(&query, share->table_name, share->table_name_length);

escape_string_for_mysql() does more than append_ident(). Can we be sure that none of the precautions in escape_string_for_mysql() is required for identifiers?

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

...
> @@ -1309,31 +1344,26 @@

Antony, can you please, please install diff-p-helper, so that I can see behind the last @@ which function this diff belongs to?

I guess it's static FEDERATED_SHARE *get_share().

> query.append(FEDERATED_SELECT);
> for (field= table->field; *field; field++)
> {
> - query.append(FEDERATED_BTICK);
> - query.append((*field)->field_name);
> - query.append(FEDERATED_BTICK);
> + append_ident(&query, (*field)->field_name, strlen((*field)->field_name));
> query.append(FEDERATED_COMMA);
> }
> query.length(query.length()- strlen(FEDERATED_COMMA));
> query.append(FEDERATED_FROM);
> - query.append(FEDERATED_BTICK);
> +
> + tmp_share.table_name_length= strlen(tmp_share.table_name);

Isn't this done in parse_url() already?

...
> +/**
> + @brief Handles extra signals from MySQL server
> + @param operation Signal
> + */
> +int ha_federated::extra(ha_extra_function operation)

Small problems with the function comment. Empty lines between sections. @param misses [in] qualifier. @return and @retval missing. Empty line after comment missing.

...

Do you need more help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

Regards
Ingo

-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140



-- 
MySQL Code Commits Mailing List
For list archives: 
http://lists.mysql.com/commits
To unsubscribe:    
http://lists.mysql.com/commits?unsub=lists@pantek.com