Add Databricks 11.3 test environment to Delta Lake by ebyhr · Pull Request #14848 · trinodb/trino

ebyhr · 2022-11-01T09:17:59Z

Description

Non-technical explanation

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

findinpath · 2022-11-08T10:15:06Z

checkstyle issue: src/main/java/io/trino/tests/product/deltalake/util/DatabricksVersion.java:[20] (regexp) RegexpSinglelineJava: No new line before extends/implements

...o/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableAsSelectCompatibility.java

alexjo2144 · 2022-11-08T15:11:16Z

...n/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableCompatibility.java

+    private String getDatabricksDefaultTableProperties()
    {
-        return "TBLPROPERTIES (\n" +
-                "  'Type' = 'EXTERNAL',\n" +


Did 11.3 get rid of the type param?

Yes from SHOW CREATE TABLE result. DESCRIBE EXTENDED still provides such information.

findinpath · 2022-11-09T05:03:24Z

Checkstyle failure The field 'databricksRuntimeVersion' is never read.

findepi · 2022-11-09T19:24:15Z

.github/workflows/ci.yml

Why is this two entries?

It's under exclude section. These two entries are required to ignore hdp3 always and then enable another one when the secret exists in my understanding.

@nineinchnick @hashhar please review the ci.yml changes

LGTM. Note it adds quite a lot of time to the total CI runtime:

Do we need to test all LTSes?

We've brought up EOLing the tests for version 7.3. Maybe it's time, though DB is supporting it through March

...n/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksCheckpointsCompatibility.java

findepi · 2022-11-09T19:26:28Z

...o/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableAsSelectCompatibility.java

But that doesn't mean we should exclude the test, do it?

What we can do in 11.3 is verifying a checkpoint file wasn't created in CREATE OR REPLACE TABLE and skip, but I don't feel such logic is helpful in this test method. The test objective is accomplished by other versions. If we verify Databricks behavior (which operation creates a checkpoint file or not), adding dedicated tests look better to me.

Can CREATE OR REPLACE TABLE create a checkpoint in 11.3?
Can we force it to do that?

Note that the test specificially wants to test the scenario when checkpoint changes the schema

Unfortunately, I couldn't find a way to create a checkpoint using the statement, such options, alternative procedures or something in 11.3.

findepi · 2022-11-09T19:27:34Z

...c/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksInsertCompatibility.java

I missed that. Does Databricks 10.4 add ZSTD support out of the box?
🎉 finally we can switch Delta's default compression to ZSTD, right?

cc @losipiuk @alexjo2144

...t-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTransactionLogCache.java

...ino-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DatabricksVersion.java

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java

ebyhr · 2022-11-10T03:11:46Z

Addressed comments.

findepi · 2022-11-14T16:23:48Z

...n/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksCheckpointsCompatibility.java

+                    "  'delta.minReaderVersion' = '1',\n" +
+                    "  'delta.minWriterVersion' = '2')\n";
+        }
+        else if (databricksRuntimeVersion.equals(DATABRICKS_104_RUNTIME_VERSION)) {


redundant else

findepi · 2022-11-14T16:25:37Z

...o/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableAsSelectCompatibility.java

Can CREATE OR REPLACE TABLE create a checkpoint in 11.3?
Can we force it to do that?

Note that the test specificially wants to test the scenario when checkpoint changes the schema

findepi · 2022-11-14T16:25:54Z

...n/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableCompatibility.java

+                    "  'delta.minReaderVersion' = '1',\n" +
+                    "  'delta.minWriterVersion' = '2')\n";
+        }
+        else if (databricksRuntimeVersion.equals(DATABRICKS_104_RUNTIME_VERSION)) {


else is redundant

findepi · 2022-11-14T16:27:09Z

...c/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksInsertCompatibility.java

                        .containsOnly(expected);

-                if ("ZSTD".equals(compressionCodec) && !databricksRuntimeVersion.equals(DATABRICKS_104_RUNTIME_VERSION)) {
+                if ("ZSTD".equals(compressionCodec) && databricksRuntimeVersion.orElseThrow().isOlderThan(DATABRICKS_104_RUNTIME_VERSION)) {


if databricksRuntimeVersion must be present, can you make the field non-Optional, as you did in other tests?

This class has DELTA_LAKE_OSS group, so we can't make it non-Optional in the field.

findepi · 2022-11-14T16:28:10Z

...ino-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DatabricksVersion.java

+                .result();
+    }
+
+    public static DatabricksVersion createFromString(String versionString)


cla-bot bot added the cla-signed label Nov 1, 2022

github-actions bot added the tests:hive label Nov 1, 2022

ebyhr force-pushed the ebi/delta-databricks-11.3 branch 3 times, most recently from f799b4a to a40fa2f Compare November 8, 2022 09:08

ebyhr marked this pull request as ready for review November 8, 2022 09:09

ebyhr force-pushed the ebi/delta-databricks-11.3 branch 2 times, most recently from 95eff6f to 2e2a25e Compare November 8, 2022 10:26

ebyhr requested review from alexjo2144, findepi, homar and krvikash November 8, 2022 12:49

findinpath reviewed Nov 8, 2022

View reviewed changes

...o/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableAsSelectCompatibility.java Outdated Show resolved Hide resolved

alexjo2144 reviewed Nov 8, 2022

View reviewed changes

ebyhr force-pushed the ebi/delta-databricks-11.3 branch from 2e2a25e to 7c73ff8 Compare November 9, 2022 02:25

ebyhr force-pushed the ebi/delta-databricks-11.3 branch from 7c73ff8 to c5c5de6 Compare November 9, 2022 05:17

findepi reviewed Nov 9, 2022

View reviewed changes

ebyhr force-pushed the ebi/delta-databricks-11.3 branch 2 times, most recently from 5a41dde to 8937957 Compare November 10, 2022 00:14

github-actions bot added the docs label Nov 10, 2022

ebyhr requested a review from findepi November 10, 2022 22:17

findepi approved these changes Nov 14, 2022

View reviewed changes

Add Databricks 11.3 test environment to Delta Lake

9336f7e

ebyhr force-pushed the ebi/delta-databricks-11.3 branch from 8937957 to 9336f7e Compare November 14, 2022 22:56

ebyhr merged commit 715c3fb into master Nov 15, 2022

ebyhr deleted the ebi/delta-databricks-11.3 branch November 15, 2022 05:00

github-actions bot added this to the 403 milestone Nov 15, 2022

colebow mentioned this pull request Nov 15, 2022

Add Trino 403 release notes #14977

Merged

Conversation

ebyhr commented Nov 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Non-technical explanation

Release notes

Uh oh!

findinpath commented Nov 8, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

findinpath commented Nov 9, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebyhr commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

ebyhr commented Nov 1, 2022 •

edited

Loading