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

[Kernel] Add type widening to supported table features #3656

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johanl-db
Copy link
Collaborator

@johanl-db johanl-db commented Sep 9, 2024

Description

Allow reading and writing to tables that have the type widening table features enabled (both preview and stable table feature).

Reading:

How was this patch tested?

Added read integration tests.
Tests are based on golden tables. Generating the tables requires Spark 4.0, due to spark master cross-compilation being broken, the table generation code is not included here.
The following steps where used to generate the tables.

  1. Create a table with initial data types and insert initial data
  2. Enable type widening and schema evolution
  3. Insert data with wider type for each column. Column types are automatically widened during schema evolution.
Column Initial type Widened Type
byte_long byte long
int_long int long
float_double float double
byte_double byte double
short_double short double
int_double int double
decimal_decimal_same_scale decimal(10, 2) decimal(20, 2)
decimal_decimal_greater_scale decimal(10, 2) decimal(20, 5)
byte_decimal byte decimal(11, 1)
short_decimal short decimal(11, 1)
int_decimal int decimal(11, 1)
long_decimal long decimal(21, 1)
date_timestamp_ntz date timestamp_ntz

Does this PR introduce any user-facing changes?

Yes, it's now possible to read from and write to delta tables with type widening enabled using kernel.

@johanl-db johanl-db changed the title [WIP][Kernel] Add type widening to supported table features [Kernel] Add type widening to supported table features Sep 25, 2024
Copy link
Collaborator

@vkorukanti vkorukanti left a comment

Choose a reason for hiding this comment

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

One minor comment about tests, otherwise LGTM

@@ -199,6 +199,22 @@ public void runTests() throws Exception {
asList(new Column("as_int"), Literal.ofInt(1))
)),
0 /* expected row count */);

// Type widening: table with various type changes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

these release verification tests are great! Can you add these to the DeltaTableReadsSuite.scala which are run as part of the Kernel tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or verifying the result is a problem? If yes, may be select one or two rows using filter and hardcode the expected values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants