C-Loftus commented on code in PR #1185:
URL: https://github.com/apache/iceberg-go/pull/1185#discussion_r3420747423
##########
catalog/hadoop/hadoop.go:
##########
@@ -723,3 +772,26 @@ func (c *Catalog) LoadNamespaceProperties(_
context.Context, ns table.Identifier
func (c *Catalog) UpdateNamespaceProperties(_ context.Context, _
table.Identifier, _ []string, _ iceberg.Properties)
(catalog.PropertiesUpdateSummary, error) {
return catalog.PropertiesUpdateSummary{}, errors.New("hadoop catalog:
UpdateNamespaceProperties not yet implemented")
}
+
+// CheckedMkdirAll is a helper function that checks
+// all subdirectories of a given identifier path exist before creating the
full path.
+// This function is not atomic and may not return ErrNamespaceAlreadyExists if
+// called concurrently with other calls that change the same identifier
+func (c *Catalog) CheckedMkdirAll(id table.Identifier) error {
+ path := c.namespaceToPath(id)
+ for pathIndex := range id {
+ subPath := id[:pathIndex]
+ parentPath := c.namespaceToPath(subPath)
+ if _, err := c.filesystem.Stat(parentPath); err != nil {
Review Comment:
Hmm sorry I am not following this.
> The old CreateNamespace had an explicit if errors.Is(err, fs.ErrNotExist)
{ ... catalog.ErrNoSuchNamespace ... } branch that this PR drops. I'd re-add
that branch in CreateNamespace (alongside the existing ErrExist check)
I believe I currently have a check for `fs.ErrNotExist` which returns an
error with `catalog.ErrNoSuchNamespace` in the result. Could you please clarify
what you would like me to change beyond that?
https://github.com/C-Loftus/iceberg-go/blob/3a7d309d7efd5c216991adae0263e90f76719334/catalog/hadoop/hadoop.go#L629-L640
```go
if err := c.checkedMkdirAll(ns); err != nil {
if errors.Is(err, fs.ErrExist) {
return fmt.Errorf("%w: %s",
catalog.ErrNamespaceAlreadyExists, strings.Join(ns, "."))
}
if errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("%w: parent namespace does not exist
for %s",
catalog.ErrNoSuchNamespace, strings.Join(ns,
"."))
}
return fmt.Errorf("hadoop catalog: failed to create namespace:
%w", err)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
