On Mon, May 6, 2013 at 3:23 PM, Marc Delisle
<marc(a)infomarc.info> wrote:
Le 2013-05-06 09:46, Isaac Bennetch a écrit :
On Sun, May 5, 2013 at 7:45 AM, Marc Delisle
<marc(a)infomarc.info
<mailto:marc@infomarc.info>> wrote:
Le 2013-05-04 10:54, Isaac Bennetch a écrit :
Hi,
Can I get some thoughts on [1]? The way the code exists
currently, you
can insert bad data (you'll get a standard
MySQL warning after the
insert). For example, inserting the output of the MD5() function
in to a
TINYINT column. This pull request implements the
AES_ENCRYPT
function,
> but silently fails to show the salt form field in the event the
user
> tries to insert to an invalid column
type...so it looks broken in
the
case the
user is trying to do something that's a bad idea.
In keeping with the current system, I thought it should always
show the
> salt field and proceed with the insert regardless of the column
type,
> however that isn't the most
user-friendly idea. To keep users most
> happy, I wondered if we should implement some sanity checks for
some
column
types and show an inline warning as needed (similar to the
recently merged warning when creating a new user with the same
username). Some of the functions require or suggest certain
column types
which we could easily check, but I'm not sure
it's phpMyAdmin's
responsibility to constantly warn the user in this case.
I added a comment in the pull request. I also asked Garvin about
this,
let's wait for his insight.
I got
a reply from Garvin. He thinks that if the mapping between the
function list and the columns type is actively maintained, there is no
reason to show all possible functions in the drop-down.
The more I think about it, the more I prefer to show a warning message. Not
showing all functions will likely confuse the user ("using table foo I see
MD5() and AES_ENCRYPT(), with table bar I only see NOW()" ) -- so I propose
that the appropriate way to resolve the situation is to always show all
available functions, but to add a popup warning that "The function you have
selected may not be compatible with the column type."
Doing so would require some method of mapping functions to compatible
column types, but that list shouldn't change much so I don't see a problem
with that.