| Bug #50195 | SQLTablePrivileges requires SELECT privilege on mysql database | ||
|---|---|---|---|
| Submitted: | 8 Jan 2010 20:43 | Modified: | 23 Jul 2010 14:39 |
| Reporter: | Farid Zidan (Candidate Quality Contributor) | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | Connector / ODBC | Severity: | S3 (Non-critical) |
| Version: | 5.0.67 | OS: | Any |
| Assigned to: | Lawrenty Novitsky | CPU Architecture: | Any |
[8 Jan 2010 20:43]
Farid Zidan
[14 Jan 2010 17:31]
Tonci Grgin
Thanks for spotting this Farid.
Verified as described by looking into 5.1 sources, catalog.c, ln1037:
pos= strxmov(buff,
"SELECT Db,User,Table_name,Grantor,Table_priv ",
"FROM mysql.tables_priv WHERE Table_name LIKE '",
NullS);
[8 Feb 2010 1:19]
Lawrenty Novitsky
patch changing default SQLTablePrivileges algorithm to use I_S
Attachment: bug50195_1.diff (text/x-diff), 18.40 KiB.
[8 Feb 2010 17:42]
Lawrenty Novitsky
One more note. Currently I return schema and catalog from i_s as schema and catalog respectively. But since we have that historical twist in parameters treatment, maybe we should have the same twist in resultset? return catalog as schema, and vice versa. it doesn't make much sense to return rows for db passed with catalog parameter, and have NULL for catalog in the resultset. But it doesn't make much sense to switch them over either. heh, that is all crazy and is killing me. I guess adding connection option for normal or "historcal" parameters interpretation is getting higher priority on my list.
[10 Feb 2010 0:28]
Jim Winstead
I find it helps to not think of the server as implementing catalogs and schemas (since it doesn't, it only implements one thing), but just 'databases' that it happens to call catalogs in some places, but which have historically been called 'schemas' by the ODBC driver (and C/J, and maybe C/NET). Rather than #including the new "catalog_oldsvr.c" within catalog.c, it should be compiled on its own. (And splitting all the pre-I_S implementations of the catalog functions to this is a great idea for a follow-on task.) If add_table_name() is to be truly SQL_ATTR_METADATA_ID aware, it is not enough to just use 'BINARY', you also need to escape wildcard characters (% and _), or the queries they are used in need to use = instead of LIKE. The MSDN docs have a good section describing how SQL_ATTR_METADATA_ID impacts arguments to all of the catalog functions: http://msdn.microsoft.com/en-us/library/ms716447(VS.85).aspx There's no test for the non-default SQL_ATTR_METADATA_ID value.
[13 Feb 2010 23:32]
Lawrenty Novitsky
a bit reworked 1st patch + SQLCatalogPrivileges changed in same manner to use I_S
Attachment: bug50195_2.diff (text/x-diff), 30.20 KiB.
[13 Feb 2010 23:50]
Lawrenty Novitsky
Here is comments to patch, but they are more like a reply on jim's remarks. It still doesn't help if we call it catalog on input of the function and schema on its output. on separate compilation - at the moment it's easier and make sense to include the file. catalog.c contains helper functions that (could be)used by both files. Since not all "old" functions have moved to the designated file yet. I watched the code and looks like it's impossible to set not default SQL_ATTR_METADATA_ID at the moment. That's why there is no test of it. But I've improved handling of it. I think the missing thing is removing of trailing blanks(if i interpreted identifier arguments description correctly).
[28 May 2010 9:00]
Lawrenty Novitsky
Merged and pushed this patch to the working_tree branch as rev#897 and then some additions in rev#898 (made affected functions care about no_i_s option introduced later, and moved more of routines to oldsvr file)
Besides diff -r896..898 contains following (pretty important)piece not related to the bug
=== modified file 'driver/execute.c'
--- driver/execute.c 2010-04-30 13:20:44 +0000
+++ driver/execute.c 2010-05-27 07:57:16 +0000
@@ -53,7 +53,7 @@
return error;
}
- if (query_length == 0)
+ if (query_length == 0)
query_length= strlen(query);
MYLOG_QUERY(stmt, query);
@@ -607,7 +607,7 @@
if(is_no_backslashes_escape_mode(stmt->dbc) &&
is_binary_sql_type(iprec->concise_type))
{
- SQLINTEGER transformed_len = 0;
+ SQLLEN transformed_len = 0;
to= add_to_buffer(net,to," 0x",3);
/* Make sure we have room for a fully-escaped string. */
if (!(to= extend_buffer(net, to, length * 2)))
Caused stack corruption on the 64bit. And it also contains some fixes of compilation warnings on win64.
[3 Jun 2010 19:57]
Jim Winstead
okay, the way the new functions report catalog/schema is different from the existing catalog functions but almost there. to fix, change the queries in i_s_list_table_priv() and i_s_list_column_priv() to "SELECT TABLE_SCHEMA as TABLE_CAT, NULL as TABLE_SCHEM". the way the WHERE clauses are built is correct, mapping the catalog argument to matches against TABLE_SCHEMA. with that change, this is okay. it might be good to add checking of the other fields in the test case so that the i_s and non-i_s versions can be compared to make sure they return consistent data.
[8 Jul 2010 21:17]
Lawrenty Novitsky
patch pushed as rev#900. if nothing changes goes to 5.1.7
[23 Jul 2010 14:39]
Tony Bedford
An entry has been added to the 5.1.7 changelog: In contrast to all other ODBC catalog functions SQLTablePrivileges required the user to have SELECT privilege on MySQL schemata, otherwise the function returned with an error: SQL Error. Native Code: 1142, SQLState: HY000, Return Code: -1 [MySQL][ODBC 5.1 Driver][mysqld-5.0.67-community-nt]SELECT command denied to user 'repadmin'@'localhost' for table 'tables_priv' [Error][SQL Error]Error executing SQLTablePrivileges for object cat: myrep, object Name: xxxxxxxxxx
