Skip to content

Commit a5424d3

Browse files
ColinLeeoColinLeeo
authored
Same name (case insensitive) in tablet / table. (apache#490)
* tmp code. * add duplicate column name check. * fix memory leak. * fix memory leak. * fix memory leak.
1 parent 1856572 commit a5424d3

File tree

8 files changed

+97
-16
lines changed

8 files changed

+97
-16
lines changed

‎cpp/src/common/schema.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ class TableSchema {
331331
}
332332
}
333333

334+
size_t get_column_pos_index_num() const {
335+
return column_pos_index_.size();
336+
}
337+
334338
void update(ChunkGroupMeta *chunk_group_meta) {
335339
for (auto iter = chunk_group_meta->chunk_meta_list_.begin();
336340
iter != chunk_group_meta->chunk_meta_list_.end(); iter++) {

‎cpp/src/common/tablet.cc

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ int Tablet::init() {
3636
std::pair<std::map<std::string, int>::iterator, bool> ins_res;
3737
for (size_t c = 0; c < schema_count; c++) {
3838
ins_res = schema_map_.insert(
39-
std::make_pair(schema_vec_->at(c).measurement_name_, c));
39+
std::make_pair(to_lower(schema_vec_->at(c).measurement_name_), c));
4040
if (!ins_res.second) {
41-
ASSERT(false);
4241
// maybe dup measurement_name
4342
return E_INVALID_ARG;
4443
}
@@ -131,6 +130,9 @@ void Tablet::destroy() {
131130
}
132131

133132
int Tablet::add_timestamp(uint32_t row_index, int64_t timestamp) {
133+
if (err_code_ != E_OK) {
134+
return err_code_;
135+
}
134136
ASSERT(timestamps_ != NULL);
135137
if (UNLIKELY(row_index >= static_cast<uint32_t>(max_row_num_))) {
136138
ASSERT(false);
@@ -223,6 +225,9 @@ void Tablet::process_val(uint32_t row_index, uint32_t schema_index, T val) {
223225

224226
template <typename T>
225227
int Tablet::add_value(uint32_t row_index, uint32_t schema_index, T val) {
228+
if (err_code_ != E_OK) {
229+
return err_code_;
230+
}
226231
int ret = common::E_OK;
227232
if (UNLIKELY(schema_index >= schema_vec_->size())) {
228233
ASSERT(false);
@@ -250,6 +255,9 @@ int Tablet::add_value(uint32_t row_index, uint32_t schema_index, T val) {
250255
template <>
251256
int Tablet::add_value(uint32_t row_index, uint32_t schema_index,
252257
common::String val) {
258+
if (err_code_ != E_OK) {
259+
return err_code_;
260+
}
253261
int ret = common::E_OK;
254262
if (UNLIKELY(schema_index >= schema_vec_->size())) {
255263
ASSERT(false);
@@ -269,9 +277,11 @@ template <typename T>
269277
int Tablet::add_value(uint32_t row_index, const std::string &measurement_name,
270278
T val) {
271279
int ret = common::E_OK;
280+
if (err_code_ != E_OK) {
281+
return err_code_;
282+
}
272283
SchemaMapIterator find_iter = schema_map_.find(measurement_name);
273284
if (LIKELY(find_iter == schema_map_.end())) {
274-
ASSERT(false);
275285
ret = E_INVALID_ARG;
276286
} else {
277287
ret = add_value(row_index, find_iter->second, val);

‎cpp/src/common/tablet.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ class Tablet {
5757

5858
public:
5959
static const uint32_t DEFAULT_MAX_ROWS = 1024;
60+
int err_code_ = common::E_OK;
6061

6162
public:
6263
Tablet(const std::string &device_id,
@@ -75,7 +76,7 @@ class Tablet {
7576
ASSERT(false);
7677
max_row_num_ = DEFAULT_MAX_ROWS;
7778
}
78-
init();
79+
err_code_ = init();
7980
}
8081

8182
Tablet(const std::string &device_id,
@@ -106,7 +107,7 @@ class Tablet {
106107
return MeasurementSchema(name, type);
107108
});
108109
schema_vec_ = std::make_shared<std::vector<MeasurementSchema>>(measurement_vec);
109-
init();
110+
err_code_ = init();
110111
}
111112

112113
Tablet(const std::string &insert_target_name,
@@ -127,7 +128,7 @@ class Tablet {
127128
common::get_default_compressor()));
128129
}
129130
set_column_categories(column_categories);
130-
init();
131+
err_code_ = init();
131132
}
132133

133134
/**
@@ -150,10 +151,10 @@ class Tablet {
150151
schema_vec_ = std::make_shared<std::vector<MeasurementSchema>>();
151152
for (size_t i = 0; i < column_names.size(); i++) {
152153
schema_vec_->emplace_back(
153-
MeasurementSchema(column_names[i], data_types[i], common::get_value_encoder(data_types[i]),
154-
common::get_default_compressor()));
154+
column_names[i], data_types[i], common::get_value_encoder(data_types[i]),
155+
common::get_default_compressor());
155156
}
156-
init();
157+
err_code_ = init();
157158
}
158159

159160
~Tablet() { destroy(); }

‎cpp/src/cwrapper/tsfile_cwrapper.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ Tablet tablet_new(char **column_name_list, TSDataType *data_types,
174174
std::vector<std::string> measurement_list;
175175
std::vector<common::TSDataType> data_type_list;
176176
for (uint32_t i = 0; i < column_num; i++) {
177-
measurement_list.emplace_back(column_name_list[i]);
177+
measurement_list.emplace_back(storage::to_lower(column_name_list[i]));
178178
data_type_list.push_back(
179179
static_cast<common::TSDataType>(*(data_types + i)));
180180
}
@@ -196,7 +196,7 @@ ERRNO tablet_add_timestamp(Tablet tablet, uint32_t row_index,
196196
const char *column_name, \
197197
const type value) { \
198198
return static_cast<storage::Tablet *>(tablet)->add_value( \
199-
row_index, column_name, value); \
199+
row_index, storage::to_lower(column_name), value); \
200200
}
201201
TABLET_ADD_VALUE_BY_NAME_DEF(int32_t);
202202
TABLET_ADD_VALUE_BY_NAME_DEF(int64_t);
@@ -208,7 +208,7 @@ ERRNO tablet_add_value_by_name_string(Tablet tablet, uint32_t row_index,
208208
const char *column_name,
209209
const char *value) {
210210
return static_cast<storage::Tablet *>(tablet)->add_value(
211-
row_index, column_name, common::String(value));
211+
row_index, storage::to_lower(column_name), common::String(value));
212212
}
213213

214214
#define TABLE_ADD_VALUE_BY_INDEX_DEF(type) \
@@ -688,7 +688,6 @@ ERRNO _tsfile_writer_flush(TsFileWriter writer) {
688688
return w->flush();
689689
}
690690

691-
692691
ResultSet _tsfile_reader_query_device(TsFileReader reader,
693692
const char *device_name,
694693
char **sensor_name, uint32_t sensor_num,

‎cpp/src/writer/tsfile_table_writer.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ int storage::TsFileTableWriter::register_table(const std::shared_ptr<TableSchema
2929
}
3030

3131
int storage::TsFileTableWriter::write_table(storage::Tablet& tablet) const {
32+
// DIRTY CODE...
33+
if (common::E_OK != error_number) {
34+
return error_number;
35+
}
3236
if (tablet.get_table_name().empty()) {
3337
tablet.set_table_name(exclusive_table_name_);
3438
} else if (!exclusive_table_name_.empty() && tablet.get_table_name() != exclusive_table_name_) {

‎cpp/src/writer/tsfile_table_writer.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class TsFileTableWriter {
6161
// Perform a deep copy. The source TableSchema object may be
6262
// stack/heap-allocated.
6363
auto table_schema_ptr = std::make_shared<TableSchema>(*table_schema);
64-
tsfile_writer_->register_table(table_schema_ptr);
64+
error_number = tsfile_writer_->register_table(table_schema_ptr);
6565
exclusive_table_name_ = table_schema->get_table_name();
6666
common::g_config_value_.chunk_group_size_threshold_ = memory_threshold;
6767
}
@@ -106,6 +106,10 @@ class TsFileTableWriter {
106106
// if this TsFile only contains one table, this will be its name, otherwise,
107107
// it will be an empty string
108108
std::string exclusive_table_name_;
109+
110+
// Some errors may not be conveyed during the construction phase, so it's
111+
// necessary to maintain an internal error code.
112+
int error_number = common::E_OK;
109113
};
110114

111115
} // namespace storage

‎cpp/src/writer/tsfile_writer.cc

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,27 @@ void TsFileWriter::set_generate_table_schema(bool generate_table_schema) {
117117
int TsFileWriter::register_table(
118118
const std::shared_ptr<TableSchema> &table_schema) {
119119
if (!table_schema) return E_INVALID_ARG;
120+
121+
// Empty table name or column name is not allowed.
122+
if (table_schema->get_table_name().empty()) {
123+
return E_INVALID_ARG;
124+
}
125+
for (const auto &name : table_schema->get_measurement_names()) {
126+
if (name.empty()) {
127+
return E_INVALID_ARG;
128+
}
129+
}
130+
131+
// Because it is not possible to return an error code for duplicate name
132+
// checks during the construction phase of TabletSchema, the duplicate name
133+
// check has been moved to the table registration stage.
134+
135+
// TODO: Add Debug INFO if ErrorCode is not enough to describe problems.
136+
if (table_schema->get_column_pos_index_num() !=
137+
table_schema->get_measurement_names().size()) {
138+
return E_INVALID_ARG;
139+
}
140+
120141
if (io_writer_->get_schema()->table_schema_map_.find(
121142
table_schema->get_table_name()) !=
122143
io_writer_->get_schema()->table_schema_map_.end()) {
@@ -671,7 +692,7 @@ int TsFileWriter::write_tablet_aligned(const Tablet &tablet) {
671692
continue;
672693
}
673694
if (RET_FAIL(value_write_column(value_chunk_writer, tablet, c, 0,
674-
tablet.get_cur_row_size()))) {
695+
tablet.get_cur_row_size()))) {
675696
return ret;
676697
}
677698
}
@@ -764,7 +785,7 @@ int TsFileWriter::write_table(Tablet &tablet) {
764785
continue;
765786
}
766787
if (RET_FAIL(write_column(chunk_writer, tablet, c, start_idx,
767-
device_id_end_index_pair.second))) {
788+
device_id_end_index_pair.second))) {
768789
return ret;
769790
}
770791
}

‎cpp/test/writer/table_view/tsfile_writer_table_test.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,4 +351,42 @@ TEST_F(TsFileWriterTableTest, WriteAndReadSimple) {
351351

352352
reader.close();
353353
delete table_schema;
354+
}
355+
356+
TEST_F(TsFileWriterTableTest, DuplicateColumnName) {
357+
std::vector<MeasurementSchema*> measurement_schemas;
358+
std::vector<ColumnCategory> column_categories;
359+
measurement_schemas.resize(3);
360+
measurement_schemas[0] = new MeasurementSchema("device", STRING);
361+
column_categories.emplace_back(ColumnCategory::TAG);
362+
measurement_schemas[1] = new MeasurementSchema("Device", STRING);
363+
column_categories.emplace_back(ColumnCategory::TAG);
364+
measurement_schemas[2] = new MeasurementSchema("value", DOUBLE);
365+
column_categories.emplace_back(ColumnCategory::FIELD);
366+
TableSchema* table_schema =
367+
new TableSchema("test_table", measurement_schemas, column_categories);
368+
auto tsfile_table_writer =
369+
std::make_shared<TsFileTableWriter>(&write_file_, table_schema);
370+
Tablet tablet = Tablet(table_schema->get_measurement_names(),
371+
table_schema->get_data_types());
372+
tablet.set_table_name("test_table");
373+
ASSERT_EQ(E_INVALID_ARG, tablet.add_timestamp(0, 10));
374+
ASSERT_EQ(E_INVALID_ARG, tablet.add_value(1, 1, 10));
375+
ASSERT_EQ(E_INVALID_ARG, tablet.add_value(1, "test", 10));
376+
std::vector<MeasurementSchema> measurement_schemas2;
377+
for (int i = 0; i < 2; i++) {
378+
measurement_schemas2.push_back(*measurement_schemas[i]);
379+
}
380+
Tablet tablet1 = Tablet(
381+
"test_table",
382+
std::make_shared<std::vector<MeasurementSchema>>(measurement_schemas2));
383+
tablet1.set_table_name("test_table");
384+
ASSERT_EQ(E_INVALID_ARG, tablet1.add_timestamp(0, 10));
385+
ASSERT_EQ(E_INVALID_ARG, tablet1.add_value(1, 1, 10));
386+
ASSERT_EQ(E_INVALID_ARG, tablet1.add_value(1, "test", 10));
387+
388+
ASSERT_EQ(E_INVALID_ARG, tsfile_table_writer->write_table(tablet));
389+
ASSERT_EQ(E_INVALID_ARG, tsfile_table_writer->register_table(
390+
std::make_shared<TableSchema>(*table_schema)));
391+
delete table_schema;
354392
}

0 commit comments

Comments
 (0)