Skip to content

feat(java): dataset supports reading transaction#4397

Merged
yanghua merged 3 commits intolance-format:mainfrom
majin1102:transaction_read
Aug 7, 2025
Merged

feat(java): dataset supports reading transaction#4397
yanghua merged 3 commits intolance-format:mainfrom
majin1102:transaction_read

Conversation

@majin1102
Copy link
Contributor

@majin1102 majin1102 commented Aug 6, 2025

Close #4382

So that java operations afterwards must implement read function

@github-actions github-actions bot added enhancement New feature or request java labels Aug 6, 2025
@majin1102 majin1102 changed the title feat(java): dataset supports read transaction feat(java): dataset supports reading transaction Aug 6, 2025
@majin1102 majin1102 marked this pull request as draft August 6, 2025 19:01
@majin1102 majin1102 marked this pull request as ready for review August 6, 2025 19:29
@majin1102
Copy link
Contributor Author

majin1102 commented Aug 6, 2025 •
edited
Loading

This PR is ready to review @jackye1995

@majin1102
Copy link
Contributor Author

majin1102 commented Aug 7, 2025

@chenghao-guo

jackye1995
jackye1995 approved these changes Aug 7, 2025
View reviewed changes
Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

looks good to me

chenghao-guo
chenghao-guo reviewed Aug 7, 2025
View reviewed changes
java/core/src/test/java/com/lancedb/lance/TransactionTest.java
dataset
.newTransactionBuilder()
.operation(Project.builder().schema(new Schema(fieldList)).build())
.build();
Copy link
Contributor

@chenghao-guo chenghao-guo Aug 7, 2025

Choose a reason for hiding this comment

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

thx, can you help add a transaction_properties in builder, and validate if we can get it correctly in readTransaction() later ?

Copy link
Contributor Author

@majin1102 majin1102 Aug 7, 2025

Choose a reason for hiding this comment

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

yeah, forgetten

Copy link
Contributor Author

@majin1102 majin1102 Aug 7, 2025

Choose a reason for hiding this comment

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

done

@majin1102 majin1102 mentioned this pull request Aug 7, 2025
Closed
@github-actions github-actions bot added the python label Aug 7, 2025
@majin1102 majin1102 force-pushed the transaction_read branch 3 times, most recently from 7d05971 to fc7f7d9 Compare August 7, 2025 05:53
@chenghao-guo
Copy link
Contributor

chenghao-guo commented Aug 7, 2025

@majin1102 I guess you are syncing the branch code to the latest main. You should use rebase instead of cherry-pick. Otherwise the commits will look a bit confusing
image

@majin1102 majin1102 mentioned this pull request Aug 7, 2025
Closed
@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.58%. Comparing base (01e9d1d) to head (aac4e0f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4397      +/-   ##
==========================================
- Coverage   80.59%   80.58%   -0.01%     
==========================================
  Files         301      301              
  Lines      108683   108683              
  Branches   108683   108683              
==========================================
- Hits        87588    87587       -1     
- Misses      17967    17969       +2     
+ Partials     3128     3127       -1
Flag Coverage Δ
unittests 80.58% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@majin1102
Copy link
Contributor Author

majin1102 commented Aug 7, 2025

@majin1102 I guess you are syncing the branch code to the latest main. You should use rebase instead of cherry-pick. Otherwise the commits will look a bit confusing

Did't cherry pick. Don't know what I did just to figure out some ci things.

@yanghua yanghua merged commit bb962d0 into lance-format:main Aug 7, 2025
8 checks passed
@majin1102 majin1102 deleted the transaction_read branch August 7, 2025 11:45
@chenghao-guo chenghao-guo mentioned this pull request Aug 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackye1995 jackye1995 jackye1995 approved these changes

+1 more reviewer

@chenghao-guo chenghao-guo chenghao-guo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request java python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Java dataset support readTransaction

5 participants

@majin1102 @chenghao-guo @codecov-commenter @jackye1995 @yanghua