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.

Sign up for GitHub

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

Jump to bottom

[Spark] Test type widening compatibility with other Delta features #3053

Merged
merged 3 commits into from
May 23, 2024
Merged

[Spark] Test type widening compatibility with other Delta features #3053

merged 3 commits into from
May 23, 2024

Conversation

johanl-db
Copy link
Collaborator

@johanl-db johanl-db commented May 6, 2024 •
edited

Description

Additional tests covering type widening and:

  • Reading CDF
  • Column mapping
  • Time travel
  • RESTORE
  • CLONE

How was this patch tested?

Test only

@johanl-db johanl-db force-pushed the more-type-widening-tests branch 3 times, most recently from cd9c3e6 to d5ed864 Compare May 7, 2024 09:02
@johanl-db johanl-db self-assigned this May 8, 2024
tomvanbussel
tomvanbussel reviewed May 8, 2024
View reviewed changes
Copy link
Contributor

@tomvanbussel tomvanbussel left a comment

Choose a reason for hiding this comment

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

Added a few suggestions for additional tests. Do we already have tests for constraints and generated columns. AFAIK we disallow type changes on generated columns, but constraints might be interesting as it's possible to create a constraint after changing the type.

spark/src/test/scala/org/apache/spark/sql/delta/DeltaTypeWideningSuite.scala Outdated Show resolved Hide resolved
spark/src/test/scala/org/apache/spark/sql/delta/DeltaTypeWideningSuite.scala Show resolved Hide resolved
spark/src/test/scala/org/apache/spark/sql/delta/DeltaTypeWideningSuite.scala Outdated Show resolved Hide resolved
spark/src/test/scala/org/apache/spark/sql/delta/DeltaTypeWideningSuite.scala Show resolved Hide resolved
@johanl-db
Copy link
Collaborator Author

Added a few suggestions for additional tests. Do we already have tests for constraints and generated columns. AFAIK we disallow type changes on generated columns, but constraints might be interesting as it's possible to create a constraint after changing the type.

There are tests already for generated columns and constraints, added as part of #2881

I added a test here to cover constraints + RESTORE which is an interesting case:

  1. Change column a type from byte to int
  2. Add a CHECK constraint on column a.
  3. RESTORE table to before the type change.

The constraint was added on type int and the column now has type byte. This does work though because CHECK constraints are part of the table metadata and it gets removed during RESTORE.

tomvanbussel
tomvanbussel approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@tomvanbussel tomvanbussel left a comment

Choose a reason for hiding this comment

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

LGTM!

@tdas tdas merged commit 039a29a into delta-io:master May 23, 2024
10 checks passed
@johanl-db johanl-db mentioned this pull request May 24, 2024
Open
longvu-db pushed a commit to longvu-db/delta that referenced this pull request May 28, 2024
@johanl-db @longvu-db
[Spark] Test type widening compatibility with other Delta features (d…
ce80530 
…elta-io#3053)

## Description
Additional tests covering type widening and:
- Reading CDF
- Column mapping
- Time travel
- RESTORE
- CLONE

## How was this patch tested?
Test only
longvu-db pushed a commit to longvu-db/delta that referenced this pull request May 30, 2024
@johanl-db @longvu-db
[Spark] Test type widening compatibility with other Delta features (d…
4a7013a 
…elta-io#3053)

Additional tests covering type widening and:
- Reading CDF
- Column mapping
- Time travel
- RESTORE
- CLONE

Test only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomvanbussel tomvanbussel tomvanbussel approved these changes

Assignees

@johanl-db johanl-db

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@johanl-db @tomvanbussel @tdas