Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default values #28

Merged
merged 21 commits into from
May 5, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bcbc157
Added default value attribute to Schema::Column
saikiriti93 Apr 18, 2019
eb4053b
Populating default value during UpdateSchema
saikiriti93 Apr 18, 2019
b56aff7
Default values tested in Select
saikiriti93 Apr 18, 2019
9978b50
Added nullptr check for default_value argument
saikiriti93 Apr 18, 2019
87531a2
Default values tested in Scan
saikiriti93 Apr 18, 2019
c0187a3
Fixed format
saikiriti93 Apr 18, 2019
932cf75
Changed back to single Column constructor with a defaultValue argument
saikiriti93 May 2, 2019
ee6f639
Merge branch 'schema_change' into default_values
saikiriti93 May 3, 2019
50c947d
Provide API for setting and clearing default values
saikiriti93 May 3, 2019
3bdca7f
Added a failing test case for populating default values. Single Defau…
saikiriti93 May 3, 2019
22c3041
Modify test case for default value of an old table
saikiriti93 May 4, 2019
404270a
Add new column check in test case
saikiriti93 May 4, 2019
48bcfde
Passing default value test cases
saikiriti93 May 4, 2019
b6631cf
Default value insertion for Scan. Fixed the Scan test case.
saikiriti93 May 4, 2019
66330ac
Added check for version match in Scan
saikiriti93 May 4, 2019
033f0cc
Handle null case in setting a default value
saikiriti93 May 4, 2019
76aebd8
Merge branch 'schema_change' into default_values
saikiriti93 May 4, 2019
8ba1635
Refactoring missing_cols calculation
saikiriti93 May 4, 2019
a9a4565
Updated test comments
saikiriti93 May 5, 2019
4fa1ee1
Changed DefaultValueMap from unordered_map to ConcurrentMap.
saikiriti93 May 5, 2019
9384ea0
Refactoring for default value filling
saikiriti93 May 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 54 additions & 10 deletions src/include/catalog/schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,48 @@ class Schema {
* @param oid internal unique identifier for this column
*/
Column(std::string name, const type::TypeId type, const bool nullable, const col_oid_t oid)
: name_(std::move(name)),
type_(type),
attr_size_(type::TypeUtil::GetTypeSize(type_)),
nullable_(nullable),
inlined_(true),
oid_(oid),
default_(nullptr) {
validateColumn();
}

/**
* Instantiates a Column object, primary to be used for building a Schema object
* NOTE: Overloaded to avoid changing all Column initializations in the code
* @param name column name
* @param type SQL type for this column
* @param nullable true if the column is nullable, false otherwise
* @param oid internal unique identifier for this column
* @param default_value default value for this column
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
*/
Column(std::string name, const type::TypeId type, const bool nullable, const col_oid_t oid, byte *default_value)
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
: name_(std::move(name)),
type_(type),
attr_size_(type::TypeUtil::GetTypeSize(type_)),
nullable_(nullable),
inlined_(true),
oid_(oid) {
if (attr_size_ == VARLEN_COLUMN) {
// this is a varlen attribute
// attr_size_ is actual size + high bit via GetTypeSize
inlined_ = false;
// ASSUMPTION: The default_value passed in is of size attr_size_
// Copy the passed in default value
if (default_value != nullptr) {
default_ = new byte[attr_size_];
std::memcpy(default_, default_value, attr_size_);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
} else {
default_ = default_value;
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
}
TERRIER_ASSERT(
attr_size_ == 1 || attr_size_ == 2 || attr_size_ == 4 || attr_size_ == 8 || attr_size_ == VARLEN_COLUMN,
"Attribute size must be 1, 2, 4, 8 or VARLEN_COLUMN bytes.");
TERRIER_ASSERT(type_ != type::TypeId::INVALID, "Attribute type cannot be INVALID.");
validateColumn();
}

/**
* Free the memory allocated to default_ in the destructor
*/
~Column() = default;

/**
* @return column name
*/
Expand All @@ -74,16 +100,34 @@ class Schema {
* @return internal unique identifier for this column
*/
col_oid_t GetOid() const { return oid_; }
/**
* @return default value for this column
*/
byte *GetDefault() const { return default_; }

private:
/**
* Validate the attr_size_ and type of the column
*/
void validateColumn() {
if (attr_size_ == VARLEN_COLUMN) {
// this is a varlen attribute
// attr_size_ is actual size + high bit via GetTypeSize
inlined_ = false;
}
TERRIER_ASSERT(
attr_size_ == 1 || attr_size_ == 2 || attr_size_ == 4 || attr_size_ == 8 || attr_size_ == VARLEN_COLUMN,
"Attribute size must be 1, 2, 4, 8 or VARLEN_COLUMN bytes.");
TERRIER_ASSERT(type_ != type::TypeId::INVALID, "Attribute type cannot be INVALID.");
}

const std::string name_;
const type::TypeId type_;
uint8_t attr_size_;
const bool nullable_;
bool inlined_;
const col_oid_t oid_;
// TODO(Matt): default value would go here
// Value default_;
byte *default_;
};

/**
Expand Down
3 changes: 2 additions & 1 deletion src/include/storage/sql_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class SqlTable {
BlockLayout layout;
ColumnMap column_map;
InverseColumnMap inverse_column_map;
// TODO(John): Add 'default_value_map' (dynamic) for col_oid->default_val
// TODO(Sai): Do we really need a defaultValueMap for every version? One for SqlTable should work?
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
DefaultValueMap default_value_map;
};

/**
Expand Down
1 change: 1 addition & 0 deletions src/include/storage/storage_defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ using ColumnMap = std::unordered_map<catalog::col_oid_t, col_id_t>;
*/
using InverseColumnMap = std::unordered_map<col_id_t, catalog::col_oid_t>;
using ProjectionMap = std::unordered_map<catalog::col_oid_t, uint16_t>;
using DefaultValueMap = std::unordered_map<catalog::col_oid_t, std::pair<byte *, uint8_t>>;
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Denote whether a record modifies the logical delete column, used when DataTable inspects deltas
Expand Down
58 changes: 56 additions & 2 deletions src/storage/sql_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ void SqlTable::UpdateSchema(const catalog::Schema &schema) {
}
}

// Populate the default value map
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
DefaultValueMap default_value_map;
for (const auto &column : schema.GetColumns()) {
byte *default_value = column.GetDefault();
uint8_t attr_size = column.GetAttrSize();
if (default_value != nullptr) {
default_value_map[column.GetOid()] = {default_value, attr_size};
}
}

BlockLayout layout = storage::BlockLayout(attr_sizes);

auto dt = new DataTable(block_store_, layout, schema.GetVersion());
Expand All @@ -117,7 +127,7 @@ void SqlTable::UpdateSchema(const catalog::Schema &schema) {
// for this allocation is in the destructor for SqlTable. clang-analyzer-cplusplus.NewDeleteLeaks identifies this
// as a potential leak and throws an error incorrectly.
// NOLINTNEXTLINE
tables_.Insert(schema.GetVersion(), {dt, layout, col_map, inv_col_map});
tables_.Insert(schema.GetVersion(), {dt, layout, col_map, inv_col_map, default_value_map});
}

bool SqlTable::Select(transaction::TransactionContext *const txn, const TupleSlot slot, ProjectedRow *const out_buffer,
Expand All @@ -136,6 +146,7 @@ bool SqlTable::Select(transaction::TransactionContext *const txn, const TupleSlo
}

auto old_dt_version = tables_.Find(old_version_num)->second;
auto curr_dt_version = tables_.Find(version_num)->second;

// The slot version is not the same as the version_num
col_id_t original_column_ids[out_buffer->NumColumns()];
Expand All @@ -145,7 +156,20 @@ bool SqlTable::Select(transaction::TransactionContext *const txn, const TupleSlo
bool result = old_dt_version.data_table->Select(txn, slot, out_buffer);
std::memcpy(out_buffer->ColumnIds(), original_column_ids, sizeof(col_id_t) * out_buffer->NumColumns());

// TODO(Yashwanth): handle default values
// Get the DefaultValueMap visible to this transaction
DefaultValueMap default_val_map = curr_dt_version.default_value_map;

// Populate default values into the empty columns of the ProjectedRow (if any)
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
STORAGE_LOG_DEBUG("Loading default values")
for (uint16_t i = 0; i < out_buffer->NumColumns(); i++) {
catalog::col_oid_t col_oid = curr_dt_version.inverse_column_map.at(out_buffer->ColumnIds()[i]);
// Fill in the default value if the entry is not null and the col_oid is in the map
if (default_val_map.count(col_oid) > 0 && out_buffer->IsNull(i)) {
auto dv_pair = default_val_map.at(col_oid);
storage::StorageUtil::CopyWithNullCheck(dv_pair.first, out_buffer, dv_pair.second, i);
}
}

return result;
}

Expand Down Expand Up @@ -287,6 +311,36 @@ void SqlTable::Scan(transaction::TransactionContext *const txn, SqlTable::SlotIt
uint32_t filled = out_buffer->NumTuples();
std::memcpy(out_buffer->ColumnIds(), original_column_ids, sizeof(col_id_t) * out_buffer->NumColumns());
out_buffer->SetNumTuples(filled);

// Populate the default values
saikiriti93 marked this conversation as resolved.
Show resolved Hide resolved
if (filled > 0) {
// Since we are populating multiple tuples, calculate the columns matching with default value map
auto curr_dt_version = tables_.Find(version_num)->second;
DefaultValueMap default_val_map = curr_dt_version.default_value_map;
// Find out the columns which contain default values and their projection list index
std::vector<std::pair<catalog::col_oid_t, uint16_t>> matching_cols;
for (uint16_t i = 0; i < out_buffer->NumColumns(); i++) {
catalog::col_oid_t col_oid = curr_dt_version.inverse_column_map.at(out_buffer->ColumnIds()[i]);
// Fill in the default value if the entry is not null and the col_oid is in the map
if (default_val_map.count(col_oid) > 0) {
matching_cols.emplace_back(col_oid, i);
}
}

// Fill in the matching default values for each of the rows
// Since each row could have different columns unfilled, using RowView to iterate over ProjectedColumns
for (uint32_t row_idx = 0; row_idx < filled; row_idx++) {
ProjectedColumns::RowView row = out_buffer->InterpretAsRow(curr_dt_version.layout, row_idx);
for (auto col_oid_idx_pair : matching_cols) {
auto col_oid = col_oid_idx_pair.first;
auto idx = col_oid_idx_pair.second;
if (row.IsNull(idx)) {
auto dv_pair = default_val_map.at(col_oid);
storage::StorageUtil::CopyWithNullCheck(dv_pair.first, &row, dv_pair.second, idx);
}
}
}
}
}

std::vector<col_id_t> SqlTable::ColIdsForOids(const std::vector<catalog::col_oid_t> &col_oids,
Expand Down
53 changes: 36 additions & 17 deletions test/storage/sql_table_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ class SqlTableTestRW {
}

void AddColumn(transaction::TransactionContext *txn, std::string name, type::TypeId type, bool nullable,
catalog::col_oid_t oid) {
catalog::col_oid_t oid, byte *default_value) {
// update columns, schema and layout
cols_.emplace_back(name, type, nullable, oid);
cols_.emplace_back(name, type, nullable, oid, default_value);
delete schema_;
delete layout_;
schema_ = new catalog::Schema(cols_, next_version_++);
Expand Down Expand Up @@ -205,6 +205,17 @@ class SqlTableTestRW {
}
}

/**
* Convert an integer to byte array
* @param n an integer
* @return byte array
*/
byte *intToByteArray(int n) {
auto byteArray = new byte[sizeof(n)];
memcpy(byteArray, reinterpret_cast<char *>(&n), sizeof(n));
return byteArray;
}

public:
// This is a public field that transactions can set and read.
// The purpose is to record the version for each transaction. In reality this information should be retrieved from
Expand Down Expand Up @@ -275,7 +286,10 @@ TEST_F(SqlTableTests, SelectTest) {

// manually set the version of the transaction to be 1
table.version_ = storage::layout_version_t(1);
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2));
int default_val = 42;
// Add a new column with a default value
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2),
table.intToByteArray(default_val));

id = table.GetIntColInRow(txn, catalog::col_oid_t(0), row1_slot);
EXPECT_EQ(100, id);
Expand All @@ -287,8 +301,11 @@ TEST_F(SqlTableTests, SelectTest) {
datname = table.GetIntColInRow(txn, catalog::col_oid_t(1), row2_slot);
EXPECT_EQ(10001, datname);

// The new_column should return the default_value for the old version slots
uint32_t new_col = table.GetIntColInRow(txn, catalog::col_oid_t(2), row1_slot);
EXPECT_EQ(12345, new_col);
EXPECT_EQ(default_val, new_col);
new_col = table.GetIntColInRow(txn, catalog::col_oid_t(2), row2_slot);
EXPECT_EQ(default_val, new_col);

txn_manager_.Commit(txn, TestCallbacks::EmptyCallback, nullptr);
delete txn;
Expand All @@ -313,7 +330,7 @@ TEST_F(SqlTableTests, InsertTest) {

// manually set the version of the transaction to be 1
table.version_ = storage::layout_version_t(1);
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2));
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2), nullptr);

// insert (300, 10002, null)
table.StartInsertRow();
Expand Down Expand Up @@ -368,7 +385,7 @@ TEST_F(SqlTableTests, DeleteTest) {

// manually set the version of the transaction to be 1
table.version_ = storage::layout_version_t(1);
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2));
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2), nullptr);

// insert (300, 10002, null)
table.StartInsertRow();
Expand Down Expand Up @@ -418,7 +435,7 @@ TEST_F(SqlTableTests, UpdateTest) {

// manually set the version of the transaction to be 1
table.version_ = storage::layout_version_t(1);
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2));
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2), nullptr);

// insert (300, 10002, null)
table.StartInsertRow();
Expand Down Expand Up @@ -489,6 +506,7 @@ TEST_F(SqlTableTests, UpdateTest) {
TEST_F(SqlTableTests, ScanTest) {
std::map<uint32_t, uint32_t> datname_map;
std::map<uint32_t, uint32_t> new_col_map;
uint32_t new_col_default_value = 1729;
jrolli marked this conversation as resolved.
Show resolved Hide resolved
std::map<uint32_t, bool> seen_map;

datname_map[100] = 10000;
Expand Down Expand Up @@ -526,9 +544,10 @@ TEST_F(SqlTableTests, ScanTest) {

// manually set the version of the transaction to be 1
table.version_ = storage::layout_version_t(1);
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2));
table.AddColumn(txn, "new_col", type::TypeId::INTEGER, true, catalog::col_oid_t(2),
table.intToByteArray(new_col_default_value));

// insert (300, 10002, null)
// insert (300, 10002, 1729)
table.StartInsertRow();
table.SetIntColInRow(catalog::col_oid_t(0), 300);
table.SetIntColInRow(catalog::col_oid_t(1), datname_map[300]);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -570,10 +589,10 @@ TEST_F(SqlTableTests, ScanTest) {
uint32_t datname = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(datname, datname_map[id]);
value = row1.AccessWithNullCheck(pc_pair.second.at(catalog::col_oid_t(2)));
uint32_t new_col = *reinterpret_cast<uint32_t *>(value);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
if (id != 400) {
EXPECT_EQ(value, nullptr);
EXPECT_EQ(new_col, new_col_default_value);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
} else {
uint32_t new_col = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(new_col, new_col_map[id]);
}

Expand All @@ -589,10 +608,10 @@ TEST_F(SqlTableTests, ScanTest) {
datname = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(datname, datname_map[id]);
value = row2.AccessWithNullCheck(pc_pair.second.at(catalog::col_oid_t(2)));
new_col = *reinterpret_cast<uint32_t *>(value);
if (id != 400) {
EXPECT_EQ(value, nullptr);
EXPECT_EQ(new_col, new_col_default_value);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
} else {
uint32_t new_col = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(new_col, new_col_map[id]);
}

Expand All @@ -613,10 +632,10 @@ TEST_F(SqlTableTests, ScanTest) {
datname = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(datname, datname_map[id]);
value = row3.AccessWithNullCheck(pc_pair.second.at(catalog::col_oid_t(2)));
new_col = *reinterpret_cast<uint32_t *>(value);
if (id != 400) {
EXPECT_EQ(value, nullptr);
EXPECT_EQ(new_col, new_col_default_value);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
} else {
uint32_t new_col = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(new_col, new_col_map[id]);
}

Expand All @@ -632,10 +651,10 @@ TEST_F(SqlTableTests, ScanTest) {
datname = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(datname, datname_map[id]);
value = row4.AccessWithNullCheck(pc_pair.second.at(catalog::col_oid_t(2)));
new_col = *reinterpret_cast<uint32_t *>(value);
if (id != 400) {
EXPECT_EQ(value, nullptr);
EXPECT_EQ(new_col, new_col_default_value);
jrolli marked this conversation as resolved.
Show resolved Hide resolved
} else {
uint32_t new_col = *reinterpret_cast<uint32_t *>(value);
EXPECT_EQ(new_col, new_col_map[id]);
}

Expand Down