Jim Starkey wrote:
Jay Pipes wrote:
Diego Medina wrote:
Hi,
After converting the unix_timestamp built-in function into a plugin, I
started to get a parser test failure. I looked into it and I found
that:
(looking at drizzled/error.cc )
1) When a built-in function gets the wrong number of parameters, you
get this error
"Incorrect parameter count in the call to native function ..."
2) When the same error occurs on a plugin (at least a udf plugin),
you get
"Incorrect parameters in the call to native function"
My question:
A) Do I just change the test, to expect the new error message?
B) Do I make both, the built-in functions and the udf's, use the same
error?
I would vote to make only a single error message for right now.
Would it make sense to rethink error messages, codes, and all that? It
would be really nice if:
1. Error codes were stable across releases (and forks!)
Perhaps, though I don't view this as much of a priority...there could
always be a translation piece if needed...
2. Error codes from the server, engines, and plugins were mutually
exclusive
+10
The current design is pretty bad in this regard. There should be some
range of valid values for plugin-specific errors.
3. A client program could decompose an error code to determine its
source (server, engine, or plugin)
See above, agreed. Though, in Drizzle-land, an engine is always a
plugin; so I would just say "kernel error" and "plugin error".
4. Locale specific error text could be computed from error code (even
on the client)
Our error messages are already translated/localized via gettext. We do
not use the errmsg file info system from MySQL.
5. A client program could decompose an error code to determine, where
possible, a generic type (i.e. file open failed, memory was
exhausted, etc.)
Sure.
On a related note, the specific error message that Diego originally
wrote about could be even more descriptive. I think this would be
better for the user, no?
_("Incorrect number of arguments supplied to function \"%s\". Expected
between %" PRIu32 " and %" PRIu32 " arguments but received %" PRIu32 ".")
However, to pull the above off properly, we need to rework the current
wobbly-looking code that validates function arguments. The
check_argument_count() method of Item_func in drizzled/function/func.h
is a virtual method that is currently only overridden in functions
defined in the plugin directory. This method is automatically called
for all created UDF objects (classes which inherit from Item_func...)
during the fix_fields_and_dec() process for the Item objects.
I think with a fairly simple change, we can clean this interface up a
bit. I propose adding two protected member variables to Item_func:
protected:
uint32_t min_num_args;
uint32_t max_num_args;
And have a new protected method hasCorrectArguments():
Item_func::hasCorrectArguments() const
{
return (arg_count >= min_num_args && arg_count <= max_num_args);
}
And change the existing check_argument_count() method to the following:
Item_func::checkArguments()
{
if (! hasCorrectArguments())
{
my_error(ER_WRONG_PARAMCOUNT_TO_FUNCTION,
_("Incorrect number of arguments supplied to function "
"\"%s\". Expected between %" PRIu32 " and %" PRIu32 "
" arguments but received %" PRIu32 "."),
func_name(),
min_num_args,
max_num_args,
arg_count);
}
}
Thoughts?
-jay
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
