[Maria-developers] Please review a patch removing LEX::text_string_is_7bit
Hello Sergei, Can you please review a patch for bb-10.2-ext fixing the problem that in case of possible changes in look-ahead in rules using TEXT_STRING or NCHAR_STRING, Lex->text_string_is_7bit can get out of sync with $1/$2/$3, etc. Thanks!
Hi, Alexander! On Mar 30, Alexander Barkov wrote:
Hello Sergei,
Can you please review a patch for bb-10.2-ext fixing the problem that in case of possible changes in look-ahead in rules using TEXT_STRING or NCHAR_STRING, Lex->text_string_is_7bit can get out of sync with $1/$2/$3, etc.
No comments on the approach, looks ok. Few details: * Lex_string_with_metadata_st is very vague. Lex_string_with_repertoire would be better (e.g. there's CSET_STRING which is LEX_STRING + CHARSET_INFO, it's also "LEX_STRING with metadata" in a sense). * It's only used in the lexer/parser, so putting it in sql_lex.h would be more appropriate. The rest of the server will hardly need to use Lex_string_with_repertoire, it's too lexer/parser specific. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hello Sergei, Thanks for review! On 04/04/2017 03:20 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Mar 30, Alexander Barkov wrote:
Hello Sergei,
Can you please review a patch for bb-10.2-ext fixing the problem that in case of possible changes in look-ahead in rules using TEXT_STRING or NCHAR_STRING, Lex->text_string_is_7bit can get out of sync with $1/$2/$3, etc.
No comments on the approach, looks ok.
Few details:
* Lex_string_with_metadata_st is very vague. Lex_string_with_repertoire would be better (e.g. there's CSET_STRING which is LEX_STRING + CHARSET_INFO, it's also "LEX_STRING with metadata" in a sense).
We'll add more flags there soon. Something like this: bool m_is_8bit:1; bool m_has_mb:1; bool m_has_bad_byte:1; bool m_backslash_escape:1; bool m_sep_escape:1; bool m_is_binary:1; and reuse the original query fragments instead of making the string copy to early, in Lex_input_stream::get_text(). This will allow to avoid unnecessary copying, as well as create more optimal Item types in sql_yacc.yy So as agreed on IRC, I'll keep the same.
* It's only used in the lexer/parser, so putting it in sql_lex.h would be more appropriate. The rest of the server will hardly need to use Lex_string_with_repertoire, it's too lexer/parser specific.
Will move to sql_lex.h. Thanks for a good suggestion!
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Alexander Barkov -
Sergei Golubchik