You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "viditochani (via GitHub)" <gi...@apache.org> on 2026/05/07 05:15:43 UTC

[PR] Fix renameTable to return 204 instead of 200 [polaris]

viditochani opened a new pull request, #4383:
URL: https://github.com/apache/polaris/pull/4383

   `renameTable` returns HTTP 200 with body text `"NO_CONTENT"` instead of HTTP 204 as per the [IRC spec](https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L1325).
   
   ### Testing
   
   Manually verified by sending a rename table request to a local Polaris instance.
   
   ### Checklist
   - [x] 🛡️ Don't disclose security issues! (contact security@apache.org)
   - [x] 🔗 Clearly explained why the changes are needed, or linked related issues
   - [x] 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
   - [ ] 💡 Added comments for complex logic
   - [ ] 🧾 Updated `CHANGELOG.md` (if needed)
   - [ ] 📚 Updated documentation in `site/content/in-dev/unreleased` (if needed)


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain merged PR #4383:
URL: https://github.com/apache/polaris/pull/4383


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "viditochani (via GitHub)" <gi...@apache.org>.

viditochani commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4401081662

   @flyrain @dimas-b Added a CHANGELOG entry and an integration test to prevent regression. @dimas-b thanks for starting the dev thread.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on code in PR #4383:
URL: https://github.com/apache/polaris/pull/4383#discussion_r3203072975


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java:
##########
@@ -439,7 +439,7 @@ public Response renameTable(
         prefix,
         catalog -> {
           catalog.renameTable(renameTableRequest);
-          return Response.ok(Response.Status.NO_CONTENT).build();
+          return Response.status(Response.Status.NO_CONTENT).build();

Review Comment:
   Correct fix — `Response.ok(...)` always sets 200 and treats the arg as entity body. `Response.status(NO_CONTENT)` is the right builder for 204 with empty body.



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.

dimas-b commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4401863516

   @flyrain : cf. #4387


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain closed pull request #4383: Fix renameTable to return 204 instead of 200
URL: https://github.com/apache/polaris/pull/4383


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4402478736

   Retriggered the CIs. 


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.

adnanhemani commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4395545958

   While I think this change is correct (and correct an error), I believe all API changes need a VOTE thread? @dimas-b thoughts?


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4408669052

   Thanks @viditochani for the fix! Thanks everyone for the review.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4401754848

   Looks like the Zizmor security audit is failing with three warnings about action hash pins missing/mismatched version comments at commit 68bde559dea0. I'd suggest to rebase against the latest main branch.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.

dimas-b commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4400125918

   Technically this is a breaking change at the API level, IMHO.
   
   However, given that Polaris implements the spec incorrectly in this case and the Iceberg spec [change](https://github.com/apache/iceberg/pull/7229) that altered this response code predates Polaris, I think it the fix is good.
   
   Thanks for noticing it, @viditochani !
   
   We should definitely discuss it on `dev` for more awareness in the community.
   


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.

nandorKollar commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4398357724

   > While I think this change is correct (and is correcting an error), I believe all API changes need a VOTE thread? @dimas-b thoughts?
   
   Isn't this also a possibly breaking change? Although the current implementation clearly doesn't conform with IRC spec, we change here the actual response status here.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.

dimas-b commented on PR #4383:
URL: https://github.com/apache/polaris/pull/4383#issuecomment-4400220980

   `dev` discussion: https://lists.apache.org/thread/oxf6cvb99v76rjn8zq77fsytggrpbxn3
   
   @viditochani : Could you add a note about this to `CHANGELOG.md`? I believe the `Fixes` section would be appropriate as we're aligning with the spec.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix renameTable to return 204 instead of 200 [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.

flyrain commented on code in PR #4383:
URL: https://github.com/apache/polaris/pull/4383#discussion_r3203072975


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java:
##########
@@ -439,7 +439,7 @@ public Response renameTable(
         prefix,
         catalog -> {
           catalog.renameTable(renameTableRequest);
-          return Response.ok(Response.Status.NO_CONTENT).build();
+          return Response.status(Response.Status.NO_CONTENT).build();

Review Comment:
   Correct fix — `Response.ok(...)` always sets 200 and treats the arg as entity body. `Response.status(NO_CONTENT)` is the right builder for 204 with empty body.



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org