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

Assign materialized Row ID and Row commit version column names #1896

Closed
wants to merge 2 commits into from
Closed

Assign materialized Row ID and Row commit version column names #1896

wants to merge 2 commits into from

Conversation

johanl-db
Copy link
Collaborator

Description

This change is part of implementing row tracking as specified in #1610 and 7272b04.
It covers assigning a column name for the materialized Row ID and Row commit version columns by setting them in the table metadata when creating or cloning a table.

How was this patch tested?

  • Add test suite rowtracking.MaterializedColumnSuite to cover assigning materialized column names in various table creation and clone scenarios.

@felipepessoto
Copy link
Contributor

felipepessoto commented Jul 11, 2023 •
edited

@johanl-db are you planning to have row tracking supported in 3.0.0 release?

@johanl-db
Copy link
Collaborator Author

johanl-db commented Jul 12, 2023 •
edited

@johanl-db are you planning to have row tracking supported in 3.0.0 release?

Row tracking support in 3.0 will be limited to writing fresh row IDs / row commit versions, i.e. the changes previously introduced with these 2 PRs:

Reading row IDs / row commit versions and writing materialized row IDs / row commit versions require changes that will be in Spark 3.5, see e.p.:

These changes (and a couple other related ones) will allow us to define new metadata fields for row IDs / row commit versions directly from Delta - before that metadata fields were hard-coded in Spark's FileFormat
My plan is to resume work on row tracking when Spark 3.5 is available.

I'm opening this PR to assign the materialized column names because it can be done already without the previous Spark changes + it's a requirement to enable row tracking or write to a table that has row tracking enabled.

@felipepessoto
Copy link
Contributor

Thanks for clarifying it.

tomvanbussel
tomvanbussel approved these changes Jul 14, 2023
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! Does this mean we can enable the Row Tracking feature outside testing now?

@johanl-db johanl-db requested a review from olaky July 17, 2023 14:38
@johanl-db
Copy link
Collaborator Author

LGTM! Does this mean we can enable the Row Tracking feature outside testing now?

Yes, we should be able to create a table with row tracking enabled, read from and write to it but we can't read the row tracking metadata just yet so it's still limited. We also can't enable row tracking on a table after it's creation, this will require backfilling row tracking information for existing files.

@johanl-db johanl-db mentioned this pull request Sep 15, 2023
Open
5 tasks
@johanl-db johanl-db mentioned this pull request Sep 27, 2023
Closed
8 tasks
This was referenced Apr 8, 2024
Closed
Closed
Merged
scottsand-db pushed a commit that referenced this pull request Apr 9, 2024
@longvu-db
[Spark] Enable Row Tracking outside of testing (#2866)
5fdba36 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [X] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [ ] Other (fill in here)

## Description
Delta Protocol for Row IDs was introduced in this PR:
#1610

Support for writing fresh row IDs / row commit versions was introduced
in the following PRs:
- #1723
- #1781
- #1896

**This is sufficient to enable row tracking on a table and write to a
table that has row tracking enabled** but not to actually read row IDs /
row commit versions back, which is also being added in Delta at the
moment ([read
BaseRowId](283ac02).
[read
defaultRowCommitVersion](#2795),
[read RowId](#2856)...)

Using row tracking is currently only allowed in testing, this change
allows enabling row tracking outside of testing so that the upcoming
Delta 3.2 release includes support for writing to tables with row
tracking enabled, making Delta writers future-proof.
<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->

## How was this patch tested?
Tests have already been added in previous changes, this only flips the
switch to let users enabled Row Tracking outside of tests.

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->

## Does this PR introduce _any_ user-facing changes?
Users are now able to enable Row Tracking when creating a delta table:
```
CREATE TABLE tbl(a int) USING DELTA TBLPROPERTIES ('delta.enableRowTracking' = 'true')
```
<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
andreaschat-db pushed a commit to andreaschat-db/delta that referenced this pull request Apr 16, 2024
@longvu-db @andreaschat-db
[Spark] Enable Row Tracking outside of testing (delta-io#2866)
42a5a3f 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [X] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [ ] Other (fill in here)

## Description
Delta Protocol for Row IDs was introduced in this PR:
delta-io#1610

Support for writing fresh row IDs / row commit versions was introduced
in the following PRs:
- delta-io#1723
- delta-io#1781
- delta-io#1896

**This is sufficient to enable row tracking on a table and write to a
table that has row tracking enabled** but not to actually read row IDs /
row commit versions back, which is also being added in Delta at the
moment ([read
BaseRowId](delta-io@283ac02).
[read
defaultRowCommitVersion](delta-io#2795),
[read RowId](delta-io#2856)...)

Using row tracking is currently only allowed in testing, this change
allows enabling row tracking outside of testing so that the upcoming
Delta 3.2 release includes support for writing to tables with row
tracking enabled, making Delta writers future-proof.
<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->

## How was this patch tested?
Tests have already been added in previous changes, this only flips the
switch to let users enabled Row Tracking outside of tests.

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->

## Does this PR introduce _any_ user-facing changes?
Users are now able to enable Row Tracking when creating a delta table:
```
CREATE TABLE tbl(a int) USING DELTA TBLPROPERTIES ('delta.enableRowTracking' = 'true')
```
<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
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

@olaky olaky Awaiting requested review from olaky

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@johanl-db @felipepessoto @olaky @tomvanbussel