-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Fix Removing Partition Builder #22735
Conversation
This PR doesn't set the lastDataCommitTime. I would love to initiate the discussion whether we need to update the corresponding attribute. If so, we might need a setter method instead of purely builder class (or if anyone else has better suggestions on this) |
There is a test failure related to your code changes
|
Tangential note, but setter methods instead of or in addition to builders are really useful in test code like this. I've been seeing a need for these in other test classes. They can be marked Immutability is a convenient property to have but only when objects are truly immutable. When the things the objects represent do change over time, classes should be mutable to reflect that. E.g. if a file's last modified time isn't fixed, then the File object shouldn't be immutable either. |
@elharo thanks it's good to know about it. Nikhil and I have synced offline and we came out with a hack way to deal with this issue instead of changing the open source code. However, I will def look into what you mentioned about |
Description
Ensuring that the partition maintains its original type (e.g., child class) and associated information.
Motivation and Context
The issue here is that the Partition class is a base class for other child-class Partition, and when using the Partition.builder() method to create a new instance of Partition, the resulting object will be of type Partition rather than child-class Partition. This means that any methods or properties specific to child-class Partition will not be available on the new object created by the builder.
Impact
Test Plan
Unit Test in
TestMemcacheHiveMetastore.java
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
== NO RELEASE NOTE ==