Hi Chuck,
See my comments below which might help you when preparing a new patch.
Rafal
Chuck Bell wrote:
> #At file:///D:/source/bzr/mysql-6.0-wl-4073/
>
> 2652 Chuck Bell 2008-07-07
> WL#4073 : Online Backup: backup and restore database object permissions
>
> Stage 1 of 3: Prepare modifications to si_objects to support:
> - iteration of database-level grant statements
> - creation of a database-level grant object
> - iterators for database-level grant objects
> - utilities to check user and process user/host 'grantee' column for IS table
> added:
> mysql-test/r/backup_db_grants.result
> mysql-test/t/backup_db_grants.test
> modified:
> sql/backup/backup_test.cc
> sql/si_objects.cc
> sql/si_objects.h
>
> per-file messages:
> mysql-test/r/backup_db_grants.result
> New result file.
> mysql-test/t/backup_db_grants.test
> Added new test to test backup and restore of database-level grants.
> Note: test is restricted to using backup_test command until stage
> 2 and 3 are complete.
> sql/backup/backup_test.cc
> Added test code to process database grants.
> Modified test procedure to iterate over list of databases
> passed on command line instead of all databases.
> sql/si_objects.cc
> Added new DBGrantObj class as specified in LLD. This class is used to find,
> iterator over, and serialize or materialize a database grant.
> sql/si_objects.h
> Added new DBGrantObj class declaration.
> === added file 'mysql-test/r/backup_db_grants.result'
> --- a/mysql-test/r/backup_db_grants.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/backup_db_grants.result 2008-07-07 22:14:30 +0000
> @@ -0,0 +1,74 @@
> +DROP DATABASE IF EXISTS bup_db_grants;
> +DROP DATABASE IF EXISTS db2;
> +Create two databases and two tables.
> +CREATE DATABASE bup_db_grants;
> +CREATE TABLE bup_db_grants.t1(a INT);
> +CREATE TABLE bup_db_grants.s1(a INT);
> +INSERT INTO bup_db_grants.t1 VALUES (1), (2), (3), (4);
> +INSERT INTO bup_db_grants.s1 VALUES (10), (20), (30), (40);
> +Now create users and assign various rights to the databases
> +CREATE USER bup_user1;
> +CREATE USER bup_user2;
> +CREATE USER bup_user3;
> +REVOKE ALL ON *.* FROM 'bup_user1'@'%';
> +REVOKE ALL ON *.* FROM 'bup_user2'@'%';
> +GRANT ALL ON db2.* TO 'bup_user1'@'%';
> +GRANT SELECT ON bup_db_grants.* TO 'bup_user1'@'%';
> +GRANT INSERT ON bup_db_grants.* TO 'bup_user2'@'%';
> +GRANT ALL ON bup_db_grants.* TO 'no_user'@'%';
> +FLUSH PRIVILEGES;
> +Demonstrate rights of the users.
> +SHOW GRANTS FOR 'bup_user1';
> +Grants for bup_user1@%
> +GRANT USAGE ON *.* TO 'bup_user1'@'%'
> +GRANT ALL PRIVILEGES ON `db2`.* TO 'bup_user1'@'%'
> +GRANT SELECT ON `bup_db_grants`.* TO 'bup_user1'@'%'
> +SHOW GRANTS FOR 'bup_user2';
> +Grants for bup_user2@%
> +GRANT USAGE ON *.* TO 'bup_user2'@'%'
> +GRANT INSERT ON `bup_db_grants`.* TO 'bup_user2'@'%'
> +SHOW GRANTS FOR 'bup_user3';
> +Grants for bup_user3@%
> +GRANT USAGE ON *.* TO 'bup_user3'@'%'
> +SHOW GRANTS FOR 'no_user';
> +Grants for no_user@%
> +GRANT USAGE ON *.* TO 'no_user'@'%'
> +GRANT ALL PRIVILEGES ON `bup_db_grants`.* TO 'no_user'@'%'
> +db name type serialization
> +bup_db_grants s1 TABLE USE `bup_db_grants`; CREATE TABLE `s1` (
> + `a` int(11) DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +bup_db_grants t1 TABLE USE `bup_db_grants`; CREATE TABLE `t1` (
> + `a` int(11) DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +bup_db_grants 'bup_user1'@'%' GRANT GRANT SELECT ON bup_db_grants TO
> 'bup_user1'@'%'
> +bup_db_grants 'bup_user2'@'%' GRANT GRANT INSERT ON bup_db_grants TO
> 'bup_user2'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT SELECT ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT INSERT ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT UPDATE ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT DELETE ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT CREATE ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT DROP ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT REFERENCES ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT INDEX ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT ALTER ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT CREATE TEMPORARY TABLES ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT LOCK TABLES ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT EXECUTE ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT CREATE VIEW ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT SHOW VIEW ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT CREATE ROUTINE ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT ALTER ROUTINE ON bup_db_grants TO
> 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT EVENT ON bup_db_grants TO 'no_user'@'%'
> +bup_db_grants 'no_user'@'%' GRANT GRANT TRIGGER ON bup_db_grants TO 'no_user'@'%'
> +SHOW TABLES FROM bup_db_grants;
> +Tables_in_bup_db_grants
> +s1
> +t1
> +Cleanup
> +DROP USER no_user;
> +DROP USER bup_user3;
> +DROP USER bup_user2;
> +DROP USER bup_user1;
> +FLUSH PRIVILEGES;
> +DROP DATABASE bup_db_grants;
>
> === added file 'mysql-test/t/backup_db_grants.test'
> --- a/mysql-test/t/backup_db_grants.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/backup_db_grants.test 2008-07-07 22:14:30 +0000
> @@ -0,0 +1,70 @@
> +#
> +# Test the backup of database grants
> +#
> +
> +--source include/not_embedded.inc
> +
> +--disable_warnings
> +DROP DATABASE IF EXISTS bup_db_grants;
> +DROP DATABASE IF EXISTS db2;
> +--enable_warnings
> +
> +--echo Create two databases and two tables.
> +CREATE DATABASE bup_db_grants;
> +CREATE TABLE bup_db_grants.t1(a INT);
> +CREATE TABLE bup_db_grants.s1(a INT);
> +INSERT INTO bup_db_grants.t1 VALUES (1), (2), (3), (4);
> +INSERT INTO bup_db_grants.s1 VALUES (10), (20), (30), (40);
> +
> +--echo Now create users and assign various rights to the databases
> +CREATE USER bup_user1;
> +CREATE USER bup_user2;
> +CREATE USER bup_user3;
> +REVOKE ALL ON *.* FROM 'bup_user1'@'%';
> +REVOKE ALL ON *.* FROM 'bup_user2'@'%';
> +GRANT ALL ON db2.* TO 'bup_user1'@'%';
> +GRANT SELECT ON bup_db_grants.* TO 'bup_user1'@'%';
> +GRANT INSERT ON bup_db_grants.* TO 'bup_user2'@'%';
> +GRANT ALL ON bup_db_grants.* TO 'no_user'@'%';
> +FLUSH PRIVILEGES;
> +
> +--echo Demonstrate rights of the users.
> +SHOW GRANTS FOR 'bup_user1';
> +SHOW GRANTS FOR 'bup_user2';
> +SHOW GRANTS FOR 'bup_user3';
> +SHOW GRANTS FOR 'no_user';
> +
> +# Remove comments when stage 3 of WL#4073 is complete and
> +# remove the calls to BACKUP TEST.
> +#--replace_column 1 #
> +#BACKUP DATABASE bup_db_grants TO 'bup_db_grants.bak';
> +
> +--disable_query_log
> +BACKUP_TEST bup_db_grants;
> +--enable_query_log
> +
> +# Remove comments when stage 3 of WL#4073 is complete and
> +# remove the calls to BACKUP TEST.
> +#--replace_column 1 #
> +#RESTORE FROM 'bup_db_grants.bak';
> +
> +SHOW TABLES FROM bup_db_grants;
> +
> +# Remove comments when stage 3 of WL#4073 is complete and
> +#--echo Demonstrate rights of the users.
> +#SHOW GRANTS FOR 'bup_user1';
> +#SHOW GRANTS FOR 'bup_user2';
> +#SHOW GRANTS FOR 'bup_user3';
> +#SHOW GRANTS FOR 'no_user';
> +
> +--echo Cleanup
> +
> +DROP USER no_user;
> +DROP USER bup_user3;
> +DROP USER bup_user2;
> +DROP USER bup_user1;
> +FLUSH PRIVILEGES;
> +DROP DATABASE bup_db_grants;
> +
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_db_grants.bak
>
> === modified file 'sql/backup/backup_test.cc'
> --- a/sql/backup/backup_test.cc 2008-06-25 13:39:04 +0000
> +++ b/sql/backup/backup_test.cc 2008-07-07 22:14:30 +0000
> @@ -45,14 +45,20 @@ int execute_backup_test_command(THD *thd
> field_list.push_back(new Item_empty_string("serialization", 13));
> protocol->send_fields(&field_list, Protocol::SEND_NUM_ROWS |
> Protocol::SEND_EOF);
>
> - obs::ObjIterator *it= obs::get_databases(thd);
> -
> - if (it)
> + //obs::ObjIterator *it= obs::get_databases(thd);
> + List_iterator<LEX_STRING> it(*db_list);
> +
> + //if (it)
> {
> obs::Obj *db;
>
> - while ((db= it->next()))
> - {
> + //while ((db= it->next()))
> + LEX_STRING *dbname;
> + while ((dbname= it++))
> + {
> + String dir;
> + dir.copy(dbname->str, dbname->length, system_charset_info);
> + db= get_database(&dir);
>
> if (is_internal_db_name(db->get_db_name()))
> continue;
> @@ -224,8 +230,38 @@ int execute_backup_test_command(THD *thd
>
> delete event;
> }
> + }
> +
> + //
> + // List db grants.
> + //
> + tit= obs::get_db_grants(thd, db->get_name());
> +
> + if (tit)
> + {
> + obs::Obj *grant;
> +
> + while ((grant= tit->next()))
> + {
> + String serial;
> + serial.length(0);
> + protocol->prepare_for_resend();
> + protocol->store(const_cast<String*>(db->get_name()));
> + protocol->store(const_cast<String*>(grant->get_name()));
> + String user;
> + String host;
> + String *user_host= (String *)grant->get_name();
> + obs::split_user_host(user_host, &user, &host);
> + protocol->store(C_STRING_WITH_LEN("GRANT"),
> + system_charset_info);
> + grant->serialize(thd, &serial);
> + protocol->store(&serial);
> + protocol->write();
> +
> + delete grant;
> + }
>
> - delete tit;
> + delete tit;
> }
>
> // That's it.
> @@ -234,10 +270,9 @@ int execute_backup_test_command(THD *thd
> }
> }
>
> - delete it;
> +// delete it;
>
> my_eof(thd);
> -// delete i;
> DBUG_RETURN(res);
> }
>
>
> === modified file 'sql/si_objects.cc'
> --- a/sql/si_objects.cc 2008-07-05 08:41:26 +0000
> +++ b/sql/si_objects.cc 2008-07-07 22:14:30 +0000
> @@ -759,6 +759,53 @@ private:
> String m_create_stmt;
> };
>
> +/**
> + @class DbGrantObj
> +
> + This class provides an abstraction to a user object for creation and
> + capture of the creation data.
> +*/
> +class DbGrantObj : public Obj
> +{
> +public:
> + DbGrantObj(const String *db_name,
> + const String *user_name,
> + const String *privilege);
> +
> +public:
> + virtual bool materialize(uint serialization_version,
> + const String *serialization);
> +
> + const String* get_name()
> + { return &m_user_name; }
I would declare get_grantee() method and implement virtual get_name() using it.
That would be more clear. If not, at least a documentation explaining what
get_name() does would be in place.
> +
> + const String *get_db_name()
> + {
> + return &m_db_name;
> + }
> +
> + const String *get_privilege()
> + {
> + return &m_privilege;
> + }
Please document that method (or the m_privilege member). I have no idea what it
might return... Actually I think it should not be in the public interface.
> +
> +private:
> + // These attributes are to be used only for serialization.
> + String m_db_name;
> + String m_user_name;
> + String m_privilege;
> +
> + virtual bool do_serialize(THD *thd, String *serialization);
> +
> + virtual bool do_execute(THD *thd);
> +
> + bool drop(THD *thd);
> +
> +private:
> + // These attributes are to be used only for materialization.
> + String m_create_stmt;
> +};
> +
> ///////////////////////////////////////////////////////////////////////////
>
> //
> @@ -938,6 +985,27 @@ private:
> String m_db_name;
> };
>
> +class DbGrantIterator : public InformationSchemaIterator
> +{
> +public:
> + DbGrantIterator(THD *thd,
> + const String *db_name,
> + TABLE *is_table,
> + handler *ha,
> + my_bitmap_map *orig_columns) :
> + InformationSchemaIterator(thd, is_table, ha, orig_columns)
> + {
> + m_db_name.copy(*db_name);
> + }
> +
> +protected:
> + virtual DbGrantObj *create_obj(TABLE *t);
> +
> +private:
> + String m_db_name;
> +};
> +
> +
> ///////////////////////////////////////////////////////////////////////////
>
> class DbStoredFuncIterator : public DbStoredProcIterator
> @@ -1055,6 +1123,12 @@ bool InformationSchemaIterator::prepare_
> *is_table= open_schema_table(thd, st, NULL);
> break;
> }
> + case SCH_SCHEMA_PRIVILEGES:
> + {
> + st= find_schema_table(thd, "SCHEMA_PRIVILEGES");
> + *is_table= open_schema_table(thd, st, NULL);
> + break;
> + }
> default:
> {
> st= get_schema_table(is_table_idx);
> @@ -1323,6 +1397,42 @@ EventObj *DbEventIterator::create_obj(TA
> ///////////////////////////////////////////////////////////////////////////
>
> //
> +// Implementation: DbGrantIterator class.
> +//
> +
> +///////////////////////////////////////////////////////////////////////////
> +
> +DbGrantObj* DbGrantIterator::create_obj(TABLE *t)
> +{
> + String db_name;
> + String user_name;
> + String privilege;
> +
> + t->field[2]->val_str(&db_name);
> + t->field[0]->val_str(&user_name);
> + t->field[3]->val_str(&privilege);
> +
> + /*
> + The fill method for SCHEMA_PRIVILEGES does not use the COND portion
> + of the generic fill() method. Thus, we have to do the restriction here.
> +
> + Ensure the only rows sent back from iterator are the ones that match the
> + database specified.
> + */
> + if (db_name == m_db_name)
> + {
> + DBUG_PRINT("DbGrantIterator::next", (" Found grant %s.%s %s",
> + db_name.ptr(), user_name.ptr(), privilege.ptr()));
> +
> + return new DbGrantObj(&db_name, &user_name, &privilege);
> + }
> + else
> + return NULL;
> +}
> +
> +///////////////////////////////////////////////////////////////////////////
> +
> +//
> // Implementation: ViewBaseObjectsIterator class.
> //
>
> @@ -1502,6 +1612,10 @@ DbEventIterator *
> create_is_iterator<DbEventIterator>(THD *, enum_schema_tables, const String
> *);
> #endif
>
> +template
> +DbGrantIterator *
> +create_is_iterator<DbGrantIterator>(THD *, enum_schema_tables, const String
> *);
> +
> ObjIterator *get_db_tables(THD *thd, const String *db_name)
> {
> return create_is_iterator<DbTablesIterator>(thd, SCH_TABLES, db_name);
> @@ -1536,6 +1650,11 @@ ObjIterator *get_db_events(THD *thd, con
> #endif
> }
>
> +ObjIterator *get_db_grants(THD *thd, const String *db_name)
> +{
> + return create_is_iterator<DbGrantIterator>(thd, SCH_SCHEMA_PRIVILEGES,
> db_name);
> +}
> +
>
> ///////////////////////////////////////////////////////////////////////////
>
> @@ -2580,6 +2699,118 @@ bool TablespaceObj::do_execute(THD *thd)
>
> ///////////////////////////////////////////////////////////////////////////
>
> +//
> +// Implementation: DbGrantObj class.
> +//
> +
> +/////////////////////////////////////////////////////////////////////////////
> +
> +DbGrantObj::DbGrantObj(const String *db_name,
> + const String *user_name,
> + const String *privilege)
> +{
> + // copy strings to newly allocated memory
> + m_db_name.copy(*db_name);
> + m_user_name.copy(*user_name);
> + m_privilege.copy(*privilege);
> +}
One case where you crate a DbGrantObj is inside the grant iterator. Then you
know the privilege which was read from the I_S table.
But there is also another case where you need to create a DbGrantObj without
knowing the privilege. This happens when you materialize a grant object from its
serialization string. I tink you need a separate constructor for that case,
because during materialziation you have no value to pass for 'privilege'.
> +
> +/**
> + Serialize the object.
> +
> + This method produces the data necessary for materializing the object
> + on restore (creates object).
> +
> + @param[in] thd Thread context.
> + @param[out] serialization The data needed to recreate this object.
> +
> + @note this method will return an error if the db_name is either
> + mysql or information_schema as these are not objects that
> + should be recreated using this interface.
> +
Why privileges to the internal tables should not be serializable using this
interface? The fact that we don't do it in backup kernel is not enough to
exclude such possibility (especially if implementing the restiction means extra
work).
In the kernel we will simply call is_internal_db_name() to detect an internal
database and not query or execute privileges for such databases.
> + @returns Error status.
> + @retval FALSE on success
> + @retval TRUE on error
> +*/
> +bool DbGrantObj::do_serialize(THD *thd, String *serialization)
> +{
> + DBUG_ENTER("DbGrantObj::do+serialize()");
> + serialization->length(0);
> + serialization->append("GRANT ");
> + serialization->append(m_privilege);
> + serialization->append(" ON ");
> + serialization->append(m_db_name);
> + serialization->append(" TO ");
> + serialization->append(m_user_name);
> + DBUG_RETURN(0);
> +}
You create a complete GRANT statement here but I don't see a good reason for
that, except that it will make the grant entries stored in the backup image
"executable". The price to pay is duplication of information (as user name and
db name will be stored separately) and waste of space in the backup image
(although neglible, I can agree).
A simpler and, in my opinion, better solution would be to just store the
m_privilege string as serialization of the grant. A complete GRANT statement
will be constructed inside materialize() or do_execute() method.
Apart from being more compact, such solution will make it easy to manipulate the
stored grants if we need that in the future. E.g., apply the grants to a
different database or for different user.
In your current implementation, it looks that the constructed grant statement is
incorrect. For example if in a grant object we have:
m_privilege = "select"
m_db_name = "test"
m_user_name = "chuck@localhost"
then the following statement will be constructed:
GRANT select ON test TO chuck@localhost
which is not correct. What you want probably is:
GRANT select ON test.* TO chuck@localhost .
> +
> +/**
> + Materialize the serialization string.
> +
> + This method saves serialization string into a member variable.
> +
> + @param[in] serialization_version version number of this interface
> + @param[in] serialization the string from serialize()
> +
> + @todo take serialization_version into account
> +
> + @returns Error status.
> + @retval FALSE on success
> + @retval TRUE on error
> +*/
> +bool DbGrantObj::materialize(uint serialization_version,
> + const String *serialization)
> +{
> + DBUG_ENTER("DbGrantObj::materialize()");
> + m_create_stmt.copy(*serialization);
> + DBUG_RETURN(0);
> +}
If serialization string contains just a list of privileges, the m_create_stmt
could be constructed here as it is done in do_serialize() now.
> +
> +/**
> + Create the object.
> +
> + This method uses serialization string in a query and executes it.
> +
> + @param[in] thd Thread context.
> +
> + @returns Error status.
> + @retval FALSE on success
> + @retval TRUE on error
> +*/
> +bool DbGrantObj::do_execute(THD *thd)
> +{
> + DBUG_ENTER("DbGrantObj::do_execute()");
> + drop(thd);
> + DBUG_RETURN(execute_with_ctx(thd, &m_create_stmt, true));
> +}
> +
> +/**
> + Drop the object.
> +
> + This method calls the silent_exec method to execute the query.
> +
> + @note This uses "IF EXISTS" and does not return error if
> + object does not exist.
> +
> + @param[in] thd Thread context.
> + @param[out] serialization the data needed to recreate this object
> +
> + @returns Error status.
> + @retval FALSE on success
> + @retval TRUE on error
> +*/
> +bool DbGrantObj::drop(THD *thd)
> +{
> + DBUG_ENTER("DbGrantObj::drop()");
> + DBUG_RETURN(drop_object(thd,
> + (char *) "USER",
> + &m_db_name,
> + &m_user_name));
I think this tries to execute the following statement:
DROP USER IF EXISTS <db name>.<user name>
I'm not even sure if this is syntacticly correct (can we qualify user names wth
db names?). But, more importantly, this is not what we want to do. We don't want
to drop any users while doing RESTORE. We even don't want to revoke any grants -
just add grants which are missing. Therefore I think the drop() method should do
nothing for a grant object.
> +}
> +
> +///////////////////////////////////////////////////////////////////////////
> +
> Obj *get_database(const String *db_name)
> {
> return new DatabaseObj(db_name);
> @@ -2716,6 +2947,25 @@ Obj *materialize_tablespace(const String
> return obj;
> }
>
> +Obj *get_grant(const String *db_name,
> + const String *user_name,
> + const String *privilege)
> +{
> + return new DbGrantObj(db_name, user_name, privilege);
> +}
> +
> +Obj *materialize_db_grant(const String *db_name,
> + const String *user_name,
> + const String *privilege,
> + uint serialization_version,
> + const String *serialization)
I don't understand the privilege parameter to that function.
> +{
> + Obj *obj= get_grant(db_name, user_name, privilege);
> + obj->materialize(serialization_version, serialization);
> +
> + return obj;
> +}
> +
> ///////////////////////////////////////////////////////////////////////////
>
> bool is_internal_db_name(const String *db_name)
> @@ -2737,6 +2987,48 @@ bool is_internal_db_name(const String *d
> bool check_db_existence(const String *db_name)
> {
> return check_db_dir_existence(((String *) db_name)->c_ptr_safe());
> +}
> +
> +int split_user_host(String *user_host, String *user, String *host)
> +{
> + int len= 0;
> + int tics= 0;
> + char *ptr= strchr(user_host->c_ptr(), '@');
> +
> + if (ptr == 0)
> + return -1;
> + len= ptr - user_host->c_ptr();
> + user->length(0);
> + char *cptr= user_host->c_ptr();
> +
> + /*
> + String ' from strings.
> + */
> + if (strncmp(cptr, "'", 1) == 0)
> + {
> + cptr++;
> + len--;
> + }
> + if (strncmp(cptr+len-1, "'", 1) == 0)
> + tics++;
> + user->append(cptr, len - tics);
> + len= user_host->length() - len - 1 - tics;
> + host->length(0);
> +
> + /*
> + String ' from strings.
> + */
> + cptr= ptr + 1;
> + tics= 0;
> + if (strncmp(cptr, "'", 1) == 0)
> + {
> + cptr++;
> + len--;
> + }
> + if (strncmp(cptr+len-1, "'", 1) == 0)
> + tics++;
> + host->append(cptr, len - tics);
> + return 0;
> }
>
> /**
>
> === modified file 'sql/si_objects.h'
> --- a/sql/si_objects.h 2008-07-05 08:41:26 +0000
> +++ b/sql/si_objects.h 2008-07-07 22:14:30 +0000
> @@ -25,9 +25,9 @@ public:
> bool serialize(THD *thd, String *serialialization);
>
> /**
> - Return the name of the object.
> + Return the user name of the object.
>
> - @return object name.
> + @return object user name.
> */
This is misleading as the comment documents a general Obj::get_name() method. In
general this method returns the name of the object. Returing user name is an
exception for grant objects.
> virtual const String *get_name() = 0;
>
> @@ -115,6 +115,12 @@ private:
> friend Obj *materialize_tablespace(const String *,
> uint,
> const String *);
> +
> + friend Obj *materialize_db_grant(const String *,
> + const String *,
> + const String *,
> + uint,
> + const String *);
> };
>
>
> @@ -346,6 +352,22 @@ Obj *get_stored_function(const String *d
>
> Obj *get_event(const String *db_name, const String *event_name);
>
> +/**
> + Construct an instance of Obj representing a grant for a user in a db.
> +
> + No actual actions are performed in the server. An object can be created
> + even for invalid database/user name or for non-existing user.
> +
> + The client is responsible to destroy the created object.
> +
> + @param[in] db_name Database name.
> + @param[in] user_name User name.
> +
> + @return a pointer to an instance of Obj representing given db grant.
> +*/
> +
> +Obj *get_db_grant(const String *db_name, const String *user_name);
> +
I think this function is not needed and should be removed from the API.
> ///////////////////////////////////////////////////////////////////////////
>
> // The functions in this section provides a way to iterator over all
> @@ -429,6 +451,17 @@ ObjIterator *get_db_stored_functions(THD
>
> ObjIterator *get_db_events(THD *thd, const String *db_name);
>
> +/**
> + Create an iterator over all grants in the particular database.
> +
> + The client is responsible to destroy the returned iterator.
> +
> + @return a pointer to an iterator object.
> + @retval NULL in case of error.
> +*/
> +
> +ObjIterator *get_db_grants(THD *thd, const String *db_name);
> +
> ///////////////////////////////////////////////////////////////////////////
>
> // The functions are intended to enumerate dependent objects.
> @@ -506,6 +539,12 @@ Obj *materialize_tablespace(const String
> uint serialization_version,
> const String *serialialization);
>
> +Obj *materialize_db_grant(const String *db_name,
> + const String *user_name,
> + const String *privilege,
> + uint serialization_version,
> + const String *serialization);
> +
The function should be documented, especially the parameters. I'm curious how
would you document the 'privilege' parameter...
> ///////////////////////////////////////////////////////////////////////////
>
> bool is_internal_db_name(const String *db_name);
> @@ -520,6 +559,8 @@ bool is_internal_db_name(const String *d
> @retval TRUE on error (the database either not exists, or not accessible).
> */
> bool check_db_existence(const String *db_name);
> +
> +int split_user_host(String *user_host, String *user, String *host);
Instead of having this function, I'd rather add function user_exists() or
check_user_existence() in line with the function declared above. Such function
will take a complete "<user name>@<host pattern>" string as an argument and
split it internally if needed. It will also create an interface to server's
internal is_acl_user() function (the whole point of creating the server services
API).
>
> /*
> This method returns a @c TablespaceObj object if the table has a tablespace.
>
>
