Jay Pipes wrote:
> 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.

There is a potential weirdness here though. Currently, error messages
are composed server side and then sent over the wire to the client. This
means that error messages from the server are going to be in the locale
of the server, not of the client.

In the general case, this is fairly unlikely to be too much of a
problem, as actual multi-locale environments _between_ _developers_ are
going to be pretty rare, and there is usually a lingua franca between
them. (and woe betide anyone who just passes database error strings to
the end user and considers it good design.)

BUT, once we hit multi-tenancy stuff, doing the translations of strings
sent from the server to the client server side is going to stop being
the right place to do it. In that case, I agree with Jim's original idea
here (or some portion of my interpretation of it) which is that if we
send an error _code_ from the server, and the _client_ grabs the code
and produces text, then the gettext localization can happen client side.
This does mean that for errors that need parameters to be sensible, we
might need to be able to return an error and a list of one or more items
of data about that error. (like key-value pairs - or even a table...)

So perhaps:

Successful query:
RETURN: 0
TABLE: (id, name, address), (1, "foo", "123 main st"), (2, "bar", "234
elm st")

Unsuccessful query:
RETURN: 12
TABLE: (column, table), ("bar", "foo")

The client gets return code 12, which is "Column not found in table" and
produces the error message "column not found in table". If the developer
_cares_ - they can pull the column and the table out of the result set?

OR SOME OTHER SCHEME that doesn't involve being tied to the need to
embed %s into a printf string somewhere on the server side.

>>   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?

Yes. I believe this was on a todo list at some point.

Monty

_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help   : https://help.launchpad.net/ListHelp

Reply via email to