Menu

#879 (ok 4.0.0-beta2) reserved word warning

Next_release
fixed
1
2013-06-11
2006-02-23
No

I would like phpmyadmin to at least warn if I'm
creating a table with a column name that is on the
reserved word list.

http://dev.mysql.com/doc/refman/5.0/en/reserved-words.html

Using some reserved words in MySQL < 5 won't cause
problems from the database server but should be
avoided. I believe phpmyadmin could help developers not
to make bad desisions about database design.

Discussion

  • ramaboo

    ramaboo - 2009-03-13

    I second this. This has caused me tons of grief over the years. I would think this would be a supper simple fix.

    I have never worked with the phpmyadmin code base but I know php very well. If someone wants to help I would be happy to help tackle this one.

     
  • shamika

    shamika - 2013-02-23

    So,shouldn't we allow to users create database,tables,column names with reserved words in mysql on this matter?

     
  • Marc Delisle

    Marc Delisle - 2013-02-23

    This is not about blocking users to do so, this is about warning them.

     
  • shamika

    shamika - 2013-02-24

    Marc,
    Okey I get it.So we can do this by checking, the names that the user enters for databases,tables and columns with the reserved Mysql keyword list.Is this a correct way to do this?

     
  • Marc Delisle

    Marc Delisle - 2013-02-24

    Yes, but before you start to type all those reserved words, we already have such a list somewhere (I must stay vague because this feature request is part of the contest).

     
    • mahendra singh meena

      Hi Marc,
      Please review the pull request https://github.com/phpmyadmin/phpmyadmin/pull/182

      1. It involves showing warning message when name for table and columns is a reserved word. (Only for the User Interface, not yet done for the sql query window)
      2. I will start doing the same for SQL query window(if necessary), once you confirms the above part.
       
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-02-24

    Hi,

    I had a few questions:

    1. Does this only apply for Column Names?
    2. Does this also apply for Queries executed from the SQL tab or is this only for fields being added via the phpMyAdmin interface?
    3. What kind of warning should we show? An alert before its created?
     
  • Marc Delisle

    Marc Delisle - 2013-02-25

    Ayush:
    1. no
    2. Showing the warning for actions done via the UI would already be a welcome addition
    3. Makes sense.

     
  • Marc Delisle

    Marc Delisle - 2013-03-06
    • summary: reserved word warning --> (ok 4.0.0-beta2) reserved word warning
    • status: open --> open-fixed
    • assigned_to: Marc Delisle
    • milestone: --> Next_release
    • priority: 5 --> 1
     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-06

    Hi,

    Just a thought: Shouldn't it have been a dismissible warning instead of a persistent warning showing up everytime we go to the Browse or Structure Tabs?

     
  • Marc Delisle

    Marc Delisle - 2013-03-06

    Yes, this would be better. Awaiting your commit ;)

    Another improvement would be to have just one warning naming all the columns.

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-06

    So, we should be basically create this functionality using JS and warn them before they submit the form, correct?

    I'll get to it as soon as my exams are over.

     
  • Marc Delisle

    Marc Delisle - 2013-03-06

    Well, the current implementation warns each time the user goes to Structure or Browse, which might be excessive.

    However, a warning at the js level might be problematic because it's at the PHP level that we have the list of reserved words (from our parser).

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-06

    But having it show only once after creation, might not be very intuitive and even its implementation maybe problematic.

    Another way I think of is to display the warning only till the table is empty and no rows have been entered?

     
  • Marc Delisle

    Marc Delisle - 2013-03-06

    Ayush,
    are you still talking about a JS warning, or a warning at the PHP level like in current master?

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-06

    What I meant was, I think we should give JS warning a thought as that would be more intuitive. If we can manage to store the list of reserved words at a separate location/config (xml, json) we could use it with both PHP and JS. Or maybe an AJAX call right before the submit (this might be expensive though)

    However, if we still have to go with PHP only, I can think of two options:

    1. If we have it display only once, we need to decide how we are going to do it. The only way I can think right now is by using some sort of session flag and display the warning the first time the user visits the Structure page

    2. Instead of showing it once, we could show the warning everytime the user comes to the Structure page until the table is empty. As soon as the table has data, we stop warning.

     
  • Marc Delisle

    Marc Delisle - 2013-03-10

    Ayush,
    Moving the list of words is a major modification and in the current state of version 4, is too major.

    I believe that showing the warning in Browse is excessive. Showing it in Structure is enough, IMO. With such a modification, the warnings would be much less annoying. Also, there could be a configuration directive so that someone can choose to not be warned.

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-10

    Yes, okay. Makes sense.

     
  • Marc Delisle

    Marc Delisle - 2013-03-10

    Ayush,
    will you take care of preparing a patch to remove the warning in Browse mode?

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-10

    Sure, but I can only do it next week, because I have my exams this week 11th through 16th. Will that be fine?

     
  • Marc Delisle

    Marc Delisle - 2013-03-10

    Yes.

     
  • Ayush Chaudhary

    Ayush Chaudhary - 2013-03-10

    Hi Mark,

    I sneaked out some time, and here is the PR: https://github.com/phpmyadmin/phpmyadmin/pull/209

     
  • Marc Delisle

    Marc Delisle - 2013-03-21

    Pull request 216 merged, thanks.

     
  • Michal Čihař

    Michal Čihař - 2013-05-09
    • Status: open-fixed --> closed-fixed
     
  • Michal Čihař

    Michal Čihař - 2013-06-11
    • Status: closed-fixed --> fixed