Skip to content

Commit 2025e74

Browse files
silvakidsilvakid
committed
Bug #29394232: REPREPARE WITH SORT NOT HAPPENING
1 parent 134de74 commit 2025e74

File tree

4 files changed

+101
-20
lines changed

4 files changed

+101
-20
lines changed

‎cdk/protocol/mysqlx/crud.cc

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,7 @@ void set_arg_scalar(Mysqlx::Datatypes::Any &msg, row_count_t val)
100100
set_arg_scalar(*msg.mutable_scalar(), val);
101101
}
102102

103-
104-
105-
106-
template <class MSG,
103+
template <bool enable_offset,class MSG,
107104
class MSG_Params>
108105
void set_limit_(const api::Limit &lim,
109106
MSG &msg,
@@ -116,9 +113,15 @@ void set_limit_(const api::Limit &lim,
116113
row_count_msg->set_position(0);
117114
set_arg_scalar(*msg_args.add_args(), lim.get_row_count());
118115

119-
auto offset_msg = limit->mutable_offset();
120-
offset_msg->set_type(Mysqlx::Expr::Expr::PLACEHOLDER);
121-
offset_msg->set_position(1);
116+
if (enable_offset)
117+
{
118+
auto offset_msg = limit->mutable_offset();
119+
offset_msg->set_type(Mysqlx::Expr::Expr::PLACEHOLDER);
120+
offset_msg->set_position(1);
121+
}
122+
//set_arg_scalar is out of enable_offset because the offset ARG is always sent
123+
//on the PrepareExecute, becaus ethere we don't know which type of message we
124+
//are executing
122125
set_arg_scalar(*msg_args.add_args(),
123126
lim.get_offset() ? *lim.get_offset() : 0);
124127

@@ -137,7 +140,7 @@ void set_limit_(const api::Limit &lim,
137140
proto_lim->set_offset(*lim_offset);
138141
}
139142

140-
template<class MSG,
143+
template<bool enable_offset,class MSG,
141144
class MSG_Params>
142145
void set_limit_(const api::Limit &limit,
143146
MSG &msg,
@@ -146,7 +149,7 @@ void set_limit_(const api::Limit &limit,
146149
{
147150
conv.set_offset(2);
148151

149-
set_limit_(limit, msg, msg_args);
152+
set_limit_<enable_offset>(limit, msg, msg_args);
150153
}
151154

152155

@@ -423,7 +426,7 @@ void Msg_builder<T>::set_limit(const api::Limit *limit)
423426
if (limit)
424427
{
425428
if (m_stmt_id != 0)
426-
set_limit_(*limit, m_msg, m_conv, m_prepare_execute);
429+
set_limit_<Prepare_traits<T>::has_offset>(*limit, m_msg, m_conv, m_prepare_execute);
427430
else
428431
set_limit_(*limit, m_msg);
429432
}

‎cdk/protocol/mysqlx/protocol.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,8 @@ struct Prepare_traits<msg_type::cli_CrudFind>
837837
{
838838
typedef Mysqlx::Crud::Find msg_type;
839839

840+
static const bool has_offset = true;
841+
840842
static void set_one(Mysqlx::Prepare::Prepare &prepare, msg_type &msg)
841843
{
842844
auto &one = *prepare.mutable_stmt();
@@ -856,6 +858,8 @@ struct Prepare_traits<msg_type::cli_CrudInsert>
856858
{
857859
typedef Mysqlx::Crud::Insert msg_type;
858860

861+
static const bool has_offset = true;
862+
859863
static void set_one(Mysqlx::Prepare::Prepare &prepare, msg_type &msg)
860864
{
861865
auto &one = *prepare.mutable_stmt();
@@ -876,6 +880,8 @@ struct Prepare_traits<msg_type::cli_CrudUpdate>
876880
{
877881
typedef Mysqlx::Crud::Update msg_type;
878882

883+
static const bool has_offset = false;
884+
879885
static void set_one(Mysqlx::Prepare::Prepare &prepare, msg_type &msg)
880886
{
881887
auto &one = *prepare.mutable_stmt();
@@ -896,6 +902,8 @@ struct Prepare_traits<msg_type::cli_CrudDelete>
896902
{
897903
typedef Mysqlx::Crud::Delete msg_type;
898904

905+
static const bool has_offset = false;
906+
899907
static void set_one(Mysqlx::Prepare::Prepare &prepare, msg_type &msg)
900908
{
901909
auto &one = *prepare.mutable_stmt();

‎common/op_impl.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,11 +790,14 @@ class Op_sort
790790

791791
void add_sort(const string &sort) override
792792
{
793+
Base::set_prepare_state(Base::PS_EXECUTE);
793794
m_order.emplace_back(sort);
794795
}
795796

796797
void clear_sort() override
797798
{
799+
if (get_order_by())
800+
Base::set_prepare_state(Base::PS_EXECUTE);
798801
m_order.clear();
799802
}
800803

@@ -885,6 +888,8 @@ class Op_having
885888

886889
void clear_having() override
887890
{
891+
if (get_having())
892+
Base::set_prepare_state(Base::PS_EXECUTE);
888893
m_having.clear();
889894
}
890895

@@ -930,11 +935,14 @@ class Op_group_by
930935

931936
void add_group_by(const string &group_by) override
932937
{
938+
Base::set_prepare_state(Base::PS_EXECUTE);
933939
m_group_by.push_back(group_by);
934940
}
935941

936942
void clear_group_by() override
937943
{
944+
if (get_group_by())
945+
Base::set_prepare_state(Base::PS_EXECUTE);
938946
m_group_by.clear();
939947
}
940948

@@ -1011,8 +1019,9 @@ class Op_projection
10111019

10121020
void clear_proj() override
10131021
{
1022+
if (get_tbl_proj())
1023+
Base::set_prepare_state(Base::PS_EXECUTE);
10141024
m_projections.clear();
1015-
Base::set_prepare_state(Base::PS_EXECUTE);
10161025
}
10171026

10181027
cdk::Projection* get_tbl_proj()

‎devapi/tests/crud-t.cc

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,13 +2623,14 @@ TEST_F(Crud, PS)
26232623
}
26242624
};
26252625

2626+
//-1 means not set
26262627
auto execute_find = [&finds](int limit, int offset, int expected)
26272628
{
26282629
for (auto &find : finds)
26292630
{
2630-
if (limit != 0)
2631+
if (limit != -1)
26312632
find.limit(limit);
2632-
if(offset != 0)
2633+
if(offset != -1)
26332634
find.offset(offset);
26342635

26352636

@@ -2642,14 +2643,31 @@ TEST_F(Crud, PS)
26422643

26432644
};
26442645

2646+
auto execute_find_sort = [&finds](bool set_sort, int expected)
2647+
{
2648+
for (auto &find : finds)
2649+
{
2650+
if (set_sort)
2651+
find.sort("name DESC");
2652+
2653+
2654+
EXPECT_EQ(expected,
2655+
find
2656+
.bind("name", "%")
2657+
.bind("age", 1000)
2658+
.execute().count());
2659+
}
2660+
2661+
};
2662+
26452663
for (int i = 0; i < 2; ++i)
26462664
{
26472665
create_find();
26482666

26492667
auto start_time = std::chrono::system_clock::now();
26502668

26512669
//direct_execute
2652-
execute_find(0, 0, 6);
2670+
execute_find(-1, -1, 6);
26532671

26542672
std::cout << "Direct Execute: "
26552673
<< std::chrono::duration_cast<std::chrono::milliseconds>(
@@ -2659,7 +2677,7 @@ TEST_F(Crud, PS)
26592677

26602678
//prepare+execute
26612679
//Even if limit/offset changes, it will not fallback to the direct execute
2662-
execute_find(6,0,6);
2680+
execute_find(6,-1,6);
26632681

26642682
std::cout << "Prepare+Execute PS: "
26652683
<< std::chrono::duration_cast<std::chrono::milliseconds>(
@@ -2668,7 +2686,7 @@ TEST_F(Crud, PS)
26682686
start_time = std::chrono::system_clock::now();
26692687

26702688
//execute prepared
2671-
execute_find(6, 0, 6);
2689+
execute_find(6, -1, 6);
26722690

26732691
std::cout << "Execute PS: "
26742692
<< std::chrono::duration_cast<std::chrono::milliseconds>(
@@ -2681,23 +2699,66 @@ TEST_F(Crud, PS)
26812699
finds.clear();
26822700
create_find();
26832701
//Execute
2684-
execute_find(0, 0, 6);
2702+
execute_find(-1, -1, 6);
26852703
//Prepare+Execute
2686-
execute_find(0, 0, 6);
2704+
execute_find(-1, -1, 6);
26872705
//ExecutePrepared
2688-
execute_find(0, 0, 6);
2706+
execute_find(-1, -1, 6);
26892707
//Prepare+Execute
2690-
execute_find(0, 5, 1);
2708+
execute_find(-1, 5, 1);
26912709
//ExecutePrepared
26922710
execute_find(1, 0, 1);
26932711
//ExecutePrepared
26942712
execute_find(1, 1, 1);
26952713
//ExecutePrepared
26962714
execute_find(1, 1, 1);
26972715

2716+
//SET SORT
2717+
//Execute
2718+
execute_find_sort(true, 1);
2719+
//Prepare+Execute
2720+
execute_find_sort(false, 1);
2721+
//ExecutePrepared
2722+
execute_find_sort(false, 1);
2723+
26982724
//clean upp the finds for next round
26992725
finds.clear();
27002726
create_find();
27012727
}
27022728

2729+
//Modify prepare
2730+
{
2731+
auto modify = coll.modify("name like :name").set("age","age+1");
2732+
modify.bind("name","foo");
2733+
//Execute
2734+
modify.execute();
2735+
//Prepare+Execute
2736+
modify.execute();
2737+
//ExecutePrepared
2738+
modify.execute();
2739+
//Execute
2740+
modify.limit(1).execute();
2741+
//Prepare+Execute
2742+
modify.execute();
2743+
//ExecutePrepared
2744+
modify.execute();
2745+
}
2746+
2747+
//Remove prepare
2748+
{
2749+
auto modify = coll.remove("age > 10");
2750+
//Execute
2751+
modify.execute();
2752+
//Prepare+Execute
2753+
modify.execute();
2754+
//ExecutePrepared
2755+
modify.execute();
2756+
//Execute
2757+
modify.limit(1).execute();
2758+
//Prepare+Execute
2759+
modify.execute();
2760+
//ExecutePrepared
2761+
modify.execute();
27032762
}
2763+
2764+
}

0 commit comments

Comments
 (0)