Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
M
MariaDB
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
CI / CD
CI / CD
Pipelines
Jobs
Schedules
Analytics
Analytics
CI / CD
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Jobs
Commits
Issue Boards
Open sidebar
nexedi
MariaDB
Commits
b4a5ad8d
Commit
b4a5ad8d
authored
Nov 11, 2020
by
Marko Mäkelä
Browse files
Options
Browse Files
Download
Plain Diff
Merge mariadb-10.5.8 into 10.5
parents
fff469db
7da6353b
Changes
7
Hide whitespace changes
Inline
Side-by-side
Showing
7 changed files
with
163 additions
and
36 deletions
+163
-36
mysql-test/main/range.result
mysql-test/main/range.result
+34
-1
mysql-test/main/range.test
mysql-test/main/range.test
+46
-0
mysql-test/main/range_mrr_icp.result
mysql-test/main/range_mrr_icp.result
+34
-1
sql/opt_range.cc
sql/opt_range.cc
+7
-7
sql/sql_prepare.cc
sql/sql_prepare.cc
+17
-4
storage/innobase/trx/trx0rec.cc
storage/innobase/trx/trx0rec.cc
+20
-21
tests/mysql_client_test.c
tests/mysql_client_test.c
+5
-2
No files found.
mysql-test/main/range.result
View file @
b4a5ad8d
...
@@ -1297,7 +1297,7 @@ SELECT * FROM t1 WHERE
...
@@ -1297,7 +1297,7 @@ SELECT * FROM t1 WHERE
25 <= a AND b = 23 OR
25 <= a AND b = 23 OR
23 <= a;
23 <= a;
id select_type table type possible_keys key key_len ref rows Extra
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range a a 5 NULL
2
Using where; Using index
1 SIMPLE t1 range a a 5 NULL
3
Using where; Using index
SELECT * FROM t1 WHERE
SELECT * FROM t1 WHERE
23 <= a AND a <= 25 OR
23 <= a AND a <= 25 OR
25 <= a AND b = 23 OR
25 <= a AND b = 23 OR
...
@@ -3121,6 +3121,39 @@ a b
...
@@ -3121,6 +3121,39 @@ a b
set eq_range_index_dive_limit=default;
set eq_range_index_dive_limit=default;
drop table t1;
drop table t1;
#
#
# MDEV-24117: Memory management problem in statistics state...
# (just the testcase)
#
create table t0(a int);
insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
create table t1(a int);
insert into t1
select A.a + B.a* 10 + C.a * 100 + D.a * 1000
from t0 A, t0 B, t0 C, t0 D
where D.a<4;
create table t2 (
a int,
b int,
key(a)
);
insert into t2 values (1,1),(2,2),(3,3);
set @query=(select group_concat(a) from t1);
set @tmp_24117= @@max_session_mem_used;
#
# On debug build, the usage was
# - 2.8M without the bug
# - 1G with the bug.
set max_session_mem_used=64*1024*1024;
set @query=concat('explain select * from t2 where a=1 or a in (', @query, ')');
prepare s from @query;
# This should not fail with an error:
execute s;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t2 ALL a NULL NULL NULL 3 Using where
set max_session_mem_used=@tmp_24117;
deallocate prepare s;
drop table t0,t1,t2;
#
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
# for the same index such that the second range condition
# for the same index such that the second range condition
# fully covers the first one. Additionally one of the disjuncts
# fully covers the first one. Additionally one of the disjuncts
...
...
mysql-test/main/range.test
View file @
b4a5ad8d
...
@@ -2092,6 +2092,52 @@ set eq_range_index_dive_limit=default;
...
@@ -2092,6 +2092,52 @@ set eq_range_index_dive_limit=default;
drop
table
t1
;
drop
table
t1
;
--
echo
#
--
echo
# MDEV-24117: Memory management problem in statistics state...
--
echo
# (just the testcase)
--
echo
#
create
table
t0
(
a
int
);
insert
into
t0
values
(
0
),(
1
),(
2
),(
3
),(
4
),(
5
),(
6
),(
7
),(
8
),(
9
);
create
table
t1
(
a
int
);
# 4K rows
insert
into
t1
select
A
.
a
+
B
.
a
*
10
+
C
.
a
*
100
+
D
.
a
*
1000
from
t0
A
,
t0
B
,
t0
C
,
t0
D
where
D
.
a
<
4
;
create
table
t2
(
a
int
,
b
int
,
key
(
a
)
);
insert
into
t2
values
(
1
,
1
),(
2
,
2
),(
3
,
3
);
set
@
query
=
(
select
group_concat
(
a
)
from
t1
);
set
@
tmp_24117
=
@@
max_session_mem_used
;
--
echo
#
--
echo
# On debug build, the usage was
--
echo
# - 2.8M without the bug
--
echo
# - 1G with the bug.
set
max_session_mem_used
=
64
*
1024
*
1024
;
set
@
query
=
concat
(
'explain select * from t2 where a=1 or a in ('
,
@
query
,
')'
);
prepare
s
from
@
query
;
--
echo
# This should not fail with an error:
execute
s
;
set
max_session_mem_used
=@
tmp_24117
;
deallocate
prepare
s
;
drop
table
t0
,
t1
,
t2
;
--
echo
#
--
echo
#
--
echo
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
--
echo
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
--
echo
# for the same index such that the second range condition
--
echo
# for the same index such that the second range condition
...
...
mysql-test/main/range_mrr_icp.result
View file @
b4a5ad8d
...
@@ -1300,7 +1300,7 @@ SELECT * FROM t1 WHERE
...
@@ -1300,7 +1300,7 @@ SELECT * FROM t1 WHERE
25 <= a AND b = 23 OR
25 <= a AND b = 23 OR
23 <= a;
23 <= a;
id select_type table type possible_keys key key_len ref rows Extra
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range a a 5 NULL
2
Using where; Using index
1 SIMPLE t1 range a a 5 NULL
3
Using where; Using index
SELECT * FROM t1 WHERE
SELECT * FROM t1 WHERE
23 <= a AND a <= 25 OR
23 <= a AND a <= 25 OR
25 <= a AND b = 23 OR
25 <= a AND b = 23 OR
...
@@ -3110,6 +3110,39 @@ a b
...
@@ -3110,6 +3110,39 @@ a b
set eq_range_index_dive_limit=default;
set eq_range_index_dive_limit=default;
drop table t1;
drop table t1;
#
#
# MDEV-24117: Memory management problem in statistics state...
# (just the testcase)
#
create table t0(a int);
insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
create table t1(a int);
insert into t1
select A.a + B.a* 10 + C.a * 100 + D.a * 1000
from t0 A, t0 B, t0 C, t0 D
where D.a<4;
create table t2 (
a int,
b int,
key(a)
);
insert into t2 values (1,1),(2,2),(3,3);
set @query=(select group_concat(a) from t1);
set @tmp_24117= @@max_session_mem_used;
#
# On debug build, the usage was
# - 2.8M without the bug
# - 1G with the bug.
set max_session_mem_used=64*1024*1024;
set @query=concat('explain select * from t2 where a=1 or a in (', @query, ')');
prepare s from @query;
# This should not fail with an error:
execute s;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t2 ALL a NULL NULL NULL 3 Using where
set max_session_mem_used=@tmp_24117;
deallocate prepare s;
drop table t0,t1,t2;
#
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
# MDEV-23811: Both disjunct of WHERE condition contain range conditions
# for the same index such that the second range condition
# for the same index such that the second range condition
# fully covers the first one. Additionally one of the disjuncts
# fully covers the first one. Additionally one of the disjuncts
...
...
sql/opt_range.cc
View file @
b4a5ad8d
...
@@ -9619,15 +9619,9 @@ tree_or(RANGE_OPT_PARAM *param,SEL_TREE *tree1,SEL_TREE *tree2)
...
@@ -9619,15 +9619,9 @@ tree_or(RANGE_OPT_PARAM *param,SEL_TREE *tree1,SEL_TREE *tree2)
}
}
bool
no_imerge_from_ranges
=
FALSE
;
bool
no_imerge_from_ranges
=
FALSE
;
SEL_TREE
*
rt1
=
tree1
;
SEL_TREE
*
rt2
=
tree2
;
/* Build the range part of the tree for the formula (1) */
/* Build the range part of the tree for the formula (1) */
if
(
sel_trees_can_be_ored
(
param
,
tree1
,
tree2
,
&
ored_keys
))
if
(
sel_trees_can_be_ored
(
param
,
tree1
,
tree2
,
&
ored_keys
))
{
{
if
(
no_merges1
)
rt1
=
new
SEL_TREE
(
tree1
,
TRUE
,
param
);
if
(
no_merges2
)
rt2
=
new
SEL_TREE
(
tree2
,
TRUE
,
param
);
bool
must_be_ored
=
sel_trees_must_be_ored
(
param
,
tree1
,
tree2
,
ored_keys
);
bool
must_be_ored
=
sel_trees_must_be_ored
(
param
,
tree1
,
tree2
,
ored_keys
);
no_imerge_from_ranges
=
must_be_ored
;
no_imerge_from_ranges
=
must_be_ored
;
...
@@ -9685,6 +9679,12 @@ tree_or(RANGE_OPT_PARAM *param,SEL_TREE *tree1,SEL_TREE *tree2)
...
@@ -9685,6 +9679,12 @@ tree_or(RANGE_OPT_PARAM *param,SEL_TREE *tree1,SEL_TREE *tree2)
else
if
(
!
no_ranges1
&&
!
no_ranges2
&&
!
no_imerge_from_ranges
)
else
if
(
!
no_ranges1
&&
!
no_ranges2
&&
!
no_imerge_from_ranges
)
{
{
/* Build the imerge part of the tree for the formula (1) */
/* Build the imerge part of the tree for the formula (1) */
SEL_TREE
*
rt1
=
tree1
;
SEL_TREE
*
rt2
=
tree2
;
if
(
no_merges1
)
rt1
=
new
SEL_TREE
(
tree1
,
TRUE
,
param
);
if
(
no_merges2
)
rt2
=
new
SEL_TREE
(
tree2
,
TRUE
,
param
);
if
(
!
rt1
||
!
rt2
||
if
(
!
rt1
||
!
rt2
||
result
->
merges
.
push_back
(
imerge_from_ranges
)
||
result
->
merges
.
push_back
(
imerge_from_ranges
)
||
imerge_from_ranges
->
or_sel_tree
(
param
,
rt1
)
||
imerge_from_ranges
->
or_sel_tree
(
param
,
rt1
)
||
...
@@ -10350,7 +10350,7 @@ key_or(RANGE_OPT_PARAM *param, SEL_ARG *key1,SEL_ARG *key2)
...
@@ -10350,7 +10350,7 @@ key_or(RANGE_OPT_PARAM *param, SEL_ARG *key1,SEL_ARG *key2)
if
(
!
tmp
->
next_key_part
)
if
(
!
tmp
->
next_key_part
)
{
{
SEL_ARG
*
key2_next
=
key2
->
next
;
SEL_ARG
*
key2_next
=
key2
->
next
;
if
(
key2
->
use_count
)
if
(
key2_shared
)
{
{
SEL_ARG
*
key2_cpy
=
new
SEL_ARG
(
*
key2
);
SEL_ARG
*
key2_cpy
=
new
SEL_ARG
(
*
key2
);
if
(
!
key2_cpy
)
if
(
!
key2_cpy
)
...
...
sql/sql_prepare.cc
View file @
b4a5ad8d
...
@@ -3252,10 +3252,19 @@ void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length)
...
@@ -3252,10 +3252,19 @@ void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length)
void
mysqld_stmt_bulk_execute
(
THD
*
thd
,
char
*
packet_arg
,
uint
packet_length
)
void
mysqld_stmt_bulk_execute
(
THD
*
thd
,
char
*
packet_arg
,
uint
packet_length
)
{
{
uchar
*
packet
=
(
uchar
*
)
packet_arg
;
// GCC 4.0.1 workaround
uchar
*
packet
=
(
uchar
*
)
packet_arg
;
// GCC 4.0.1 workaround
DBUG_ENTER
(
"mysqld_stmt_execute_bulk"
);
const
uint
packet_header_lenght
=
4
+
2
;
//ID & 2 bytes of flags
if
(
packet_length
<
packet_header_lenght
)
{
my_error
(
ER_MALFORMED_PACKET
,
MYF
(
0
));
DBUG_VOID_RETURN
;
}
ulong
stmt_id
=
uint4korr
(
packet
);
ulong
stmt_id
=
uint4korr
(
packet
);
uint
flags
=
(
uint
)
uint2korr
(
packet
+
4
);
uint
flags
=
(
uint
)
uint2korr
(
packet
+
4
);
uchar
*
packet_end
=
packet
+
packet_length
;
uchar
*
packet_end
=
packet
+
packet_length
;
DBUG_ENTER
(
"mysqld_stmt_execute_bulk"
);
if
(
!
(
thd
->
client_capabilities
&
if
(
!
(
thd
->
client_capabilities
&
MARIADB_CLIENT_STMT_BULK_OPERATIONS
))
MARIADB_CLIENT_STMT_BULK_OPERATIONS
))
...
@@ -3263,16 +3272,18 @@ void mysqld_stmt_bulk_execute(THD *thd, char *packet_arg, uint packet_length)
...
@@ -3263,16 +3272,18 @@ void mysqld_stmt_bulk_execute(THD *thd, char *packet_arg, uint packet_length)
DBUG_PRINT
(
"error"
,
DBUG_PRINT
(
"error"
,
(
"An attempt to execute bulk operation without support"
));
(
"An attempt to execute bulk operation without support"
));
my_error
(
ER_UNSUPPORTED_PS
,
MYF
(
0
));
my_error
(
ER_UNSUPPORTED_PS
,
MYF
(
0
));
DBUG_VOID_RETURN
;
}
}
/* Check for implemented parameters */
/* Check for implemented parameters */
if
(
flags
&
(
~
STMT_BULK_FLAG_CLIENT_SEND_TYPES
))
if
(
flags
&
(
~
STMT_BULK_FLAG_CLIENT_SEND_TYPES
))
{
{
DBUG_PRINT
(
"error"
,
(
"unsupported bulk execute flags %x"
,
flags
));
DBUG_PRINT
(
"error"
,
(
"unsupported bulk execute flags %x"
,
flags
));
my_error
(
ER_UNSUPPORTED_PS
,
MYF
(
0
));
my_error
(
ER_UNSUPPORTED_PS
,
MYF
(
0
));
DBUG_VOID_RETURN
;
}
}
/* stmt id and two bytes of flags */
/* stmt id and two bytes of flags */
packet
+=
4
+
2
;
packet
+=
packet_header_lenght
;
mysql_stmt_execute_common
(
thd
,
stmt_id
,
packet
,
packet_end
,
0
,
TRUE
,
mysql_stmt_execute_common
(
thd
,
stmt_id
,
packet
,
packet_end
,
0
,
TRUE
,
(
flags
&
STMT_BULK_FLAG_CLIENT_SEND_TYPES
));
(
flags
&
STMT_BULK_FLAG_CLIENT_SEND_TYPES
));
DBUG_VOID_RETURN
;
DBUG_VOID_RETURN
;
...
@@ -3349,9 +3360,11 @@ stmt_execute_packet_sanity_check(Prepared_statement *stmt,
...
@@ -3349,9 +3360,11 @@ stmt_execute_packet_sanity_check(Prepared_statement *stmt,
{
{
/*
/*
If there is no parameters, this should be normally already end
If there is no parameters, this should be normally already end
of the packet. If it's not - then error
of the packet, but it is not a problem if something left (popular
mistake in protocol implementation) because we will not read anymore
from the buffer.
*/
*/
return
(
packet_end
>
packet
)
;
return
false
;
}
}
return
false
;
return
false
;
}
}
...
...
storage/innobase/trx/trx0rec.cc
View file @
b4a5ad8d
...
@@ -53,15 +53,18 @@ const dtuple_t trx_undo_metadata = {
...
@@ -53,15 +53,18 @@ const dtuple_t trx_undo_metadata = {
/*=========== UNDO LOG RECORD CREATION AND DECODING ====================*/
/*=========== UNDO LOG RECORD CREATION AND DECODING ====================*/
/** Calculate the free space left for extending an undo log record.
/** Calculate the free space left for extending an undo log record.
@param
[in] undo_block
undo log page
@param
undo_block
undo log page
@param
[in] ptr
current end of the undo page
@param
ptr
current end of the undo page
@return bytes left */
@return bytes left */
static
ulint
trx_undo_left
(
const
buf_block_t
*
undo_block
,
const
byte
*
ptr
)
static
ulint
trx_undo_left
(
const
buf_block_t
*
undo_block
,
const
byte
*
ptr
)
{
{
/* The 10 is a safety margin, in case we have some small
ut_ad
(
ptr
>=
&
undo_block
->
frame
[
TRX_UNDO_PAGE_HDR
+
TRX_UNDO_PAGE_HDR_SIZE
]);
calculation error below */
ut_ad
(
ptr
<=
&
undo_block
->
frame
[
srv_page_size
-
10
-
FIL_PAGE_DATA_END
]);
return
srv_page_size
-
ulint
(
ptr
-
undo_block
->
frame
)
-
(
10
+
FIL_PAGE_DATA_END
);
/* The 10 is supposed to be an extra safety margin (and needed for
compatibility with older versions) */
return
srv_page_size
-
ulint
(
ptr
-
undo_block
->
frame
)
-
(
10
+
FIL_PAGE_DATA_END
);
}
}
/**********************************************************************//**
/**********************************************************************//**
...
@@ -133,17 +136,15 @@ trx_undo_log_v_idx(
...
@@ -133,17 +136,15 @@ trx_undo_log_v_idx(
ut_ad
(
!
vcol
->
v_indexes
.
empty
());
ut_ad
(
!
vcol
->
v_indexes
.
empty
());
/* Size to reserve, max 5 bytes for each index id and position, plus
ulint
size
=
first_v_col
?
1
+
2
:
2
;
5 bytes for num of indexes, 2 bytes for write total length.
1 byte for undo log record format version marker */
ulint
size
=
5
+
2
+
(
first_v_col
?
1
:
0
);
const
ulint
avail
=
trx_undo_left
(
undo_block
,
ptr
);
const
ulint
avail
=
trx_undo_left
(
undo_block
,
ptr
);
if
(
avail
<
size
)
{
/* The mach_write_compressed(ptr, flen) in
trx_undo_page_report_modify() will consume additional 1 to 5 bytes. */
if
(
avail
<
size
+
5
)
{
return
(
NULL
);
return
(
NULL
);
}
}
size
=
0
;
ulint
n_idx
=
0
;
ulint
n_idx
=
0
;
for
(
const
auto
&
v_index
:
vcol
->
v_indexes
)
{
for
(
const
auto
&
v_index
:
vcol
->
v_indexes
)
{
n_idx
++
;
n_idx
++
;
...
@@ -151,12 +152,14 @@ trx_undo_log_v_idx(
...
@@ -151,12 +152,14 @@ trx_undo_log_v_idx(
size
+=
mach_get_compressed_size
(
uint32_t
(
v_index
.
index
->
id
));
size
+=
mach_get_compressed_size
(
uint32_t
(
v_index
.
index
->
id
));
size
+=
mach_get_compressed_size
(
v_index
.
nth_field
);
size
+=
mach_get_compressed_size
(
v_index
.
nth_field
);
}
}
size
+=
2
+
mach_get_compressed_size
(
n_idx
);
if
(
avail
<
size
)
{
size
+=
mach_get_compressed_size
(
n_idx
);
if
(
avail
<
size
+
5
)
{
return
(
NULL
);
return
(
NULL
);
}
}
ut_d
(
const
byte
*
orig_ptr
=
ptr
);
if
(
first_v_col
)
{
if
(
first_v_col
)
{
/* write the version marker */
/* write the version marker */
...
@@ -179,6 +182,8 @@ trx_undo_log_v_idx(
...
@@ -179,6 +182,8 @@ trx_undo_log_v_idx(
ptr
+=
mach_write_compressed
(
ptr
,
v_index
.
nth_field
);
ptr
+=
mach_write_compressed
(
ptr
,
v_index
.
nth_field
);
}
}
ut_ad
(
orig_ptr
+
size
==
ptr
);
mach_write_to_2
(
old_ptr
,
ulint
(
ptr
-
old_ptr
));
mach_write_to_2
(
old_ptr
,
ulint
(
ptr
-
old_ptr
));
return
(
ptr
);
return
(
ptr
);
...
@@ -394,9 +399,6 @@ trx_undo_page_report_insert(
...
@@ -394,9 +399,6 @@ trx_undo_page_report_insert(
+
undo_block
->
frame
));
+
undo_block
->
frame
));
byte
*
ptr
=
undo_block
->
frame
+
first_free
;
byte
*
ptr
=
undo_block
->
frame
+
first_free
;
ut_ad
(
first_free
>=
TRX_UNDO_PAGE_HDR
+
TRX_UNDO_PAGE_HDR_SIZE
);
ut_ad
(
first_free
<=
srv_page_size
-
FIL_PAGE_DATA_END
);
if
(
trx_undo_left
(
undo_block
,
ptr
)
<
2
+
1
+
11
+
11
)
{
if
(
trx_undo_left
(
undo_block
,
ptr
)
<
2
+
1
+
11
+
11
)
{
/* Not enough space for writing the general parameters */
/* Not enough space for writing the general parameters */
return
(
0
);
return
(
0
);
...
@@ -803,9 +805,6 @@ trx_undo_page_report_modify(
...
@@ -803,9 +805,6 @@ trx_undo_page_report_modify(
const
uint16_t
first_free
=
mach_read_from_2
(
ptr_to_first_free
);
const
uint16_t
first_free
=
mach_read_from_2
(
ptr_to_first_free
);
byte
*
ptr
=
undo_block
->
frame
+
first_free
;
byte
*
ptr
=
undo_block
->
frame
+
first_free
;
ut_ad
(
first_free
>=
TRX_UNDO_PAGE_HDR
+
TRX_UNDO_PAGE_HDR_SIZE
);
ut_ad
(
first_free
<=
srv_page_size
-
FIL_PAGE_DATA_END
);
if
(
trx_undo_left
(
undo_block
,
ptr
)
<
50
)
{
if
(
trx_undo_left
(
undo_block
,
ptr
)
<
50
)
{
/* NOTE: the value 50 must be big enough so that the general
/* NOTE: the value 50 must be big enough so that the general
fields written below fit on the undo log page */
fields written below fit on the undo log page */
...
...
tests/mysql_client_test.c
View file @
b4a5ad8d
...
@@ -21076,8 +21076,11 @@ static void test_mdev19838()
...
@@ -21076,8 +21076,11 @@ static void test_mdev19838()
" VALUES "
" VALUES "
"(0x1111111111111111)"
,
-
1
);
"(0x1111111111111111)"
,
-
1
);
/* Expecting an error if parameters are sent */
/*
DIE_UNLESS
(
rc
!=
0
||
paramCount
==
0
);
We allow junk at the end of the packet in case of
no parameters. So it will succeed.
*/
DIE_UNLESS
(
rc
==
0
);
}
}
mysql_stmt_close
(
stmt
);
mysql_stmt_close
(
stmt
);
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment