Skip to content

Run test_create_table_transaction for Rest V1 tables #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Fokko merged 4 commits into from
Jun 23, 2025
Merged

Run test_create_table_transaction for Rest V1 tables #2137

Fokko merged 4 commits into from
Jun 23, 2025

Conversation

@Fokko
Copy link
Contributor

@Fokko Fokko commented Jun 23, 2025 •
edited
Loading

Rationale for this change

Moar test coverage :)

Are these changes tested?

Are there any user-facing changes?

kevinjqliu
kevinjqliu approved these changes Jun 23, 2025
View reviewed changes
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM!

Theres another one of these in

iceberg-python/tests/integration/test_writes/test_writes.py

Lines 945 to 948 in e937f6a

if format_version == 1 and isinstance(catalog, RestCatalog):
pytest.skip(
"There is a bug in the REST catalog image (https://github.com/apache/iceberg/issues/8756) that prevents create and commit a staged version 1 table"
)

I did a search for pytest.skip https://grep.app/search?f.repo.pattern=iceberg-python&q=pytest.skip

@Fokko Fokko force-pushed the fd-has-been-fixed branch from 4f5c68f to f78dbf4 Compare June 23, 2025 20:24
kevinjqliu
kevinjqliu reviewed Jun 23, 2025
View reviewed changes
dev/provision.py
# There is an issue with CREATE OR REPLACE
# https://github.com/apache/iceberg/issues/8756
spark.sql(f"DROP TABLE IF EXISTS {catalog_name}.default.test_table_version")

Copy link
Contributor

@kevinjqliu kevinjqliu Jun 23, 2025

Choose a reason for hiding this comment

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

good catch, i think we can also remove L276-L278

Copy link
Contributor Author

@Fokko Fokko Jun 23, 2025

Choose a reason for hiding this comment

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

Unfortunally not, the OR REPLACE syntax requires a SQL extension I believe, so I'll leave that one for now.

@Fokko Fokko merged commit acbdeca into main Jun 23, 2025
10 checks passed
@Fokko Fokko deleted the fd-has-been-fixed branch June 23, 2025 20:48
@Fokko
Copy link
Contributor Author

Fokko commented Jun 23, 2025

Thanks @kevinjqliu for the review 🙌

This was referenced Jun 23, 2025
Merged
Merged
Fokko pushed a commit that referenced this pull request Jun 24, 2025
@kevinjqliu
test: use create or replace (#2140)
62a95cf 
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change
Follow up to #2137
Remove all mentions of `CREATE OR REPLACE` issue,
apache/iceberg#8756

# Are these changes tested?

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
Fokko pushed a commit to apache/iceberg-go that referenced this pull request Jun 24, 2025
@kevinjqliu
test: use create or replace (#466)
15b386b 
Remove all mentions of `CREATE OR REPLACE` issue,
apache/iceberg#8756
Similar to apache/iceberg-python#2137 and
apache/iceberg-python#2140
amitgilad3 pushed a commit to amitgilad3/iceberg-python that referenced this pull request Jul 7, 2025
@Fokko @amitgilad3
Run test_create_table_transaction for Rest V1 tables (apache#2137)
2925917 
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change

Moar test coverage :)

# Are these changes tested?

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
amitgilad3 pushed a commit to amitgilad3/iceberg-python that referenced this pull request Jul 7, 2025
@kevinjqliu @amitgilad3
test: use create or replace (apache#2140)
f1d2af2 
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change
Follow up to apache#2137
Remove all mentions of `CREATE OR REPLACE` issue,
apache/iceberg#8756

# Are these changes tested?

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
gabeiglio pushed a commit to Netflix/iceberg-python that referenced this pull request Aug 13, 2025
@Fokko @gabeiglio
Run test_create_table_transaction for Rest V1 tables (apache#2137)
af174b1 
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change

Moar test coverage :)

# Are these changes tested?

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
gabeiglio pushed a commit to Netflix/iceberg-python that referenced this pull request Aug 13, 2025
@kevinjqliu @gabeiglio
test: use create or replace (apache#2140)
f8a5ee2 
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change
Follow up to apache#2137
Remove all mentions of `CREATE OR REPLACE` issue,
apache/iceberg#8756

# Are these changes tested?

# Are there any user-facing changes?

<!-- In the case of user-facing changes, please add the changelog label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevinjqliu kevinjqliu kevinjqliu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Fokko @kevinjqliu