Skip to content

Prevent creating the table when the specified schema does not exist#15788

Merged
ebyhr merged 3 commits intotrinodb:masterfrom
krvikash:fix-schema-does-not-exists-in-create-table
Feb 13, 2023
Merged

Prevent creating the table when the specified schema does not exist#15788
ebyhr merged 3 commits intotrinodb:masterfrom
krvikash:fix-schema-does-not-exists-in-create-table

Conversation

@krvikash
Copy link
Copy Markdown
Contributor

@krvikash krvikash commented Jan 19, 2023

Description

Fixes #15779

Release notes

(X) Release notes are required, with the following suggested text:

# Iceberg
* Prevent creating the table when the specified schema does not exist. ({issue}`15779`)

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jan 20, 2023

@krvikash Could you confirm CI failures?

@findinpath
Copy link
Copy Markdown
Contributor

@krvikash would there a similar approach as in Delta Lake help in this case ?

public void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, boolean ignoreExisting)
{
SchemaTableName schemaTableName = tableMetadata.getTable();
String schemaName = schemaTableName.getSchemaName();
String tableName = schemaTableName.getTableName();
Database schema = metastore.getDatabase(schemaName).orElseThrow(() -> new SchemaNotFoundException(schemaName));

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 1a1bc4c to 0f2e0e2 Compare January 27, 2023 06:29
Copy link
Copy Markdown
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Can we test this?

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 0f2e0e2 to e9ada98 Compare January 30, 2023 17:59
@findinpath
Copy link
Copy Markdown
Contributor

Failing tests on trino-iceberg relate to #15905

Pls push an empty commit when the above mentioned PR lands.

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 73d09d2 to c3fb2eb Compare February 1, 2023 06:16
@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 1, 2023

Addressed comments.

Copy link
Copy Markdown
Member

@ebyhr ebyhr Feb 1, 2023

Choose a reason for hiding this comment

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

We would like to keep the smoke test as simple as possible. Can we move to BaseIcebergConnectorTest instead? Or do you have a specific reason to run this test in the smoke test class?

Copy link
Copy Markdown
Contributor Author

@krvikash krvikash Feb 1, 2023

Choose a reason for hiding this comment

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

Yes, there is a specific reason to have this test in BaseIcebergConnectorSmokeTest. I wanted to test against cloud storage different metastore. BaseIcebergConnectorTest only covers the Local file system.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

without this fix:
Using Hive metastore -> files are created and Exception is thrown Schema (.*) not found
Using Glue metastore -> files are created and Exception is thrown <“Database non_existing_schema_mjmeupcfxy not found. (Service: AWSGlue; Status Code: 400; Error Code: EntityNotFoundException; Request ID: 2225b446-d53f-48f5-9fe9-de4fd3e74e54; Proxy: null)“>

with this fix: Schema (.*) not found exception is thrown for all metastore.

BaseIcebergConnector does not verify for glue. So IMO BaseIcebergConnectorSmokeTest is best suited.

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from c3fb2eb to 0f8b97e Compare February 1, 2023 09:52
@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 0f8b97e to 9b98279 Compare February 2, 2023 07:16
@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 2, 2023

Addressed comments.

@krvikash krvikash changed the title Fix Create Table when schema does not exist Prevent creating the table when the specified schema does not exist Feb 2, 2023
@findinpath
Copy link
Copy Markdown
Contributor

Build is red see ci / maven-checks

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 9b98279 to 84a9eb2 Compare February 3, 2023 07:06
@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 3, 2023

Error: Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.3.0:analyze-only (default) on project trino-iceberg: Dependency problems found -> [Help 1]

Warning: Used undeclared dependencies found:
Warning: com.qubole.rubix:rubix-presto-shaded:jar:0.3.18:compile
Warning: Non-test scoped test only dependencies found:
Warning: com.qubole.rubix:rubix-presto-shaded:jar:0.3.18:compile

I think it's unrelated to my change. Retriggerd the CI build.

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch 2 times, most recently from d6be2a6 to fe53676 Compare February 6, 2023 04:52
@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 6, 2023

Build is red see ci / maven-checks

Addressed. It was a shaded class import that was causing issues.

@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 6, 2023

CI is failing with the Flaky test

2023-02-06T05:46:29.8748259Z tests | 2023-02-06 11:31:29 INFO: FAILURE / io.trino.tests.product.kudu.TestKuduConnectoKerberosSmokeTest.kerberosAuthTicketExpiryTest (Groups: profile_specific_tests, kudu) took 5.4 seconds

#14441

@krvikash krvikash self-assigned this Feb 7, 2023
@findinpath findinpath requested a review from ebyhr February 7, 2023 13:00
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 10, 2023

/test-with-secrets sha=fe53676f991b788e14fdfd0cbb2eda81236aa6a7

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from c2f6ab6 to 1e97a1e Compare February 10, 2023 09:04
@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch 2 times, most recently from 8d483af to 1f50b86 Compare February 10, 2023 10:03
@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4141358815

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 1f50b86 to 6b2619c Compare February 10, 2023 11:17
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 10, 2023

/test-with-secrets sha=6b2619c9795863e24e55c61f312c8514e47c4004

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 6b2619c to e63cd92 Compare February 10, 2023 20:48
@krvikash
Copy link
Copy Markdown
Contributor Author

Fixed CI failure.

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4145615692

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 10, 2023

/test-with-secrets sha=e63cd92d7c334d6140d233d832a90dddeac7d10e

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4148929977

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from e63cd92 to 8fa7ac9 Compare February 11, 2023 11:46
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 12, 2023

/test-with-secrets sha=8fa7ac9eea1281f24fbc7eb546a65ebfaabbabc2

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4153825112

@krvikash krvikash force-pushed the fix-schema-does-not-exists-in-create-table branch from 8fa7ac9 to 93d31bd Compare February 12, 2023 09:48
@krvikash
Copy link
Copy Markdown
Contributor Author

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4153825112

Fixed it

Error:  Failures: 
Error:    TestIcebergGlueCatalogAccessOperations.testCreateTable:143->assertGlueMetastoreApiInvocations:429->assertGlueMetastoreApiInvocations:466 Expected: 
		1 fewer occurrences of GET_DATABASE
Error:    TestIcebergGlueCatalogAccessOperations.testCreateTableAsSelect:159->assertGlueMetastoreApiInvocations:429->assertGlueMetastoreApiInvocations:466 Expected: 
		1 fewer occurrences of GET_DATABASE
[INFO] 
Error:  Tests run: 198, Failures: 2, Errors: 0, Skipped: 0

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 12, 2023

/test-with-secrets sha=93d31bd217f6e503d7b037680942206bd120c2f7

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4158597848

@krvikash
Copy link
Copy Markdown
Contributor Author

krvikash commented Feb 13, 2023

CI Failure:
Exiting issue: #13199

io.trino.tests.product.deltalake.TestDeltaLakeAlterTableCompatibility.testCommentOnColumn (Groups: profile_specific_tests, delta-lake-databricks, delta-lake-oss) took 5.1 seconds

This is a new error (https://github.com/trinodb/trino/actions/runs/4158597848/jobs/7194014765). I don't think this error is related to this PR.

io.trino.tests.product.kafka.TestKafkaProtobuf.testInsertAllDataType (Groups: profile_specific_tests, kafka) took 0.8 seconds

@ebyhr ebyhr merged commit 607decc into trinodb:master Feb 13, 2023
@ebyhr ebyhr mentioned this pull request Feb 13, 2023
@krvikash krvikash deleted the fix-schema-does-not-exists-in-create-table branch February 13, 2023 06:20
@github-actions github-actions bot added this to the 407 milestone Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Files get created on S3 while creating a iceberg table even if schema does not exist

4 participants