Skip to content
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.

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

Remove BaseIcebergConnectorTest.dropTable method #21347

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Apr 1, 2024

Description

BaseConnectorTest#testDropTable ensures non-existence of the table after dropping. No need to verify that in each test.

Also, getQueryRunner().tableExists was misused in testPartitionedTableStatistics. The method always returns false if we pass qualified table names.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Apr 1, 2024
@github-actions github-actions bot added the iceberg Iceberg connector label Apr 1, 2024
@ebyhr ebyhr requested a review from wendigo April 1, 2024 23:59
@oneonestar
Copy link
Member

(NIT)

I think

dropTable(tableName);
dropTable("test_create_partitioned_table_as");

is easier to read than:

assertUpdate("DROP TABLE " + tableName);
assertUpdate("DROP TABLE " + "test_create_partitioned_table_as");

@ebyhr ebyhr force-pushed the ebi/iceberg-test-drop-table branch from b2b584d to 8c0a55d Compare April 2, 2024 00:53
@wendigo
Copy link
Contributor

wendigo commented Apr 2, 2024

Can we instead use a TestTable here?

@ebyhr
Copy link
Member Author

ebyhr commented Apr 3, 2024

@wendigo Yes, but let me handle in follow-up.

@ebyhr ebyhr merged commit a0bcfb8 into trinodb:master Apr 3, 2024
44 checks passed
@ebyhr ebyhr deleted the ebi/iceberg-test-drop-table branch April 3, 2024 05:28
@github-actions github-actions bot added this to the 444 milestone Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed iceberg Iceberg connector
Development

Successfully merging this pull request may close these issues.

3 participants