Skip to content

Conversation

@rambleraptor
Copy link
Contributor

Part of #818

This adds create_view to the REST Catalog. As part of this, it also defines the View and ViewMetadata class.

Rationale for this change

PyIceberg's REST Catalog doesn't support create_view. Furthermore, PyIceberg doesn't support views at all. This is the first part in getting views supported.

Are these changes tested?

Unit tests included

Are there any user-facing changes?

Added Iceberg REST Catalog support for create_view

@jayceslesar
Copy link
Contributor

Is it worth setting integration tests in rest integration tests?

@rambleraptor
Copy link
Contributor Author

I'm a little unclear what Catalog the REST integration tests run against, so I can't say if there's been enough implemented to actually have a valid REST Catalog test.

The integration tests are missing most of the methods that have been implemented, so my reaction is that we're still missing pieces for an integration test.

@rambleraptor rambleraptor requested a review from jayceslesar June 29, 2025 00:03
@rambleraptor
Copy link
Contributor Author

@Fokko @kevinjqliu mind taking a look when you can? thanks!

Copy link
Collaborator

@sungwy sungwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this up @rambleraptor

I've taken a stab at reviewing the PR. I think adding an integration test is a must-have for introducing a new API. I think a great way to verify that the created view works as expected is to add a view as a spark dialect sql view and read the view table through a spark session, similar to how we validate that PyIceberg writes are reflected correctly even when the table is read through a Spark session accessing the same REST Catalog (example). WDYT?

@rambleraptor
Copy link
Contributor Author

@sungwy @jayceslesar please take a look when you can!

@rambleraptor rambleraptor requested a review from Fokko July 3, 2025 00:17
@rambleraptor
Copy link
Contributor Author

addressed the comments, thanks for the review!

@sungwy
Copy link
Collaborator

sungwy commented Jul 4, 2025

Hi @rambleraptor the CI is failing due to a lint error.

pyiceberg/view/__init__.py:23:22: F401 `pydantic.Field` imported but unused
   |
21 | )
22 |
23 | from pydantic import Field
   |                      ^^^^^ F401
24 |
25 | from pyiceberg.typedef import Identifier
   |
   = help: Remove unused import: `pydantic.Field`

Could you run make lint and commit the changes?

@rambleraptor
Copy link
Contributor Author

@sungwy whoops, thanks! all fixed.

@rambleraptor
Copy link
Contributor Author

@sungwy @jayceslesar @Fokko can I get a review on this when you can? Thanks!

rambleraptor and others added 11 commits July 22, 2025 22:02
Co-authored-by: Sung Yun <107272191+sungwy@users.noreply.github.com>
Co-authored-by: Sung Yun <107272191+sungwy@users.noreply.github.com>
Co-authored-by: Sung Yun <107272191+sungwy@users.noreply.github.com>
Co-authored-by: Sung Yun <107272191+sungwy@users.noreply.github.com>
@rambleraptor rambleraptor changed the title Add create_view to REST Catalog [View Support] Add create_view to REST Catalog Jul 24, 2025
@rambleraptor rambleraptor changed the title [View Support] Add create_view to REST Catalog [View Support] Add ViewMetadata read support and create_view to REST Catalog Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants