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

Add test for remote partition evolution in Delta Lake #13489

Merged
merged 1 commit into from
Aug 4, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,33 @@ private void testTrinoCanReadFromTableUpdatedByDatabricksWithCpIntervalSet(int i
onDelta().executeQuery("DROP TABLE default." + tableName);
}
}

@Test(groups = {DELTA_LAKE_DATABRICKS, PROFILE_SPECIFIC_TESTS})
public void testTrinoCanReadFromTablePartitionChangedByDatabricks()
{
String tableName = "test_dl_create_table_partition_changed_by_databricks_" + randomTableSuffix();
String tableDirectory = "databricks-compatibility-test-" + tableName;

ImmutableList<QueryAssert.Row> expected = ImmutableList.of(row(1, "part"));

onDelta().executeQuery(format("CREATE TABLE default.%s " +
"USING DELTA " +
"PARTITIONED BY (`original_part_col`) LOCATION 's3://%s/%s' AS " +
"SELECT 1 AS original_part_col, 'part' AS new_part_col",
tableName,
bucketName,
tableDirectory));

try {
assertThat(onTrino().executeQuery("SELECT * FROM delta.default." + tableName)).contains(expected);

onDelta().executeQuery("REPLACE TABLE default." + tableName + " USING DELTA PARTITIONED BY (new_part_col) AS SELECT * FROM " + tableName);

// This 2nd SELECT query caused NPE when the connector had cache for partitions and the column was changed remotely
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is not necessary because this compatibility test is a valid usecase.

Copy link
Member Author

@ebyhr ebyhr Aug 4, 2022

Choose a reason for hiding this comment

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

I suppose future readers can't quickly realize this test purpose without comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that the name of the test could be centered around REPLACE which is a SQL command specific for Databricks.

testTrinoCanReadAndWriteIntoTableReplacedInDatabricks

Copy link
Member Author

@ebyhr ebyhr Aug 4, 2022

Choose a reason for hiding this comment

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

REPLACE statement is just a part of steps to reproduce and it's not purpose of this test. Let me keep as-is.

assertThat(onTrino().executeQuery("SELECT * FROM delta.default." + tableName)).contains(expected);
}
finally {
onDelta().executeQuery("DROP TABLE default." + tableName);
}
}
}