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

[AMORO-2091] 0.4.x resolve the optimzing exception for KeyedTable with Timestamp(without zone) column as Primary key #2092

Closed
wants to merge 1 commit into from

Conversation

wangtaohz
Copy link
Contributor

Why are the changes needed?

Close #2091 . fix in 0.4.x

Brief change log

  • NodeFilter should use InternalRecordWrapper to build PrimaryKeyData

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@wangtaohz wangtaohz marked this pull request as ready for review October 12, 2023 13:01
@github-actions github-actions bot added the module:core Core module label Oct 12, 2023
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...m/netease/arctic/io/reader/ArcticDeleteFilter.java 76.26% <0.00%> (-0.39%) ⬇️

📢 Thoughts on this report? Let us know!.

@zhoujinsong
Copy link
Contributor

zhoujinsong commented Oct 13, 2023

@wangtaohz Should we merge the changes to master branch too?

BTW,If the issue exist in master branch, we may fix it and merge back to 0.4.x.

@wangtaohz
Copy link
Contributor Author

@wangtaohz Should we merge the changes to master branch too?
BTW,If the issue exist in master branch, we may fix it and merge back to 0.4.x.

Yes, the master has the same code, we'd better fix in master and cherry-pick to 0.4.x and also 0.5.x.

However, we can't reproduce it in master and 0.5.x (Mixed Format KeyedTable, Timestamp witout timezone as Primary Key: optimizing), because the sourceNodes for reader is null after version 0.5.x, so the NodeFilter didn't work in optimizing.

I also tried to reproduce it with MOR in flink and spark, and they all works well.

@wangtaohz
Copy link
Contributor Author

We should not merge this PR, and I will cherry-pick #2095 to 0.4.x instead, then close this PR.

@wangtaohz wangtaohz closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:core Core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants