Skip to content
  • Erik Froseth's avatar
    f62be3eb
    Bug#28333657 MAKE CREATE_FIELD::LENGTH PRIVATE · f62be3eb
    Erik Froseth authored
    Create_field::length has for a very long time had the comment
    "At various stages in execution this can be length of field in
    bytes or max number of characters.". It has been difficult to know
    whether this variable has been in number of bytes, number of
    characters, number of code points etc. when using it, and this has
    also been a source of several bugs. Furthermore, there are comments
    like "Divide by four here, so we can multiply by four later" in the
    code which makes it even more confusing. This patch tries to
    resolve some of this mess by:
    
    1) Renaming Create_field::length to Create_field::m_max_display_width_in_codepoints.
       We have chosen code points instead of bytes since that is what the
       user enters when writing DDL statements (VARCHAR(20) etc...).
    
    2) Making the variable private, so it can't be modified from the
       outside. Two getters are added; max_display_width_in_bytes(),
       max_display_width_in_codepoints(). This removes all this "divide
       by four so we can multiply by four later".
    
    3) Remove create_length_to_internal_length() which is used throuhout
       the code to convert Create_field::length from number of code points
       to number of bytes.
    
    4) Move Create_field out to a separate file "sql/create_field.{h|cc}"
    
    As a side-effect of the above, Create_field::pack_length and
    Create_field::key_length is removed and replaced by member functions
    pack_length() and key_length() since they both are calculated from
    max_display_widht_in_bytes().
    
    By making it much more explicit what we get from Create_field, we notice
    some interesting things such as "set_char_length" expects the number
    of bytes etc. This patch does not try to figure out whether or not this
    is correct, but merely makes it a lot more explicit what's going on.
    
    Change-Id: I5ad725649e86b38ff32b5c4f07c9be42c7995c64
    f62be3eb
    Bug#28333657 MAKE CREATE_FIELD::LENGTH PRIVATE
    Erik Froseth authored
    Create_field::length has for a very long time had the comment
    "At various stages in execution this can be length of field in
    bytes or max number of characters.". It has been difficult to know
    whether this variable has been in number of bytes, number of
    characters, number of code points etc. when using it, and this has
    also been a source of several bugs. Furthermore, there are comments
    like "Divide by four here, so we can multiply by four later" in the
    code which makes it even more confusing. This patch tries to
    resolve some of this mess by:
    
    1) Renaming Create_field::length to Create_field::m_max_display_width_in_codepoints.
       We have chosen code points instead of bytes since that is what the
       user enters when writing DDL statements (VARCHAR(20) etc...).
    
    2) Making the variable private, so it can't be modified from the
       outside. Two getters are added; max_display_width_in_bytes(),
       max_display_width_in_codepoints(). This removes all this "divide
       by four so we can multiply by four later".
    
    3) Remove create_length_to_internal_length() which is used throuhout
       the code to convert Create_field::length from number of code points
       to number of bytes.
    
    4) Move Create_field out to a separate file "sql/create_field.{h|cc}"
    
    As a side-effect of the above, Create_field::pack_length and
    Create_field::key_length is removed and replaced by member functions
    pack_length() and key_length() since they both are calculated from
    max_display_widht_in_bytes().
    
    By making it much more explicit what we get from Create_field, we notice
    some interesting things such as "set_char_length" expects the number
    of bytes etc. This patch does not try to figure out whether or not this
    is correct, but merely makes it a lot more explicit what's going on.
    
    Change-Id: I5ad725649e86b38ff32b5c4f07c9be42c7995c64
Loading