Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-5772

Granting multiple roles in single statement does not work

    Details

    • Type: Bug
    • Status: Stalled
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 10.0.8
    • Fix Version/s: 10.1
    • Component/s: None
    • Labels:
      None

      Description

      One can create several roles at once:

      MariaDB [test]> create role r1, r2;
      Query OK, 0 rows affected (0.01 sec)
      
      

      One can grant multiple privileges to multiple users at once:

      MariaDB [test]> grant SELECT,INSERT on db.* to foo@localhost, bar@localhost;
      Query OK, 0 rows affected (0.00 sec)
      
      

      But one cannot grant multiple roles at once:

      MariaDB [test]> grant r1, r2 to foo@localhost;
      ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ' r2 to foo@localhost' at line 1
      

      SQL standard seems to think that it should work.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              cvicentiu Vicentiu Ciorbaru added a comment - - edited

              I've started working on this and I'm somewhat stuck at the moment with the following grammar extension issue:

              I've redefined GRANT grant_role TO .... to
              GRANT role_list TO ...
              This leads to 7 reduce/reduce conflicts, because:

              We define

              • object_privilege_list as:
                object_privilege_list:
                      object_privilege
                    | object_privilege_list ',' object_privilege
                
              • role_list as:
                role_list:
                      grant_role
                    | role_list ',' grant_role
                

              Within these two rules we allow for keywords such as EVENT, EXECUTE, FILE, PROCESS, RELOAD, SHUTDOWN.

              Technically, the two grant statements should be unambigous:

              GRANT EVENT, EXECUTE ON .....
              GRANT EVENT, EXECUTE TO .....
              

              These 2 statements are different. The first one refers to object privileges, while the other one refers to role grants. Bison finds these to be ambiguous because it needs to make a decision on which rule (object_privilege_list or role_list) to use, when it encounters the ',' (comma) token.
              The decision would be clear, if the parser would look for the TO or ON keyword, but the lookahead token only looks forward one step.

              Possible solutions:
              Do not allow EVENT, EXECUTE, FILE, etc. as role names. (It will break people's existing databases on update).
              Redefine both rules to account for this overlap. The downside to this approach is that it requires a pretty big amount of changes.

              Show
              cvicentiu Vicentiu Ciorbaru added a comment - - edited I've started working on this and I'm somewhat stuck at the moment with the following grammar extension issue: I've redefined GRANT grant_role TO .... to GRANT role_list TO ... This leads to 7 reduce/reduce conflicts, because: We define object_privilege_list as: object_privilege_list: object_privilege | object_privilege_list ',' object_privilege role_list as: role_list: grant_role | role_list ',' grant_role Within these two rules we allow for keywords such as EVENT, EXECUTE, FILE, PROCESS, RELOAD, SHUTDOWN. Technically, the two grant statements should be unambigous: GRANT EVENT, EXECUTE ON ..... GRANT EVENT, EXECUTE TO ..... These 2 statements are different. The first one refers to object privileges, while the other one refers to role grants. Bison finds these to be ambiguous because it needs to make a decision on which rule (object_privilege_list or role_list) to use, when it encounters the ',' (comma) token. The decision would be clear, if the parser would look for the TO or ON keyword, but the lookahead token only looks forward one step. Possible solutions: Do not allow EVENT, EXECUTE, FILE, etc. as role names. (It will break people's existing databases on update). Redefine both rules to account for this overlap. The downside to this approach is that it requires a pretty big amount of changes.
              Hide
              serg Sergei Golubchik added a comment -

              I wouldn't like the first solution. The second is straightforward, but requires a notable amount of changes. Like, create one rule that accepts a list of identifiers or privileges. Create a structure that can accumulate that list and at the end — when you see ON or TO — decide how to parse it. There are details, like having a column list forces the structure to be interpreted as a list of privileges, etc. One would need to remember positions in the list for every word t be able to do correct error messages. All in all not while not complex, but still a big change.

              Taking into account that this is a minor task, I'd postpone it.

              Show
              serg Sergei Golubchik added a comment - I wouldn't like the first solution. The second is straightforward, but requires a notable amount of changes. Like, create one rule that accepts a list of identifiers or privileges. Create a structure that can accumulate that list and at the end — when you see ON or TO — decide how to parse it. There are details, like having a column list forces the structure to be interpreted as a list of privileges, etc. One would need to remember positions in the list for every word t be able to do correct error messages. All in all not while not complex, but still a big change. Taking into account that this is a minor task, I'd postpone it.
              Hide
              cvicentiu Vicentiu Ciorbaru added a comment -

              I agree that the first option is rather undesirable. I thought that there might be a simpler implementation but I guess there isn't one. I'll keep it as a back burner.

              Show
              cvicentiu Vicentiu Ciorbaru added a comment - I agree that the first option is rather undesirable. I thought that there might be a simpler implementation but I guess there isn't one. I'll keep it as a back burner.

                People

                • Assignee:
                  cvicentiu Vicentiu Ciorbaru
                  Reporter:
                  elenst Elena Stepanova
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  5 Start watching this issue

                  Dates

                  • Created:
                    Updated:

                    Time Tracking

                    Estimated:
                    Original Estimate - Not Specified
                    Not Specified
                    Remaining:
                    Remaining Estimate - Not Specified
                    Not Specified
                    Logged:
                    Time Spent - 1 day
                    1d