lib_mysqludf_str: functions fail when parameter is in a table field.

[Steve]

Hi there,
I've been banging my head against a wall all afternoon trying to get
lib_mysqludf_str to work. Firstly it was Ubuntu's apparmor profiles
preventing mysql from loading a library from /usr/local/lib. Then the
configure/make scripts packaged with lib_mysqludf_str produced a
library that didn't contain any functions, but the (much simpler)
compiler options given in the source worked:
gcc -Wall -I/usr/include/mysql -shared lib_mysqludf_str.c -
olib_mysqludf_str.so

Now the library loads and I've created the functions. Calling the
functions directly with suitable strings works fine. What doesn't is
when I want to use the function with data held in a table. I've
created a test table containing a single varchar field, and populated
it with a couple of rows:

mysql> select * from test;
+--------+
| string |
+--------+
| test1 |
| test2 |
| test3 |
+--------+

If I try and run one of the functions on that:

mysql> select str_ucwords(string) from test;
ERROR:
str_ucwords requires one string argument

I looked at the code and modified str_ucwords_init to show which of
the failure conditions were being met, and the cause of the failure
was args->args[0]==NULL. I then printf-ed "args: %s",args->args[0] to
check:
ERROR:
args: (null)

But yet:
mysql> select udf_arg_value_is_null(string) from test;
+-------------------------------+
| udf_arg_value_is_null(string) |
+-------------------------------+
| 0 |
| 0 |
| 0 |
+-------------------------------+

The function works as expected if I select str_ucwords("a test
string"); so it's clearly just when the data is in a table.
So, is there a bug in the library? Something wrong with the way I
compiled it (I used exactly the same compiler commands to compile
lib_mysqludf_udf) or some other weirdness? Why is the original
function saying the expression is null, when what appears to be
exactly the same test in lib_mysqludf_udf is returning non-null?

I'm aware this has got a bit long, but hopefully I've included all
relevant information and test input and output.
Anything else I should try?

Cheers,
Steve.

[Roland Bouman]

Hi Steve,]

(Claudio - are you reading this too? I can do the fixes, but maybe you
want to take a look yourself. Thanks)

> I've been banging my head against a wall all afternoon trying to get
> lib_mysqludf_str to work. Firstly it was Ubuntu's apparmor profiles
> preventing mysql from loading a library from /usr/local/lib. Then the
> configure/make scripts packaged with lib_mysqludf_str produced a
> library that didn't contain any functions, but the (much simpler)

Sorry to hear that. I don't know too much about the autotools autoconf
stuff. I just went ahead and compiled using a gcc line like you.

> mysql> select str_ucwords(string) from test;
> ERROR:
> str_ucwords requires one string argument

I noticed an error in the c source file. In the declarations in the
top, i suspect that this set:

DLLEXP
my_bool str_ucwords_init(UDF_INIT *initid, UDF_ARGS *args, char
*message);
DLLEXP
void str_ucfirst_deinit(UDF_INIT *initid);
DLLEXP
char *str_ucfirst(UDF_INIT *initid, UDF_ARGS *args, char *result,
unsigned long *res_length, char *null_value, char *error);

Should have been:

DLLEXP
my_bool str_ucfirst_init(UDF_INIT *initid, UDF_ARGS *args, char
*message);
DLLEXP
void str_ucfirst_deinit(UDF_INIT *initid);
DLLEXP
char *str_ucfirst(UDF_INIT *initid, UDF_ARGS *args, char *result,
unsigned long *res_length, char *null_value, char *error);

(So, the init function for ucwords was accidentally declared twice.
This would prevent the str_ucfirst function from being available)

Second, I found an error in the argument checking of ucwords:

my_bool str_ucwords_init(UDF_INIT *initid, UDF_ARGS *args, char
*message)
{
/* make sure user has provided exactly one string argument */
if (args->arg_count != 1 || args->arg_type[0] != STRING_RESULT ||
args->args[0] == NULL)
{
strcpy(message,"str_ucwords requires one string argument");
return 1;
}

/* str_ucwords() will not be returning null */
initid->maybe_null=0;

return 0;
}

Well, the check that goes:

args->args[0] == NULL

is wrong. In the init function,

args->args[0] == NULL

means that the argument is not a constant.

Clearly, this is wrong. It should be ok to pass variable values.

I found more problems: the main function

char *str_ucwords(UDF_INIT *initid, UDF_ARGS *args,
char *result, unsigned long *res_length,
char *null_value, char *error)

does not check if the argument has a NULL value. IMO, it should return
NULL in this case.
Second, this line:

// copy the argument string into result
strncpy(result, args->args[0], args->lengths[0]);

is an accident waiting to happen. The result buffer is 255 bytes
long.
The code currently does not check the length of the argument.
So, if args->args[0] > 255 then we will likely get a crash of the
server.

IMO, what should happen is that the init function should allocate
sufficient memory:

my_bool str_ucwords_init(UDF_INIT *initid, UDF_ARGS *args, char
*message)
{
/* make sure user has provided exactly one string argument */
if (args->arg_count != 1 || args->arg_type[0] != STRING_RESULT)
{
strcpy(message,"str_ucwords requires one string argument");
return 1;
}
if (! (initid->ptr = malloc(args->lengths[0])) )
{
strcpy(message,"str_ucwords could not allocate memory");
return 1;
}
return 0;
}

and in the row level function, this memory should be used rather than
result.

[Claudio Cherubino]

Hi Roland,
it seems like my code is a mess! :)

Sorry for my limited participation in the group's activity, but I've
been quite busy recently. Actually, I accepted a new job offer some
months ago and, mostly, I'll get married soon!
As you can imagine there is not much time left for anything...

Please, feel free to apply all the changes you listed to my code, and
thanks for fixing all the bugs.
All the best

Claudio

[Roland Bouman]

Hi Claudio,

> it seems like my code is a mess! :)

heh, np. Part of this group is finding out what the best ways are,
what to avoid etc.

> Sorry for my limited participation in the group's activity, but I've

np.

> been quite busy recently. Actually, I accepted a new job offer some
> months ago and, mostly, I'll get married soon!

Great news! Good luck with the new job, and the best wishes for you
and your wife to be ;)

> As you can imagine there is not much time left for anything...
>
> Please, feel free to apply all the changes you listed to my code, and
> thanks for fixing all the bugs.

Ok. I will try and package a new c source file. Unfortunately automake
and friends is still black magic to me - no idea were to start with
that.

> All the best
>
> Claudio

[Steve]

Many thanks for the speedy suggestions and fix, removing the
args[0]==NULL clause has fixed the problem (it's also in the _init
functions for some of the others too).
Another implementation issue that I noticed in ucwords but can't think
of how to get round it is treatment of words containing apostrophes.
Currently "there's" gets converted to "There'S". However the flip
side is when you have a name such as "o'brien", you *do* want the
letter after the apostrophe to be capitalised so it becomes
"O'Brien"... (I'm only capitalising names so it's currently perfect!)

Thanks also for setting up the MySQL UDF Repository itself and for
taking time to write the UDFs, it is much appreciated!

And Claudio, best of luck with the job and married life!

Cheers,
Steve.