nickva commented on code in PR #5611:
URL: https://github.com/apache/couchdb/pull/5611#discussion_r2248680667


##########
src/couch/src/couch_bt_engine_header.erl:
##########
@@ -204,17 +204,16 @@ upgrade_tuple(Old) when is_record(Old, db_header) ->
     Old;
 upgrade_tuple(Old) when is_tuple(Old) ->
     NewSize = record_info(size, db_header),
-    if
-        tuple_size(Old) < NewSize -> ok;
-        true -> erlang:error({invalid_header_size, Old})
-    end,
-    {_, New} = lists:foldl(
-        fun(Val, {Idx, Hdr}) ->
-            {Idx + 1, setelement(Idx, Hdr, Val)}
+    Upgrade = tuple_size(Old) < NewSize,
+    ProhibitDowngrade = config:get_boolean("couchdb", "prohibit_downgrade", 
true),
+    OldKVs =
+        case {Upgrade, ProhibitDowngrade} of
+            {true, AnyBool} when is_boolean(AnyBool) -> tuple_to_list(Old);
+            {false, true} -> error({invalid_header_size, Old});
+            {false, false} -> lists:sublist(tuple_to_list(Old), NewSize)

Review Comment:
   Good point, currently this is only meant to work as a counterpart to
   
   ```
   % As long the changes are limited to new header fields (with inline
   % defaults) added to the end of the record, then there is no need to 
increment
   % the disk revision number.
   ```
   
   In other words, this doesn't apply when disk version changes. It only 
applies when the disk version matches and it's just filed fields appended to 
the end of the header.
   
   > The downgrade in general is not solvable problem if we don't maintain 
fields order for a previous version.
   
   Agree, that's why we don't attempt to solve a more general problem just a 
specific corner case. 



-- 
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: notifications-unsubscr...@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to