poojanilangekar commented on code in PR #1959:
URL: https://github.com/apache/polaris/pull/1959#discussion_r2195600057


##########
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 don't think we support multiple invalid keys. We track multiple `legacy` 
keys. 
   
   From a semantic perspective, a `PrimaryKey` in a RDBMS, a `key`in a 
key-value store or a Java Map (or any other programming language) maps to a 
single row, value or entity. In all these examples, two keys never map to the 
same entity. 
   
   What we have with the `legacyKeys` is a way to track historic versions of 
the key-value pair. In any DBMS or KV store that supports multi-versioning, 
it's pretty normal to have multiple entries (multiple versions) for a single 
key. When the primaryKey/key changes, the DBMS/KV store adds a tombstone for 
the old record and creates a new record with the new key.  While changing keys 
in PolarisConfig, we are essentially creating  a new key-value pair in itself 
but its nice to track the older key, hence the support for legacy keys. 
Essentially one could rewrite the entire PolarisConfig class with a Map where 
the `key` is the active key and the value is everything else. In which case, 
we'd track the `legacyKeys` as a part of the value. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to