Hi Vicentiu, It seems that Item_func_sysconst abuses const_charset_converter(). I have the following proposals: - Instead of adding a new code into Item::const_charset_converter(), can you please override Item_func_sysconst::safe_charset_converter()? The direction proposed by Sergei's looks good: Item *safe_charset_converter(THD *thd, CHARSET_INFO *tocs) { + if (thd->lex->context_analysis_only & CONTEXT_ANALYSIS_ONLY_VIEW) + return Item_str_func::safe_charset_converter(thd, tocs); return const_charset_converter(thd, tocs, true, fully_qualified_func_name() } But it probably needs to be extended to handle prepared statement. At PREPARE-time it should not call const_charset_converter(). At EXECUTE-time it should be Ok to call const_charset_converter(). So can you please change Item_func_sysconst::const_item() to report false when in views and during PREPARE-time? And then change safe_charset_converter() as follows: Item *safe_charset_converter(THD *thd, CHARSET_INFO *tocs) { if (!const_item()) return Item_str_func::safe_charset_converter(thd, tocs); return const_charset_converter(thd, tocs, true, fully_qualified_func_name() } - Please add tests for DATABASE() and CURRENT_USER - Please add tests for PS with changing context between PREPARE and EXECUTE. Thanks. On 05/22/2017 06:15 PM, vicentiu wrote:
revision-id: 55a4858fe6f3fdc63a072b93b9bc3d8e51dc5e94 (mariadb-10.0.30-76-g55a4858fe6f) parent(s): 8d1827f5540a0def82b1e2cbc557245c3e5e14ca author: Vicențiu Ciorbaru committer: Vicențiu Ciorbaru timestamp: 2017-05-22 17:06:01 +0300 message:
MDEV-12666: CURRENT_ROLE() does not work in a view
The problem lies in how CURRENT_ROLE is defined. The Item_func_current_role inherits from Item_func_sysconst, which defines a safe_charset_converter to be a const_charset_converter.
During view creation, if there is no role previously set, the current_role() function returns NULL.
This is captured on item instantiation and the const_charset_converter call subsequently returns an Item_null. In turn, the function is replaced with Item_null and the view is then created with an Item_null instead of Item_func_current_role.
Without this patch, the first SHOW CREATE VIEW from the testcase would have a where clause of WHERE role_name = NULL, while the second SHOW CREATE VIEW would show a correctly created view.
The solution proposed is to not replace any function with an Item_null, but instead use an Item_static_string_func.
--- .../suite/roles/current_role_view-12666.result | 48 ++++++++++++++++++++ .../suite/roles/current_role_view-12666.test | 52 ++++++++++++++++++++++ sql/item.cc | 13 +++++- 3 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/mysql-test/suite/roles/current_role_view-12666.result b/mysql-test/suite/roles/current_role_view-12666.result new file mode 100644 index 00000000000..525748b4875 --- /dev/null +++ b/mysql-test/suite/roles/current_role_view-12666.result @@ -0,0 +1,48 @@ +CREATE USER has_role@'localhost'; +GRANT ALL PRIVILEGES ON *.* TO has_role@'localhost'; +CREATE ROLE test_role; +GRANT test_role TO has_role@'localhost'; +CREATE USER no_role@'localhost'; +GRANT ALL PRIVILEGES ON *.* TO no_role@'localhost'; +CREATE TABLE view_role_test ( +id int primary key, +role_name varchar(50) +); +INSERT INTO view_role_test VALUES (1, 'test_role'); +CREATE OR REPLACE +DEFINER = no_role@localhost +SQL SECURITY INVOKER +VIEW v_view_role_test +AS +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); +show create view v_view_role_test; +View Create View character_set_client collation_connection +v_view_role_test CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test` AS select `view_role_test`.`id` AS `id`,`view_role_test`.`role_name` AS `role_name` from `view_role_test` where (`view_role_test`.`role_name` = current_role()) latin1 latin1_swedish_ci +SET ROLE test_role; +CREATE OR REPLACE +DEFINER = no_role@localhost +SQL SECURITY INVOKER +VIEW v_view_role_test_2 +AS +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); +show create view v_view_role_test_2; +View Create View character_set_client collation_connection +v_view_role_test_2 CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test_2` AS select `view_role_test`.`id` AS `id`,`view_role_test`.`role_name` AS `role_name` from `view_role_test` where (`view_role_test`.`role_name` = current_role()) latin1 latin1_swedish_ci +SELECT CURRENT_USER(); +CURRENT_USER() +root@localhost +SELECT CURRENT_ROLE(); +CURRENT_ROLE() +test_role +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); +id role_name +1 test_role +SELECT * FROM v_view_role_test; +id role_name +1 test_role +drop user has_role@'localhost'; +drop user no_role@'localhost'; +drop role test_role; +drop table view_role_test; +drop view v_view_role_test; +drop view v_view_role_test_2; diff --git a/mysql-test/suite/roles/current_role_view-12666.test b/mysql-test/suite/roles/current_role_view-12666.test new file mode 100644 index 00000000000..86c74260b6e --- /dev/null +++ b/mysql-test/suite/roles/current_role_view-12666.test @@ -0,0 +1,52 @@ +CREATE USER has_role@'localhost'; +GRANT ALL PRIVILEGES ON *.* TO has_role@'localhost'; + +CREATE ROLE test_role; +GRANT test_role TO has_role@'localhost'; + +CREATE USER no_role@'localhost'; +GRANT ALL PRIVILEGES ON *.* TO no_role@'localhost'; + +CREATE TABLE view_role_test ( + id int primary key, + role_name varchar(50) + ); + +INSERT INTO view_role_test VALUES (1, 'test_role'); + + +CREATE OR REPLACE +DEFINER = no_role@localhost +SQL SECURITY INVOKER +VIEW v_view_role_test +AS +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); + +show create view v_view_role_test; + +SET ROLE test_role; + +CREATE OR REPLACE +DEFINER = no_role@localhost +SQL SECURITY INVOKER +VIEW v_view_role_test_2 +AS +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); + +show create view v_view_role_test_2; + + +SELECT CURRENT_USER(); +SELECT CURRENT_ROLE(); + +SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE(); +SELECT * FROM v_view_role_test; + + +drop user has_role@'localhost'; +drop user no_role@'localhost'; +drop role test_role; + +drop table view_role_test; +drop view v_view_role_test; +drop view v_view_role_test_2; diff --git a/sql/item.cc b/sql/item.cc index 4ce8396f71e..2ee3a6d61dc 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1263,9 +1263,20 @@ Item *Item::const_charset_converter(CHARSET_INFO *tocs, DBUG_ASSERT(fixed); StringBuffer<64>tmp; String *s= val_str(&tmp); - if (!s) + + /* Functions that return NULL during prepare must not be replaced with + Item_null as they may return non-null values during execution. + Ex: CURRENT_ROLE() during view creation time can be NULL if no role + is set. */ + if (!s && !func_name) return new Item_null((char *) func_name, tocs);
+ if (!s) + { + tmp.set_ascii("", 0); + s = &tmp; + } + if (!needs_charset_converter(s->length(), tocs)) { if (collation.collation == &my_charset_bin && tocs != &my_charset_bin && _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits