Hi, Sergey! On Oct 23, Sergey Petrunya wrote:
Hello,
Could somebody please review the below? It's a very simple patch.
I also need comments about which version this should go into. I've made the patch againist 5.1 just in case, but my opinion is that this should go into 5.2 (under rationale that 5.1 is stable and one can live without this patch).
as discussed on the phone - it should go in 5.3 Now, the patch. Disclaimer - I didn't grep for error messages to verify that you patched all places where ER_TABLEACCESS_DENIED_ERROR or ER_COLUMNACCESS_DENIED_ERROR are used. I am sure you have done it yourself already :) There is only one problem. - eng "%-.16s command denied to user '%-.48s'@'%-.64s' for table '%-.192s'" + eng "%-.16s command denied to user '%-.48s'@'%-.64s' for table '%-.192s'.'%-.192s'" you've added a new %s argument to an existing error message. Generally it's something we are never supposed to do. It makes error message files backward incompatible - old mysqld running on new .sys files will most certainly crash. In our - MariaDB - case it also means that running vanilla MySQL on our .sys files will crash too. I'd prefer you to keep old error message untouched, even if inconvenient. Two possible solutions. Add new error message with the table and db name. Or use an utility formatting function, like my_error(ER_COLUMNACCESS_DENIED_ERROR, MYF(0), "ANY", thd->security_ctx->priv_user, - thd->security_ctx->host_or_ip, field_name, tab); + thd->security_ctx->host_or_ip, field_name, format_name(db, tab, buf)); where format_name() will do something like sprintf(buf, "%s'.'%s", db, tab); return buf; This second solution is, I think, preferred, as it simplifies future merges of errmst.txt - in particular, it won't cause problems with shifted error numbers.
MWL#152: Show database name in error messages - Add database name to ER_TABLEACCESS_DENIED_ERROR and ER_COLUMNACCESS_DENIED_ERROR message strings. - Amend the code to provide db name - Update test results
Regards, Sergei