eric-maynard commented on code in PR #1959:
URL: https://github.com/apache/polaris/pull/1959#discussion_r2196442870
##########
polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java:
##########
@@ -34,11 +35,12 @@ public class BehaviorChangeConfiguration<T> extends
PolarisConfiguration<T> {
protected BehaviorChangeConfiguration(
String key,
Review Comment:
> I disagree with this:
> > It seems reasonable to expect that multiple different entries could be
used to configure the same feature.
Your current PR has this functionality... except only partially:
| Keys | Legacy Keys | OK? |
| ------------- | ------------- | --- |
| 0 | 0 | Yes |
| 0 | 1 | Yes |
| 0 | 2+ | Yes |
| 1 | 0 | Yes |
| 1 | 1 | Yes |
| 1 | 2+ | Yes |
| 2 | 0 | No |
| 2 | 1 | No |
| 2 | 2+ | No |
What is the rationale behind this asymmetry? If, as stated before, it is
because the key is thought of as a primary key then this is just not correct.
You can write a valid config that violates the "primary key constraint" by
duplicating an entry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
