Hello Sergei, Please find a new patch here: https://github.com/MariaDB/server/commit/be66e975ecf15ae696d2e645da4aaf1a06c... Thanks. Comments follow below: On 1/9/24 16:32, Sergei Golubchik wrote:
Hi, Alexander,
On Jan 09, Alexander Barkov wrote:
Hello Sergei,
This is a new version:
I've looked at the diff of this and previous commit excluding InnoDB.
Only one comment, about Lex_ident_routine::check_routine_name_with_error() and Lex_ident_fs::check_table_name().
Why do they have different semantics? The former is a static method and takes the name as an argument. The latter checks `this`, needs a Lex_ident_fs to be constructed first.
It's confusing, can you modify one of these methods to work like the other? I believe the generated code is the same either way, so it's only a question of consistency and predictability.
Also, I'd suggest you rename them to "check_name", I feel that Lex_ident_routine::check_routine_name_with_error() is a bit redundant. Same for check_table_name().
I renamed and moved these methods to static methods as follows: class Lex_ident_db: public Lex_ident_fs { ... static bool check_name(const LEX_CSTRING &str); static bool check_name_with_error(const LEX_CSTRING &str); ... }; class Lex_ident_table: public Lex_ident_fs { ... static bool check_name(const LEX_CSTRING &str, bool check_for_path_chars); ... };
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org