-
Notifications
You must be signed in to change notification settings - Fork 330
[Policy Store] Add policyTypeCode to Slice/Index for Future Filtering Support and Update Policy Persistence Method #1628
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
[Policy Store] Add policyTypeCode to Slice/Index for Future Filtering Support and Update Policy Persistence Method #1628
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's worth bothering with EclipseLink support for new features since EclipseLink is deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with either way. Adding it to EclipseLink is nice to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since policy support is already available in EclipseLink and this PR introduces only a minor change ahead of the 1.0 release, I think it's reasonable to include it. My understanding is that we don’t plan to add any major new features to EclipseLink moving forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker: I feel like these two classes are interchangeable in Polaris. Can we merge them to avoid any type mismatching like this? cc @dennishuo
flyrain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
Please rebase to get the latest CI changes |
4395fbb to
dc12b13
Compare
| default void deleteAllEntityPolicyMappingRecordsInCurrentTxn( | ||
| @Nonnull PolarisCallContext callCtx, | ||
| @Nonnull PolarisEntityCore entity, | ||
| @Nonnull PolarisBaseEntity entity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note: we should probably review the PolarisEntityCore type hierarchy. PolarisBaseEntity dominates all child types, so it can probably be folded into PolarisEntityCore (or the other way around).
This PR adds
policyTypeCodeto both the in-memory tree map store's slice and the SQL tablepolicy_mapping_recordsindex (JDBC has already included this in #1468 ). This change lays the groundwork for future features that require efficient filtering by policy type—such as fetching all entities with a data compaction policy attached.As part of this update, the signature of
loadAllTargetsOnPolicyis also modified to acceptpolicyTypeCode, allowing the method to take advantage of the new index for improved performance.cc: @flyrain @singhpk234