Fokko commented on code in PR #97:
URL: https://github.com/apache/iceberg-rust/pull/97#discussion_r1398974623
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -586,16 +648,46 @@ mod _serde {
pub(super) metadata: TableMetadata,
pub(super) config: Option<HashMap<String, String>>,
}
+
+ #[derive(Debug, Serialize, Deserialize)]
+ #[serde(rename_all = "kebab-case")]
+ pub(super) struct CreateTableRequest {
+ pub(super) name: String,
+ pub(super) location: Option<String>,
+ pub(super) schema: Schema,
+ pub(super) partition_spec: Option<PartitionSpec>,
+ pub(super) write_order: Option<SortOrder>,
+ pub(super) stage_create: bool,
Review Comment:
https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.py#L764
```suggestion
pub(super) stage_create: Option<bool>,
```
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1017,31 +1109,31 @@ mod tests {
.with_summary(Summary {
operation: Operation::Append,
other: HashMap::from_iter([
- ("spark.app.id",
"local-1646787004168"),
- ("added-data-files", "1"),
- ("added-records", "1"),
- ("added-files-size", "697"),
- ("changed-partition-count", "1"),
- ("total-records", "1"),
- ("total-files-size", "697"),
- ("total-data-files", "1"),
- ("total-delete-files", "0"),
- ("total-position-deletes", "0"),
- ("total-equality-deletes", "0")
- ].iter().map(|p|(p.0.to_string(), p.1.to_string())))
+ ("spark.app.id", "local-1646787004168"),
Review Comment:
Nit: shouldn't this be caught by the linter? Same for the trailing comma's
below.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -18,10 +18,12 @@
/*!
Defines the [table metadata](https://iceberg.apache.org/spec/#table-metadata).
The main struct here is [TableMetadataV2] which defines the data for a table.
-*/
+ */
Review Comment:
```suggestion
*/
```
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -446,6 +490,24 @@ impl RestCatalog {
Ok(())
}
+
+ fn load_file_io(
+ &self,
+ metadata_location: Option<&str>,
+ extra_config: Option<HashMap<String, String>>,
+ ) -> Result<FileIO> {
+ let mut props = self.config.props.clone();
+ if let Some(config) = extra_config {
+ props.extend(config);
+ }
+
+ let file_io = match
self.config.warehouse.as_deref().or(metadata_location) {
+ Some(url) => FileIO::from_path(url)?.with_props(props).build()?,
+ None => FileIOBuilder::new("s3").with_props(props).build()?,
Review Comment:
Do we want to fallback to S3 by default? I would not expect to fallback to
S3, and this might lead to unexpected error messages.
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1092,4 +1184,375 @@ mod tests {
config_mock.assert_async().await;
rename_table_mock.assert_async().await;
}
+
+ #[tokio::test]
+ async fn test_create_table() {
+ let mut server = Server::new_async().await;
+
+ let config_mock = create_config_mock(&mut server).await;
+
+ let create_table_mock = server
+ .mock("POST", "/v1/namespaces/ns1/tables")
+ .with_status(200)
+ .with_body_from_file(format!(
+ "{}/testdata/{}",
+ env!("CARGO_MANIFEST_DIR"),
+ "create_table_response.json"
+ ))
+ .create_async()
+ .await;
+
+ let catalog =
RestCatalog::new(RestCatalogConfig::builder().uri(server.url()).build())
+ .await
+ .unwrap();
+
+ let table_creation = TableCreation::builder()
+ .name("test1".to_string())
+ .schema(
+ Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "foo",
Type::Primitive(PrimitiveType::String))
+ .into(),
+ NestedField::required(2, "bar",
Type::Primitive(PrimitiveType::Int)).into(),
+ NestedField::optional(3, "baz",
Type::Primitive(PrimitiveType::Boolean))
+ .into(),
+ ])
+ .with_schema_id(1)
+ .with_identifier_field_ids(vec![2])
+ .build()
+ .unwrap(),
+ )
+ .properties(HashMap::from([("owner".to_string(),
"testx".to_string())]))
+ .partition_spec(
+ PartitionSpec::builder()
+ .with_fields(vec