Drizzle

Merge lp:~stewart/drizzle/enum_field_type_to_table_message_type into lp:~drizzle-trunk/drizzle/development

  1. enum_field_type_to_table_message_type
  2. Merge into development
Proposed by Stewart Smith
Status: Merged
Merged at revision: not available
Proposed branch: lp:~stewart/drizzle/enum_field_type_to_table_message_type
Merge into: lp:~drizzle-trunk/drizzle/development
Prerequisite: lp:~stewart/drizzle/create-tmp-table-name
Diff against target: 269 lines (+54/-67)
4 files modified
drizzled/message/statement_transform.cc (+32/-0)
drizzled/message/statement_transform.h (+4/-0)
drizzled/replication_services.cc (+5/-35)
drizzled/table_proto_write.cc (+13/-32)
To merge this branch: bzr merge lp:~stewart/drizzle/enum_field_type_to_table_message_type
Reviewer Review Type Date Requested Status
Jay Pipes (community) Approve
Brian Aker Pending
Review via email: mp+18680@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Stewart Smith (stewart) wrote :

Move out conversion between server and proto types to common code.

Revision history for this message
Jay Pipes (jaypipes) wrote :

Hi!

Unfortunately, this code duplicates a function I put in /drizzled/replication_services.cc:

static message::Table::Field::FieldType internalFieldTypeToFieldProtoType(enum enum_field_types in_type);

static message::Table::Field::FieldType internalFieldTypeToFieldProtoType(enum enum_field_types in_type)
{
  switch (in_type)
  {
    case DRIZZLE_TYPE_LONGLONG:
      return message::Table::Field::BIGINT;
    case DRIZZLE_TYPE_LONG:
      return message::Table::Field::INTEGER;
    case DRIZZLE_TYPE_DECIMAL:
      return message::Table::Field::DECIMAL;
    case DRIZZLE_TYPE_DOUBLE:
      return message::Table::Field::DOUBLE;
    case DRIZZLE_TYPE_DATE:
      return message::Table::Field::DATE;
    case DRIZZLE_TYPE_DATETIME:
      return message::Table::Field::DATETIME;
    case DRIZZLE_TYPE_TIMESTAMP:
      return message::Table::Field::TIMESTAMP;
    case DRIZZLE_TYPE_VARCHAR:
      return message::Table::Field::VARCHAR;
    case DRIZZLE_TYPE_BLOB:
      return message::Table::Field::BLOB;
    case DRIZZLE_TYPE_ENUM:
      return message::Table::Field::ENUM;
    default:
      return message::Table::Field::VARCHAR;
  }
}

Would you mind doing the following:

1) Move the above function into the statement_transform library code in /drizzled/message/statement_transform.h|.cc

2) #include "drizzled/message/statement_transform.h" in /drizzled/table_proto_write.cc and remove the duplicated code

3) Adding that include to the /drizzled/replication_services.cc file, too?

Less duplicated code is A Good Thing! :)

Thanks!

Jay

review: Needs Fixing
lp:~stewart/drizzle/enum_field_type_to_table_message_type updated
1293. By Stewart Smith

de-duplicate internal field type to field proto field type code. Consolidate that in replication_services and table_proto_write and now just have it in statement_transform.

Revision history for this message
Jay Pipes (jaypipes) wrote :

Cheers!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'drizzled/message/statement_transform.cc'
2--- drizzled/message/statement_transform.cc 2010-02-04 08:14:46 +0000
3+++ drizzled/message/statement_transform.cc 2010-02-08 01:34:13 +0000
4@@ -707,4 +707,36 @@
5 }
6 }
7
8+drizzled::message::Table::Field::FieldType message::internalFieldTypeToFieldProtoType(enum enum_field_types type)
9+{
10+ switch (type) {
11+ case DRIZZLE_TYPE_LONG:
12+ return message::Table::Field::INTEGER;
13+ case DRIZZLE_TYPE_DOUBLE:
14+ return message::Table::Field::DOUBLE;
15+ case DRIZZLE_TYPE_NULL:
16+ assert(false); /* Not a user definable type */
17+ return message::Table::Field::INTEGER; /* unreachable */
18+ case DRIZZLE_TYPE_TIMESTAMP:
19+ return message::Table::Field::TIMESTAMP;
20+ case DRIZZLE_TYPE_LONGLONG:
21+ return message::Table::Field::BIGINT;
22+ case DRIZZLE_TYPE_DATETIME:
23+ return message::Table::Field::DATETIME;
24+ case DRIZZLE_TYPE_DATE:
25+ return message::Table::Field::DATE;
26+ case DRIZZLE_TYPE_VARCHAR:
27+ return message::Table::Field::VARCHAR;
28+ case DRIZZLE_TYPE_DECIMAL:
29+ return message::Table::Field::DECIMAL;
30+ case DRIZZLE_TYPE_ENUM:
31+ return message::Table::Field::ENUM;
32+ case DRIZZLE_TYPE_BLOB:
33+ return message::Table::Field::BLOB;
34+ }
35+
36+ assert(false);
37+ return message::Table::Field::INTEGER; /* unreachable */
38+}
39+
40 } /* namespace drizzled */
41
42=== modified file 'drizzled/message/statement_transform.h'
43--- drizzled/message/statement_transform.h 2010-02-04 08:14:46 +0000
44+++ drizzled/message/statement_transform.h 2010-02-08 01:34:13 +0000
45@@ -35,6 +35,8 @@
46 #include <string>
47 #include <vector>
48
49+#include "drizzled/common.h"
50+
51 namespace drizzled
52 {
53 namespace message
54@@ -308,6 +310,8 @@
55 */
56 bool shouldQuoteFieldValue(Table::Field::FieldType in_type);
57
58+drizzled::message::Table::Field::FieldType internalFieldTypeToFieldProtoType(enum enum_field_types type);
59+
60 } /* namespace message */
61 } /* namespace drizzled */
62
63
64=== modified file 'drizzled/replication_services.cc'
65--- drizzled/replication_services.cc 2010-02-04 08:14:46 +0000
66+++ drizzled/replication_services.cc 2010-02-08 01:34:13 +0000
67@@ -56,6 +56,7 @@
68 #include "drizzled/plugin/transaction_applier.h"
69 #include "drizzled/message/transaction.pb.h"
70 #include "drizzled/message/table.pb.h"
71+#include "drizzled/message/statement_transform.h"
72 #include "drizzled/gettext.h"
73 #include "drizzled/session.h"
74 #include "drizzled/error.h"
75@@ -67,8 +68,6 @@
76 namespace drizzled
77 {
78
79-static message::Table::Field::FieldType internalFieldTypeToFieldProtoType(enum enum_field_types in_type);
80-
81 ReplicationServices::ReplicationServices()
82 {
83 is_active= false;
84@@ -392,7 +391,7 @@
85 {
86 field_metadata= header->add_field_metadata();
87 field_metadata->set_name(current_field->field_name);
88- field_metadata->set_type(internalFieldTypeToFieldProtoType(current_field->type()));
89+ field_metadata->set_type(message::internalFieldTypeToFieldProtoType(current_field->type()));
90 }
91 }
92
93@@ -513,7 +512,7 @@
94 {
95 field_metadata= header->add_key_field_metadata();
96 field_metadata->set_name(current_field->field_name);
97- field_metadata->set_type(internalFieldTypeToFieldProtoType(current_field->type()));
98+ field_metadata->set_type(message::internalFieldTypeToFieldProtoType(current_field->type()));
99 }
100
101 /*
102@@ -531,7 +530,7 @@
103 /* Field is changed from old to new */
104 field_metadata= header->add_set_field_metadata();
105 field_metadata->set_name(current_field->field_name);
106- field_metadata->set_type(internalFieldTypeToFieldProtoType(current_field->type()));
107+ field_metadata->set_type(message::internalFieldTypeToFieldProtoType(current_field->type()));
108 }
109 }
110 }
111@@ -687,7 +686,7 @@
112 {
113 field_metadata= header->add_key_field_metadata();
114 field_metadata->set_name(current_field->field_name);
115- field_metadata->set_type(internalFieldTypeToFieldProtoType(current_field->type()));
116+ field_metadata->set_type(message::internalFieldTypeToFieldProtoType(current_field->type()));
117 }
118 }
119 }
120@@ -824,33 +823,4 @@
121 }
122 }
123
124-static message::Table::Field::FieldType internalFieldTypeToFieldProtoType(enum enum_field_types in_type)
125-{
126- switch (in_type)
127- {
128- case DRIZZLE_TYPE_LONGLONG:
129- return message::Table::Field::BIGINT;
130- case DRIZZLE_TYPE_LONG:
131- return message::Table::Field::INTEGER;
132- case DRIZZLE_TYPE_DECIMAL:
133- return message::Table::Field::DECIMAL;
134- case DRIZZLE_TYPE_DOUBLE:
135- return message::Table::Field::DOUBLE;
136- case DRIZZLE_TYPE_DATE:
137- return message::Table::Field::DATE;
138- case DRIZZLE_TYPE_DATETIME:
139- return message::Table::Field::DATETIME;
140- case DRIZZLE_TYPE_TIMESTAMP:
141- return message::Table::Field::TIMESTAMP;
142- case DRIZZLE_TYPE_VARCHAR:
143- return message::Table::Field::VARCHAR;
144- case DRIZZLE_TYPE_BLOB:
145- return message::Table::Field::BLOB;
146- case DRIZZLE_TYPE_ENUM:
147- return message::Table::Field::ENUM;
148- default:
149- return message::Table::Field::VARCHAR;
150- }
151-}
152-
153 } /* namespace drizzled */
154
155=== modified file 'drizzled/table_proto_write.cc'
156--- drizzled/table_proto_write.cc 2010-02-04 08:14:46 +0000
157+++ drizzled/table_proto_write.cc 2010-02-08 01:34:13 +0000
158@@ -19,6 +19,7 @@
159 #include <drizzled/unireg.h>
160 #include "drizzled/sql_table.h"
161 #include "drizzled/global_charset_info.h"
162+#include "drizzled/message/statement_transform.h"
163
164 #include "drizzled/internal/my_sys.h"
165
166@@ -94,15 +95,14 @@
167
168 message::Table::Field::FieldType parser_type= attribute->type();
169
170- switch (field_arg->sql_type) {
171- case DRIZZLE_TYPE_LONG:
172- attribute->set_type(message::Table::Field::INTEGER);
173+ attribute->set_type(message::internalFieldTypeToFieldProtoType(field_arg->sql_type));
174+
175+ switch (attribute->type()) {
176+ default: /* Only deal with types that need extra information */
177 break;
178- case DRIZZLE_TYPE_DOUBLE:
179+ case message::Table::Field::DOUBLE:
180 {
181- attribute->set_type(message::Table::Field::DOUBLE);
182-
183- /*
184+ /*
185 * For DOUBLE, we only add a specific scale and precision iff
186 * the fixed decimal point has been specified...
187 */
188@@ -117,26 +117,12 @@
189 }
190 }
191 break;
192- case DRIZZLE_TYPE_NULL :
193- assert(1); /* Not a user definable type */
194- case DRIZZLE_TYPE_TIMESTAMP:
195- attribute->set_type(message::Table::Field::TIMESTAMP);
196- break;
197- case DRIZZLE_TYPE_LONGLONG:
198- attribute->set_type(message::Table::Field::BIGINT);
199- break;
200- case DRIZZLE_TYPE_DATETIME:
201- attribute->set_type(message::Table::Field::DATETIME);
202- break;
203- case DRIZZLE_TYPE_DATE:
204- attribute->set_type(message::Table::Field::DATE);
205- break;
206- case DRIZZLE_TYPE_VARCHAR:
207+ case message::Table::Field::VARCHAR:
208 {
209 message::Table::Field::StringFieldOptions *string_field_options;
210
211 string_field_options= attribute->mutable_string_options();
212- attribute->set_type(message::Table::Field::VARCHAR);
213+
214 if (! use_existing_fields || string_field_options->length()==0)
215 string_field_options->set_length(field_arg->length
216 / field_arg->charset->mbmaxlen);
217@@ -150,24 +136,22 @@
218 }
219 break;
220 }
221- case DRIZZLE_TYPE_DECIMAL:
222+ case message::Table::Field::DECIMAL:
223 {
224 message::Table::Field::NumericFieldOptions *numeric_field_options;
225
226- attribute->set_type(message::Table::Field::DECIMAL);
227 numeric_field_options= attribute->mutable_numeric_options();
228 /* This is magic, I hate magic numbers -Brian */
229 numeric_field_options->set_precision(field_arg->length + ( field_arg->decimals ? -2 : -1));
230 numeric_field_options->set_scale(field_arg->decimals);
231 break;
232 }
233- case DRIZZLE_TYPE_ENUM:
234+ case message::Table::Field::ENUM:
235 {
236 message::Table::Field::SetFieldOptions *set_field_options;
237
238 assert(field_arg->interval);
239
240- attribute->set_type(message::Table::Field::ENUM);
241 set_field_options= attribute->mutable_set_options();
242
243 for (uint32_t pos= 0; pos < field_arg->interval->count; pos++)
244@@ -181,10 +165,8 @@
245 set_field_options->set_collation(field_arg->charset->name);
246 break;
247 }
248- case DRIZZLE_TYPE_BLOB:
249+ case message::Table::Field::BLOB:
250 {
251- attribute->set_type(message::Table::Field::BLOB);
252-
253 message::Table::Field::StringFieldOptions *string_field_options;
254
255 string_field_options= attribute->mutable_string_options();
256@@ -193,8 +175,6 @@
257 }
258
259 break;
260- default:
261- assert(0); /* Tell us, since this shouldn't happend */
262 }
263
264 assert (!use_existing_fields || parser_type == attribute->type());
265@@ -591,3 +571,4 @@
266 } /* rea_create_table */
267
268 } /* namespace drizzled */
269+