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

Loader: Add legacy file hierarchy support #1320

Closed
wants to merge 2 commits into from

Conversation

oguzhanunlu
Copy link
Member

No description provided.

@oguzhanunlu oguzhanunlu self-assigned this Nov 6, 2023
@oguzhanunlu oguzhanunlu changed the base branch from master to develop November 6, 2023 12:44
@oguzhanunlu oguzhanunlu changed the title Add legacy file hierarchy support for loader Loader: Add legacy file hierarchy support Nov 6, 2023
): LoadStatements = {
val shreddedStatements = discovery.shreddedTypes
.filterNot(_.isAtomic)
.groupBy(_.getLoadPath)
.groupBy(_.getLoadPath(legacyPartitioning))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think lines 97-101 are now incorrect for the case when legacy partitioning is enabled and if the schema requires a recovery table.

If legacy partitioning is disabled, then it is OK. And if schema evolution rules are not broken, then it is OK.

Imagine this batch uses versions 1-0-0 and 1-0-1 of the same schema. And imagine 1-0-1 is broken so needs a recovery table.

When you call .map(_.head) you do not know whether the head is 1-0-0 or 1-0-1. So when you call mergeResult = .... (line 97) you don't know if you get the good schema or the recovery schema.


Before I offer a solution... I need to understand the corresponding feature in the transformer, #1315 . I have not yet read through that PR, so shall do so next.

@oguzhanunlu
Copy link
Member Author

This is cancelled

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

Successfully merging this pull request may close these issues.

2 participants