zeroshade commented on code in PR #2104:
URL: https://github.com/apache/iceberg-go/pull/2104#discussion_r4257816378
##########
catalog/rest/rest.go:
##########
@@ -1473,36 +1481,56 @@ func (r *Catalog) namespaceToQueryParam(namespace
table.Identifier) string {
return strings.Join(namespace, r.decodedNamespaceSeparator())
}
-func (r *Catalog) splitIdentForPath(ident table.Identifier) (string, string,
error) {
+// identifierPath keeps rawName for JSON request bodies separate from
Review Comment:
**Minor: no test catches a raw name going into the path on most endpoints.**
Using `identPath.rawName` instead of `encodedName` in the paths for
`loadTable`, `DropView`, `fetchTableCreds` or `loadFunction` leaves the whole
suite passing. Those tests use names that encode to themselves. A name
containing `+` would then go out as a literal `+`, which form decoders read as
a space. The body-to-URL test only covers commit, update and `UpdateView`.
The simplest fix is laskoviymishka's optional suggestion: a method `func (p
identifierPath) reqPath(ep endpoint, extra ...string) (string, error)` with the
encoded fields used only inside it. That removes this whole class of mistake
rather than testing for it at every call site. Otherwise, extend the httptest
case to the load/drop/exists/credentials endpoints using `a b+c`.
--
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]
