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

Issue #405: DataWriter refactory #402

Merged
merged 20 commits into from
Sep 10, 2024
Merged

Issue #405: DataWriter refactory #402

merged 20 commits into from
Sep 10, 2024

Conversation

cugni
Copy link
Member

@cugni cugni commented Sep 6, 2024 •
edited
Loading

Description

This PR is working toward the abstraction of Qbeast-spark from Delta code #398. It makes the RollupDataWrite more independent of the Open Table Format used and reorganizes the roles of accessing the data between the DataWriter and the QbeastSnapshot. This PR also solves a potential issue when optimizing a table with a changed schema.

Checklist:

Here is the list of things you should do before submitting this pull request:

  • New feature / bug fix has been committed following the Contribution guide.
  • Add logging to the code following the Contribution guide.
  • Add comments to the code (make it easier for the community!).
  • Change the documentation.
  • Add tests.
  • Your branch is updated to the main branch (dependent changes have been merged).

@cugni
Start refactoring of DataWriter, to centralize logic
14e8178
@cugni
making the case class construction private in the package, so it is c…
9eb30d2 
…lear which constructor should be used.
@cugni
Removing the API optimize from DataWriter
44af46e
@cugni
Fixing the analyzeOptimize implementation
8eb9ba8
@cugni
making sure we use the right schema when loading the parquet
e563198
@cugni
Unpersisting the optimized analyzed data once it has been written.
d033801
@cugni
Fixing test removing private method access
be35616
@cugni
scalafmt
8ec672f
@cugni
Merge branch 'main' into data-writer-refact
9e96bd1
@cugni
Renaming Cube to CubeState and moving it to model. Also, we reduced t…
c515123 
…he states to ANNOUNCED and LEAF (REPLICATION WAS NEVER CONSIDRED NOW)
@cugni
Calcolating the blockSize with teh domain
50266a7
@cugni
Fixed formatting and headers
e16a01b
@cugni
avoid linting warning
55d4f14
@cugni
changing types
5438731
@cugni
Rollback some wrong changes
59ac953
@cugni
fix test
70fa039
@cugni
wip on the test
0c908a3
@cugni
Renaming and fixing tests.
0eb6bb0
@cugni cugni marked this pull request as ready for review September 9, 2024 13:53
@cugni cugni requested review from Jiaweihu08, fpj and osopardo1 and removed request for fpj September 9, 2024 13:54
osopardo1
osopardo1 reviewed Sep 10, 2024
View reviewed changes
Copy link
Member

@osopardo1 osopardo1 left a comment

Choose a reason for hiding this comment

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

thanks for reducing the refactor PR size :)
few comments:

build.sbt Show resolved Hide resolved
spark-warehouse/student/_delta_log/00000000000000000000.json Outdated Show resolved Hide resolved
src/main/scala/io/qbeast/core/model/QbeastSnapshot.scala Outdated Show resolved Hide resolved
src/main/scala/io/qbeast/spark/delta/writer/BlockWriter.scala Show resolved Hide resolved
src/main/scala/io/qbeast/spark/table/IndexedTable.scala Show resolved Hide resolved
src/test/scala/io/qbeast/spark/delta/QbeastSnapshotTest.scala Show resolved Hide resolved
@cugni cugni mentioned this pull request Sep 10, 2024
Open
@osopardo1 osopardo1 changed the title DataWriter refactory Issue #405: DataWriter refactory Sep 10, 2024
@osopardo1
Copy link
Member

FYI @cugni : I've opened a new issue with a more fine-grained scope than the Delta Refactor, and I've also changed the PR title with the Issue number linked.

@osopardo1 osopardo1 self-requested a review September 10, 2024 12:43
osopardo1
osopardo1 approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@osopardo1 osopardo1 left a comment

Choose a reason for hiding this comment

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

Approved 👍

@osopardo1 osopardo1 merged commit 29cb3ac into Qbeast-io:main Sep 10, 2024
1 check passed
@osopardo1 osopardo1 mentioned this pull request Sep 10, 2024
Closed
@JosepSampe JosepSampe mentioned this pull request Oct 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@osopardo1 osopardo1 osopardo1 approved these changes

@Jiaweihu08 Jiaweihu08 Awaiting requested review from Jiaweihu08

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

Successfully merging this pull request may close these issues.
2 participants
@cugni @osopardo1