Skip to content

Commit

Permalink
MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conve…
Browse files Browse the repository at this point in the history
…rsion

Item_func_group_concat::print() did not take into account
that Item_func_group_concat::separator can be of a different character set
than the "String *str" (when the printing is being done to).
Therefore, printing did not work correctly for:
- non-ASCII separators when GROUP_CONCAT is done on 8bit data
  or multi-byte data with mbminlen==1.
- all separators (even including simple ones like comma)
  when GROUP_CONCAT is done on ucs2/utf16/utf32 data (mbminlen>1).

Because of this problem, VIEW definitions did not print correctly to
their FRM files. This later led to a wrong SELECT and SHOW CREATE output.

Fix:

- Adding new String methods:

  bool append_for_single_quote_using_mb_wc(const char *str, size_t length,
                                           CHARSET_INFO *cs);

  bool append_for_single_quote_opt_convert(const char *str,
                                           size_t length,
                                           CHARSET_INFO *cs)

  which perform both escaping and character set conversion at the same time.

- Adding a new String method escaped_wc_for_single_quote(),
  to reuse the code between the old and the new methods.

- Fixing Item_func_group_concat::print() to use the new
  method append_for_single_quote_opt_convert().
  • Loading branch information
abarkov committed Mar 27, 2024
1 parent 58df209 commit 0fc123c
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 15 deletions.
20 changes: 20 additions & 0 deletions mysql-test/main/ctype_ucs.result
Original file line number Diff line number Diff line change
Expand Up @@ -6520,5 +6520,25 @@ SELECT 1 COLLATE latin1_swedish_ci;
ERROR 42000: COLLATION 'latin1_swedish_ci' is not valid for CHARACTER SET 'ucs2'
SET NAMES utf8;
#
# MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion
#
SET NAMES utf8mb3, @@collation_connection=ucs2_general_ci;
CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET ucs2;
INSERT INTO t1 VALUES ('a'),('A');
CREATE OR REPLACE VIEW v1 AS
SELECT COUNT(*) AS cnt, GROUP_CONCAT(c) AS c1 FROM t1 GROUP BY c;
SELECT * FROM v1;
cnt c1
2 a,A
SELECT HEX(c1) FROM v1;
HEX(c1)
0061002C0041
SHOW CREATE VIEW v1;
View Create View character_set_client collation_connection
v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select count(0) AS `cnt`,group_concat(`t1`.`c` separator ',') AS `c1` from `t1` group by `t1`.`c` utf8 ucs2_general_ci
DROP VIEW v1;
DROP TABLE t1;
SET NAMES utf8mb3;
#
# End of 10.5 tests
#
17 changes: 17 additions & 0 deletions mysql-test/main/ctype_ucs.test
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,23 @@ SELECT HEX(1 COLLATE ucs2_bin);
SELECT 1 COLLATE latin1_swedish_ci;
SET NAMES utf8;

--echo #
--echo # MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion
--echo #

SET NAMES utf8mb3, @@collation_connection=ucs2_general_ci;
CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET ucs2;
INSERT INTO t1 VALUES ('a'),('A');
CREATE OR REPLACE VIEW v1 AS
SELECT COUNT(*) AS cnt, GROUP_CONCAT(c) AS c1 FROM t1 GROUP BY c;
SELECT * FROM v1;
SELECT HEX(c1) FROM v1;
SHOW CREATE VIEW v1;
DROP VIEW v1;
DROP TABLE t1;
SET NAMES utf8mb3;


--echo #
--echo # End of 10.5 tests
--echo #
20 changes: 20 additions & 0 deletions mysql-test/main/func_gconcat.result
Original file line number Diff line number Diff line change
Expand Up @@ -1517,4 +1517,24 @@ deallocate prepare stmt;
set join_cache_level=default;
set group_concat_max_len=default;
drop table t1,t2;
#
# MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion
#
SET NAMES utf8, @@collation_connection=latin1_swedish_ci;
CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET latin1;
INSERT INTO t1 VALUES ('a'),('A');
CREATE OR REPLACE VIEW v1 AS
SELECT GROUP_CONCAT(c SEPARATOR 'ß') AS c1 FROM t1 GROUP BY c;
SELECT * FROM v1;
c1
aßA
SELECT HEX(c1) FROM v1;
HEX(c1)
61DF41
SHOW CREATE VIEW v1;
View Create View character_set_client collation_connection
v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select group_concat(`t1`.`c` separator 'ß') AS `c1` from `t1` group by `t1`.`c` utf8 latin1_swedish_ci
DROP VIEW v1;
DROP TABLE t1;
SET NAMES latin1;
# End of 10.5 tests
16 changes: 16 additions & 0 deletions mysql-test/main/func_gconcat.test
Original file line number Diff line number Diff line change
Expand Up @@ -1105,4 +1105,20 @@ set group_concat_max_len=default;

drop table t1,t2;

--echo #
--echo # MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion
--echo #

SET NAMES utf8, @@collation_connection=latin1_swedish_ci;
CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET latin1;
INSERT INTO t1 VALUES ('a'),('A');
CREATE OR REPLACE VIEW v1 AS
SELECT GROUP_CONCAT(c SEPARATOR 'ß') AS c1 FROM t1 GROUP BY c;
SELECT * FROM v1;
SELECT HEX(c1) FROM v1;
SHOW CREATE VIEW v1;
DROP VIEW v1;
DROP TABLE t1;
SET NAMES latin1;

--echo # End of 10.5 tests
2 changes: 1 addition & 1 deletion sql/item_sum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4587,7 +4587,7 @@ void Item_func_group_concat::print(String *str, enum_query_type query_type)
if (sum_func() == GROUP_CONCAT_FUNC)
{
str->append(STRING_WITH_LEN(" separator \'"));
str->append_for_single_quote(separator->ptr(), separator->length());
str->append_for_single_quote_opt_convert(*separator);
str->append(STRING_WITH_LEN("\'"));
}

Expand Down
47 changes: 33 additions & 14 deletions sql/sql_string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1126,26 +1126,45 @@ bool String::append_for_single_quote(const char *st, size_t len)
int chlen;
for (; st < end; st++)
{
switch (*st)
char ch2= (char) (uchar) escaped_wc_for_single_quote((uchar) *st);
if (ch2)
{
case '\\': APPEND(STRING_WITH_LEN("\\\\")); break;
case '\0': APPEND(STRING_WITH_LEN("\\0")); break;
case '\'': APPEND(STRING_WITH_LEN("\\'")); break;
case '\n': APPEND(STRING_WITH_LEN("\\n")); break;
case '\r': APPEND(STRING_WITH_LEN("\\r")); break;
case '\032': APPEND(STRING_WITH_LEN("\\Z")); break;
default: if ((chlen=charset()->charlen(st, end)) > 0)
{
APPEND(st, chlen);
st+= chlen-1;
}
else
APPEND(*st);
if (append('\\') || append(ch2))
return true;
continue;
}
if ((chlen= charset()->charlen(st, end)) > 0)
{
APPEND(st, chlen);
st+= chlen-1;
}
else
APPEND(*st);
}
return 0;
}


bool String::append_for_single_quote_using_mb_wc(const char *src,
size_t length,
CHARSET_INFO *cs)
{
DBUG_ASSERT(&my_charset_bin != charset());
DBUG_ASSERT(&my_charset_bin != cs);
const uchar *str= (const uchar *) src;
const uchar *end= (const uchar *) src + length;
int chlen;
my_wc_t wc;
for ( ; (chlen= cs->cset->mb_wc(cs, &wc, str, end)) > 0; str+= chlen)
{
my_wc_t wc2= escaped_wc_for_single_quote(wc);
if (wc2 ? (append_wc('\\') || append_wc(wc2)) : append_wc(wc))
return true;
}
return false;
}


void String::print(String *str) const
{
str->append_for_single_quote(Ptr, str_length);
Expand Down
36 changes: 36 additions & 0 deletions sql/sql_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,42 @@ class String: public Charset, public Binary_string
print_with_conversion(to, cs);
}

static my_wc_t escaped_wc_for_single_quote(my_wc_t ch)
{
switch (ch)
{
case '\\': return '\\';
case '\0': return '0';
case '\'': return '\'';
case '\n': return 'n';
case '\r': return 'r';
case '\032': return 'Z';
}
return 0;
}

// Append for single quote using mb_wc/wc_mb Unicode conversion
bool append_for_single_quote_using_mb_wc(const char *str, size_t length,
CHARSET_INFO *cs);

// Append for single quote with optional mb_wc/wc_mb conversion
bool append_for_single_quote_opt_convert(const char *str,
size_t length,
CHARSET_INFO *cs)
{
return charset() == &my_charset_bin || cs == &my_charset_bin ||
my_charset_same(charset(), cs) ?
append_for_single_quote(str, length) :
append_for_single_quote_using_mb_wc(str, length, cs);
}

bool append_for_single_quote_opt_convert(const String &str)
{
return append_for_single_quote_opt_convert(str.ptr(),
str.length(),
str.charset());
}

bool append_for_single_quote(const char *st, size_t len);
bool append_for_single_quote(const String *s)
{
Expand Down

0 comments on commit 0fc123c

Please sign in to comment.