Skip to content

Commit 398fca8

Browse files
kahatlenkahatlen
committed
Bug#29866474: ALLOW CONST MYSQL_TIME ARGUMENTS IN PROTOCOL STORE FUNCTIONS
Stop modifying the MYSQL_TIME argument in Protocol_binary::store_date() and Protocol_binary::store_time(), so that they can take an immutable MYSQL_TIME as argument. Rename Protocol::store(MYSQL_TIME *, unsigned) to store_datetime(), to be consistent with how store_date() and store_time() are named. Make the functions write directly to the packet buffer instead of writing to a stack buffer and copying to the packet buffer. Microbenchmarks (64-bit, Intel Core i7-4770 3.4 GHz, GCC 8.3.0): BM_Protocol_binary_store_date 9 ns/iter -> 4 ns/iter BM_Protocol_binary_store_time 9 ns/iter -> 5 ns/iter BM_Protocol_binary_store_datetime 9 ns/iter -> 6 ns/iter Change-Id: I8cfcdeec0e973b8f268ce7c6c62528b5c9696a09
1 parent 83c323d commit 398fca8

12 files changed

+230
-93
lines changed

‎sql/field.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -4947,7 +4947,7 @@ bool Field_temporal_with_date::send_to_protocol(Protocol *protocol) const {
49474947
DBUG_ASSERT(type() == MYSQL_TYPE_TIMESTAMP);
49484948
set_zero_time(&ltime, MYSQL_TIMESTAMP_DATETIME);
49494949
}
4950-
return protocol->store(&ltime, dec);
4950+
return protocol->store_datetime(ltime, dec);
49514951
}
49524952

49534953
type_conversion_status Field_temporal_with_date::store_internal_adjust_frac(
@@ -5442,7 +5442,7 @@ bool Field_time_common::send_to_protocol(Protocol *protocol) const {
54425442
DBUG_ASSERT(0);
54435443
set_zero_time(&ltime, MYSQL_TIMESTAMP_TIME);
54445444
}
5445-
return protocol->store_time(&ltime, dec);
5445+
return protocol->store_time(ltime, dec);
54465446
}
54475447

54485448
my_time_flags_t Field_time_common::date_flags(const THD *thd) const {
@@ -5771,7 +5771,7 @@ bool Field_newdate::send_to_protocol(Protocol *protocol) const {
57715771
if (is_null()) return protocol->store_null();
57725772
MYSQL_TIME ltime;
57735773
get_date(&ltime, 0);
5774-
return protocol->store_date(&ltime);
5774+
return protocol->store_date(ltime);
57755775
}
57765776

57775777
longlong Field_newdate::val_int() const {

‎sql/item.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -6613,20 +6613,20 @@ bool Item::send(Protocol *protocol, String *buffer) {
66136613
case MYSQL_TYPE_DATE: {
66146614
MYSQL_TIME tm;
66156615
get_date(&tm, TIME_FUZZY_DATE);
6616-
if (!null_value) return protocol->store_date(&tm);
6616+
if (!null_value) return protocol->store_date(tm);
66176617
break;
66186618
}
66196619
case MYSQL_TYPE_DATETIME:
66206620
case MYSQL_TYPE_TIMESTAMP: {
66216621
MYSQL_TIME tm;
66226622
get_date(&tm, TIME_FUZZY_DATE);
6623-
if (!null_value) return protocol->store(&tm, decimals);
6623+
if (!null_value) return protocol->store_datetime(tm, decimals);
66246624
break;
66256625
}
66266626
case MYSQL_TYPE_TIME: {
66276627
MYSQL_TIME tm;
66286628
get_time(&tm);
6629-
if (!null_value) return protocol->store_time(&tm, decimals);
6629+
if (!null_value) return protocol->store_time(tm, decimals);
66306630
break;
66316631
}
66326632
}

‎sql/protocol.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,9 @@ class Protocol {
140140
String *buffer) = 0;
141141
virtual bool store(double from, uint32 decimals, uint32 zerofill,
142142
String *buffer) = 0;
143-
virtual bool store(MYSQL_TIME *time, uint precision) = 0;
144-
virtual bool store_date(MYSQL_TIME *time) = 0;
145-
virtual bool store_time(MYSQL_TIME *time, uint precision) = 0;
143+
virtual bool store_datetime(const MYSQL_TIME &time, uint precision) = 0;
144+
virtual bool store_date(const MYSQL_TIME &time) = 0;
145+
virtual bool store_time(const MYSQL_TIME &time, uint precision) = 0;
146146
virtual bool store_field(const Field *field) = 0;
147147
// Convenience wrappers
148148
bool store(int from) { return store_long(longlong{from}, 0); }

‎sql/protocol_callback.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -144,20 +144,20 @@ bool Protocol_callback::store(double from, uint32 decimals, uint32, String *) {
144144
return false;
145145
}
146146

147-
bool Protocol_callback::store(MYSQL_TIME *time, uint precision) {
147+
bool Protocol_callback::store_datetime(const MYSQL_TIME &time, uint precision) {
148148
if (callbacks.get_datetime)
149-
return callbacks.get_datetime(callbacks_ctx, time, precision);
149+
return callbacks.get_datetime(callbacks_ctx, &time, precision);
150150
return false;
151151
}
152152

153-
bool Protocol_callback::store_date(MYSQL_TIME *time) {
154-
if (callbacks.get_datetime) return callbacks.get_date(callbacks_ctx, time);
153+
bool Protocol_callback::store_date(const MYSQL_TIME &time) {
154+
if (callbacks.get_datetime) return callbacks.get_date(callbacks_ctx, &time);
155155
return false;
156156
}
157157

158-
bool Protocol_callback::store_time(MYSQL_TIME *time, uint precision) {
158+
bool Protocol_callback::store_time(const MYSQL_TIME &time, uint precision) {
159159
if (callbacks.get_time)
160-
return callbacks.get_time(callbacks_ctx, time, precision);
160+
return callbacks.get_time(callbacks_ctx, &time, precision);
161161
return false;
162162
}
163163

‎sql/protocol_callback.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ class Protocol_callback final : public Protocol {
205205
false success
206206
true failure
207207
*/
208-
bool store(MYSQL_TIME *time, uint precision) override;
208+
bool store_datetime(const MYSQL_TIME &time, uint precision) override;
209209

210210
/**
211211
Sends DATE value
@@ -216,7 +216,7 @@ class Protocol_callback final : public Protocol {
216216
false success
217217
true failure
218218
*/
219-
bool store_date(MYSQL_TIME *time) override;
219+
bool store_date(const MYSQL_TIME &time) override;
220220

221221
/**
222222
Sends TIME value
@@ -228,7 +228,7 @@ class Protocol_callback final : public Protocol {
228228
false success
229229
true failure
230230
*/
231-
bool store_time(MYSQL_TIME *time, uint precision) override;
231+
bool store_time(const MYSQL_TIME &time, uint precision) override;
232232

233233
/**
234234
Sends Field

‎sql/protocol_classic.cc

+82-49
Original file line numberDiff line numberDiff line change
@@ -3373,39 +3373,39 @@ bool Protocol_text::store(double from, uint32 decimals, uint32 zerofill,
33733373
we support 0-6 decimals for time.
33743374
*/
33753375

3376-
bool Protocol_text::store(MYSQL_TIME *tm, uint decimals) {
3376+
bool Protocol_text::store_datetime(const MYSQL_TIME &tm, uint decimals) {
33773377
#ifndef DBUG_OFF
33783378
// field_types check is needed because of the embedded protocol
33793379
DBUG_ASSERT(send_metadata || field_types == 0 ||
33803380
is_temporal_type_with_date_and_time(field_types[field_pos]));
33813381
field_pos++;
33823382
#endif
33833383
char buff[MAX_DATE_STRING_REP_LENGTH];
3384-
size_t length = my_datetime_to_str(*tm, buff, decimals);
3384+
size_t length = my_datetime_to_str(tm, buff, decimals);
33853385
return net_store_data(pointer_cast<const uchar *>(buff), length, packet);
33863386
}
33873387

3388-
bool Protocol_text::store_date(MYSQL_TIME *tm) {
3388+
bool Protocol_text::store_date(const MYSQL_TIME &tm) {
33893389
#ifndef DBUG_OFF
33903390
// field_types check is needed because of the embedded protocol
33913391
DBUG_ASSERT(send_metadata || field_types == 0 ||
33923392
field_types[field_pos] == MYSQL_TYPE_DATE);
33933393
field_pos++;
33943394
#endif
33953395
char buff[MAX_DATE_STRING_REP_LENGTH];
3396-
size_t length = my_date_to_str(*tm, buff);
3396+
size_t length = my_date_to_str(tm, buff);
33973397
return net_store_data(pointer_cast<const uchar *>(buff), length, packet);
33983398
}
33993399

3400-
bool Protocol_text::store_time(MYSQL_TIME *tm, uint decimals) {
3400+
bool Protocol_text::store_time(const MYSQL_TIME &tm, uint decimals) {
34013401
#ifndef DBUG_OFF
34023402
// field_types check is needed because of the embedded protocol
34033403
DBUG_ASSERT(send_metadata || field_types == 0 ||
34043404
field_types[field_pos] == MYSQL_TYPE_TIME);
34053405
field_pos++;
34063406
#endif
34073407
char buff[MAX_DATE_STRING_REP_LENGTH];
3408-
size_t length = my_time_to_str(*tm, buff, decimals);
3408+
size_t length = my_time_to_str(tm, buff, decimals);
34093409
return net_store_data(pointer_cast<const uchar *>(buff), length, packet);
34103410
}
34113411

@@ -3683,82 +3683,115 @@ bool Protocol_binary::store(double from, uint32 decimals, uint32 zerofill,
36833683
return 0;
36843684
}
36853685

3686-
bool Protocol_binary::store(MYSQL_TIME *tm, uint precision) {
3687-
if (send_metadata) return Protocol_text::store(tm, precision);
3686+
bool Protocol_binary::store_datetime(const MYSQL_TIME &tm, uint precision) {
3687+
if (send_metadata) return Protocol_text::store_datetime(tm, precision);
36883688

36893689
#ifndef DBUG_OFF
36903690
// field_types check is needed because of the embedded protocol
36913691
DBUG_ASSERT(field_types == nullptr ||
3692-
field_types[field_pos] == MYSQL_TYPE_DATE ||
36933692
is_temporal_type_with_date_and_time(field_types[field_pos]));
36943693
#endif
3695-
char buff[12], *pos;
3696-
size_t length;
36973694
field_pos++;
3698-
pos = buff + 1;
3699-
3700-
int2store(pos, tm->year);
3701-
pos[2] = (uchar)tm->month;
3702-
pos[3] = (uchar)tm->day;
3703-
pos[4] = (uchar)tm->hour;
3704-
pos[5] = (uchar)tm->minute;
3705-
pos[6] = (uchar)tm->second;
3706-
int4store(pos + 7, tm->second_part);
3707-
if (tm->second_part)
3695+
3696+
size_t length;
3697+
if (tm.second_part)
37083698
length = 11;
3709-
else if (tm->hour || tm->minute || tm->second)
3699+
else if (tm.hour || tm.minute || tm.second)
37103700
length = 7;
3711-
else if (tm->year || tm->month || tm->day)
3701+
else if (tm.year || tm.month || tm.day)
37123702
length = 4;
37133703
else
37143704
length = 0;
3715-
buff[0] = (char)length; // Length is stored first
3716-
return packet->append(buff, length + 1, PACKET_BUFFER_EXTRA_ALLOC);
3705+
3706+
char *pos = packet->prep_append(length + 1, PACKET_BUFFER_EXTRA_ALLOC);
3707+
if (pos == nullptr) return true;
3708+
3709+
*pos++ = char(length);
3710+
3711+
const char *const end = pos + length;
3712+
if (pos == end) return false; // Only zero parts.
3713+
3714+
int2store(pos, tm.year);
3715+
pos += 2;
3716+
*pos++ = char(tm.month);
3717+
*pos++ = char(tm.day);
3718+
3719+
if (pos == end) return false; // Only date parts.
3720+
3721+
*pos++ = char(tm.hour);
3722+
*pos++ = char(tm.minute);
3723+
*pos++ = char(tm.second);
3724+
3725+
if (pos == end) return false; // No microseconds.
3726+
3727+
int4store(pos, tm.second_part);
3728+
DBUG_ASSERT(pos + 4 == end);
3729+
return false;
37173730
}
37183731

3719-
bool Protocol_binary::store_date(MYSQL_TIME *tm) {
3732+
bool Protocol_binary::store_date(const MYSQL_TIME &tm) {
37203733
if (send_metadata) return Protocol_text::store_date(tm);
37213734
#ifndef DBUG_OFF
37223735
// field_types check is needed because of the embedded protocol
37233736
DBUG_ASSERT(field_types == nullptr ||
37243737
field_types[field_pos] == MYSQL_TYPE_DATE);
37253738
#endif
3726-
tm->hour = tm->minute = tm->second = 0;
3727-
tm->second_part = 0;
3728-
return Protocol_binary::store(tm, 0);
3739+
field_pos++;
3740+
3741+
if (tm.year == 0 && tm.month == 0 && tm.day == 0) {
3742+
// Nothing to send, except a single byte to indicate length = 0.
3743+
return packet->append(char{0});
3744+
}
3745+
3746+
char *pos = packet->prep_append(5, PACKET_BUFFER_EXTRA_ALLOC);
3747+
if (pos == nullptr) return true;
3748+
pos[0] = char{4}; // length
3749+
int2store(pos + 1, tm.year);
3750+
pos[3] = char(tm.month);
3751+
pos[4] = char(tm.day);
3752+
return false;
37293753
}
37303754

3731-
bool Protocol_binary::store_time(MYSQL_TIME *tm, uint precision) {
3755+
bool Protocol_binary::store_time(const MYSQL_TIME &tm, uint precision) {
37323756
if (send_metadata) return Protocol_text::store_time(tm, precision);
3733-
char buff[13], *pos;
3734-
size_t length;
37353757
#ifndef DBUG_OFF
37363758
// field_types check is needed because of the embedded protocol
37373759
DBUG_ASSERT(field_types == nullptr ||
37383760
field_types[field_pos] == MYSQL_TYPE_TIME);
37393761
#endif
37403762
field_pos++;
3741-
pos = buff + 1;
3742-
pos[0] = tm->neg ? 1 : 0;
3743-
if (tm->hour >= 24) {
3744-
// Move hours to days if we have 24 hours or more.
3745-
uint days = tm->hour / 24;
3746-
tm->hour -= days * 24;
3747-
tm->day += days;
3748-
}
3749-
int4store(pos + 1, tm->day);
3750-
pos[5] = (uchar)tm->hour;
3751-
pos[6] = (uchar)tm->minute;
3752-
pos[7] = (uchar)tm->second;
3753-
int4store(pos + 8, tm->second_part);
3754-
if (tm->second_part)
3763+
3764+
size_t length;
3765+
if (tm.second_part)
37553766
length = 12;
3756-
else if (tm->hour || tm->minute || tm->second || tm->day)
3767+
else if (tm.hour || tm.minute || tm.second || tm.day)
37573768
length = 8;
37583769
else
37593770
length = 0;
3760-
buff[0] = (char)length; // Length is stored first
3761-
return packet->append(buff, length + 1, PACKET_BUFFER_EXTRA_ALLOC);
3771+
3772+
char *pos = packet->prep_append(length + 1, PACKET_BUFFER_EXTRA_ALLOC);
3773+
if (pos == nullptr) return false;
3774+
*pos++ = char(length);
3775+
3776+
const char *const end = pos + length;
3777+
if (pos == end) return false; // zero date
3778+
3779+
// Move hours to days if we have 24 hours or more.
3780+
const unsigned days = tm.day + tm.hour / 24;
3781+
const unsigned hours = tm.hour % 24;
3782+
3783+
*pos++ = tm.neg ? 1 : 0;
3784+
int4store(pos, days);
3785+
pos += 4;
3786+
*pos++ = char(hours);
3787+
*pos++ = char(tm.minute);
3788+
*pos++ = char(tm.second);
3789+
3790+
if (pos == end) return false; // no second part
3791+
3792+
int4store(pos, tm.second_part);
3793+
DBUG_ASSERT(pos + 4 == end);
3794+
return false;
37623795
}
37633796

37643797
/**

‎sql/protocol_classic.h

+6-6
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,9 @@ class Protocol_text : public Protocol_classic {
221221
String *buffer) override;
222222
bool store(double from, uint32 decimals, uint32 zerofill,
223223
String *buffer) override;
224-
bool store(MYSQL_TIME *time, uint precision) override;
225-
bool store_date(MYSQL_TIME *time) override;
226-
bool store_time(MYSQL_TIME *time, uint precision) override;
224+
bool store_datetime(const MYSQL_TIME &time, uint precision) override;
225+
bool store_date(const MYSQL_TIME &time) override;
226+
bool store_time(const MYSQL_TIME &time, uint precision) override;
227227
void start_row() override;
228228
bool send_parameters(List<Item_param> *parameters, bool) override;
229229

@@ -249,9 +249,9 @@ class Protocol_binary final : public Protocol_text {
249249
bool store_longlong(longlong from, bool unsigned_flag,
250250
uint32 zerofill) override;
251251
bool store_decimal(const my_decimal *, uint, uint) override;
252-
bool store(MYSQL_TIME *time, uint precision) override;
253-
bool store_date(MYSQL_TIME *time) override;
254-
bool store_time(MYSQL_TIME *time, uint precision) override;
252+
bool store_datetime(const MYSQL_TIME &time, uint precision) override;
253+
bool store_date(const MYSQL_TIME &time) override;
254+
bool store_time(const MYSQL_TIME &time, uint precision) override;
255255
bool store(float nr, uint32 decimals, uint32 zerofill,
256256
String *buffer) override;
257257
bool store(double from, uint32 decimals, uint32 zerofill,

‎sql/sql_prepare.cc

+9-11
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,9 @@ class Protocol_local final : public Protocol {
248248
bool store_longlong(longlong from, bool unsigned_flag, uint32) override;
249249
bool store_decimal(const my_decimal *, uint, uint) override;
250250
bool store(const char *from, size_t length, const CHARSET_INFO *cs) override;
251-
bool store(MYSQL_TIME *time, uint precision) override;
252-
bool store_date(MYSQL_TIME *time) override;
253-
bool store_time(MYSQL_TIME *time, uint precision) override;
251+
bool store_datetime(const MYSQL_TIME &time, uint precision) override;
252+
bool store_date(const MYSQL_TIME &time) override;
253+
bool store_time(const MYSQL_TIME &time, uint precision) override;
254254
bool store(float value, uint32 decimals, uint32 zerofill,
255255
String *buffer) override;
256256
bool store(double value, uint32 decimals, uint32 zerofill,
@@ -3487,22 +3487,20 @@ bool Protocol_local::store(const char *str, size_t length,
34873487

34883488
/* Store MYSQL_TIME (in binary format) */
34893489

3490-
bool Protocol_local::store(MYSQL_TIME *time,
3491-
uint precision MY_ATTRIBUTE((unused))) {
3492-
return store_column(time, sizeof(MYSQL_TIME));
3490+
bool Protocol_local::store_datetime(const MYSQL_TIME &time, uint) {
3491+
return store_column(&time, sizeof(MYSQL_TIME));
34933492
}
34943493

34953494
/** Store MYSQL_TIME (in binary format) */
34963495

3497-
bool Protocol_local::store_date(MYSQL_TIME *time) {
3498-
return store_column(time, sizeof(MYSQL_TIME));
3496+
bool Protocol_local::store_date(const MYSQL_TIME &time) {
3497+
return store_column(&time, sizeof(MYSQL_TIME));
34993498
}
35003499

35013500
/** Store MYSQL_TIME (in binary format) */
35023501

3503-
bool Protocol_local::store_time(MYSQL_TIME *time,
3504-
uint precision MY_ATTRIBUTE((unused))) {
3505-
return store_column(time, sizeof(MYSQL_TIME));
3502+
bool Protocol_local::store_time(const MYSQL_TIME &time, uint) {
3503+
return store_column(&time, sizeof(MYSQL_TIME));
35063504
}
35073505

35083506
/* Store a floating point number, as is. */

‎sql/sql_show.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -4164,7 +4164,7 @@ static bool show_create_trigger_impl(THD *thd, Trigger *trigger) {
41644164
if (!trigger->is_created_timestamp_null()) {
41654165
MYSQL_TIME timestamp;
41664166
my_tz_SYSTEM->gmt_sec_to_TIME(&timestamp, trigger->get_created_timestamp());
4167-
rc = p->store(&timestamp, 2);
4167+
rc = p->store_datetime(timestamp, 2);
41684168
} else
41694169
rc = p->store_null();
41704170

0 commit comments

Comments
 (0)