WIP feat: InMemoryCatalog create table #416
Conversation
WZhuo
force-pushed
the
create_table
branch
2 times, most recently
from
b83be34 to
805973f
Compare
WZhuo
force-pushed
the
create_table
branch
2 times, most recently
from
165ab79 to
a399aee
Compare
| int64_t next_row_id; | ||
|
|
||
| static Result<std::unique_ptr<TableMetadata>> Make( | ||
| const iceberg::Schema& schema, const iceberg::PartitionSpec& spec, |
There was a problem hiding this comment.
Do you think we should rename functions below to GetSchema, GetSnapshot, etc. so we don't need to write iceberg:: prefix here and elsewhere?
| namespace std { | ||
|
|
||
| template <> | ||
| struct formatter<iceberg::Namespace> : std::formatter<std::string> { |
There was a problem hiding this comment.
You don't need to explicitly add these as they are automatically supported by including iceberg/util/formatter.h.
|
|
||
| std::string ToString() const { | ||
| std::ostringstream oss; | ||
| for (size_t i = 0; i < levels.size(); ++i) { |
There was a problem hiding this comment.
formatter_internal.h has a FormatRange to implement this. Using it requires adding a table_identifiler.cc since we cannot export internal header here.
| } | ||
|
|
||
| // Reassign all column ids to ensure consistency | ||
| std::atomic<int32_t> last_column_id = 0; |
There was a problem hiding this comment.
Why do we need to use atomic here?
| const TableMetadata& metadata() const { return metadata_; } | ||
|
|
||
| void SetLocation(std::string_view location) { | ||
| metadata_.location = std::string(location); |
There was a problem hiding this comment.
We might need to follow the java impl to validate the input and then produce a update to the changes list.
We might need to follow the Java impl to validate the input location and then produce a TableUpdate to the changes list.
```java
public Builder setLocation(String newLocation) {
if (location != null && location.equals(newLocation)) {
return this;
}
this.location = newLocation;
changes.add(new MetadataUpdate.SetLocation(newLocation));
return this;
}| constexpr std::string_view kMetadataFolderName = "metadata"; | ||
|
|
||
| // TableMetadata private static methods | ||
| Result<std::shared_ptr<PartitionSpec>> FreshPartitionSpec( |
There was a problem hiding this comment.
I think FreshPartitionSpec and FreshSortOrder should also be called in the SetDefaultPartitionSpec and SetDefaultSortOrder, respectively.
| Result<bool> TableRequirements::IsCreate( | ||
| const std::vector<std::unique_ptr<TableRequirement>>& requirements) { | ||
| bool is_create = std::ranges::any_of(requirements, [](const auto& req) { | ||
| return dynamic_cast<table::AssertDoesNotExist*>(req.get()) != nullptr; |
There was a problem hiding this comment.
Please add an enum type to TableRequirement like TableUpdate::kind() and use it here.
2 participants
No description provided.