File 1207-erts-Fix-ets-insert-with-list-into-bag-being-reverse.patch of Package erlang
From 68328c1f89c3622f9f6882f4924207c5cc5c44b2 Mon Sep 17 00:00:00 2001
From: Julian Doherty <madlep@madlep.com>
Date: Sat, 28 Jan 2023 17:27:02 +1300
Subject: [PATCH 1/2] erts: Fix ets:insert with list into bag being reversed
After discussion in https://github.com/erlang/otp/issues/6730
Changed `db_term_list_prepend*` to `db_term_list_append*` so values to
be stored against a key have their order preserved. Previously due to
building the linked list via prepending, the values insert order was
reversed from OTP 23 onward.
---
erts/emulator/beam/erl_db.c | 7 ++++---
erts/emulator/beam/erl_db_catree.c | 2 +-
erts/emulator/beam/erl_db_hash.c | 10 ++++-----
erts/emulator/beam/erl_db_tree.c | 8 +++----
erts/emulator/beam/erl_db_tree_util.h | 2 +-
erts/emulator/beam/erl_db_util.h | 2 +-
lib/stdlib/doc/src/ets.xml | 21 +++++++++++++++++++
lib/stdlib/test/ets_SUITE.erl | 30 ++++++++++++++++++++++++++-
8 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/erts/emulator/beam/erl_db.c b/erts/emulator/beam/erl_db.c
index 9ce88c35d6..392a4b901c 100644
--- a/erts/emulator/beam/erl_db.c
+++ b/erts/emulator/beam/erl_db.c
@@ -1682,17 +1682,18 @@ static void* ets_insert_2_list_copy_term_list(DbTableMethod* meth,
{
void* db_term_list = NULL;
void *term;
+ void *last_term;
Eterm lst;
for (lst = list; is_list(lst); lst = CDR(list_val(lst))) {
term = meth->db_eterm_to_dbterm(compress,
keypos,
CAR(list_val(lst)));
if (db_term_list != NULL) {
- db_term_list =
- meth->db_dbterm_list_prepend(db_term_list,
- term);
+ last_term =
+ meth->db_dbterm_list_append(last_term, term);
} else {
db_term_list = term;
+ last_term = term;
}
}
diff --git a/erts/emulator/beam/erl_db_catree.c b/erts/emulator/beam/erl_db_catree.c
index 3dd8c0b609..035aaaa3da 100644
--- a/erts/emulator/beam/erl_db_catree.c
+++ b/erts/emulator/beam/erl_db_catree.c
@@ -220,7 +220,7 @@ DbTableMethod db_catree =
db_lookup_dbterm_catree,
db_finalize_dbterm_catree,
db_eterm_to_dbterm_tree_common,
- db_dbterm_list_prepend_tree_common,
+ db_dbterm_list_append_tree_common,
db_dbterm_list_remove_first_tree_common,
db_put_dbterm_catree,
db_free_dbterm_tree_common,
diff --git a/erts/emulator/beam/erl_db_hash.c b/erts/emulator/beam/erl_db_hash.c
index 7178d7dc84..89d82cc436 100644
--- a/erts/emulator/beam/erl_db_hash.c
+++ b/erts/emulator/beam/erl_db_hash.c
@@ -726,7 +726,7 @@ db_lookup_dbterm_hash(Process *p, DbTable *tbl, Eterm key, Eterm obj,
static void
db_finalize_dbterm_hash(int cret, DbUpdateHandle* handle);
static void* db_eterm_to_dbterm_hash(int compress, int keypos, Eterm obj);
-static void* db_dbterm_list_prepend_hash(void* list, void* db_term);
+static void* db_dbterm_list_append_hash(void* last_term, void* db_term);
static void* db_dbterm_list_remove_first_hash(void** list);
static int db_put_dbterm_hash(DbTable* tb,
void* obj,
@@ -866,7 +866,7 @@ DbTableMethod db_hash =
db_lookup_dbterm_hash,
db_finalize_dbterm_hash,
db_eterm_to_dbterm_hash,
- db_dbterm_list_prepend_hash,
+ db_dbterm_list_append_hash,
db_dbterm_list_remove_first_hash,
db_put_dbterm_hash,
db_free_dbterm_hash,
@@ -4170,11 +4170,11 @@ static void* db_eterm_to_dbterm_hash(int compress, int keypos, Eterm obj)
return term;
}
-static void* db_dbterm_list_prepend_hash(void* list, void* db_term)
+static void* db_dbterm_list_append_hash(void* last_term, void* db_term)
{
- HashDbTerm* l = list;
+ HashDbTerm* l = last_term;
HashDbTerm* t = db_term;
- t->next = l;
+ l->next = t;
return t;
}
diff --git a/erts/emulator/beam/erl_db_tree.c b/erts/emulator/beam/erl_db_tree.c
index 23b28e47f8..e96c35e9ef 100644
--- a/erts/emulator/beam/erl_db_tree.c
+++ b/erts/emulator/beam/erl_db_tree.c
@@ -519,7 +519,7 @@ DbTableMethod db_tree =
db_lookup_dbterm_tree,
db_finalize_dbterm_tree,
db_eterm_to_dbterm_tree_common,
- db_dbterm_list_prepend_tree_common,
+ db_dbterm_list_append_tree_common,
db_dbterm_list_remove_first_tree_common,
db_put_dbterm_tree,
db_free_dbterm_tree_common,
@@ -3533,11 +3533,11 @@ void* db_eterm_to_dbterm_tree_common(int compress, int keypos, Eterm obj)
return term;
}
-void* db_dbterm_list_prepend_tree_common(void *list, void *db_term)
+void* db_dbterm_list_append_tree_common(void *last_term, void *db_term)
{
- TreeDbTerm* l = list;
+ TreeDbTerm* l = last_term;
TreeDbTerm* t = db_term;
- t->left = l;
+ l->left = t;
return t;
}
diff --git a/erts/emulator/beam/erl_db_tree_util.h b/erts/emulator/beam/erl_db_tree_util.h
index 75c82003b2..7592efb2b8 100644
--- a/erts/emulator/beam/erl_db_tree_util.h
+++ b/erts/emulator/beam/erl_db_tree_util.h
@@ -172,7 +172,7 @@ void db_finalize_dbterm_tree_common(int cret,
TreeDbTerm **root,
DbTableTree *stack_container);
void* db_eterm_to_dbterm_tree_common(int compress, int keypos, Eterm obj);
-void* db_dbterm_list_prepend_tree_common(void* list, void* db_term);
+void* db_dbterm_list_append_tree_common(void* last_term, void* db_term);
void* db_dbterm_list_remove_first_tree_common(void **list);
int db_put_dbterm_tree_common(DbTableCommon *tb, TreeDbTerm **root, TreeDbTerm *value_to_insert,
int key_clash_fail, DbTableTree *stack_container);
diff --git a/erts/emulator/beam/erl_db_util.h b/erts/emulator/beam/erl_db_util.h
index 11f1be017e..1d8261b4ca 100644
--- a/erts/emulator/beam/erl_db_util.h
+++ b/erts/emulator/beam/erl_db_util.h
@@ -237,7 +237,7 @@ typedef struct db_table_method
** not DB_ERROR_NONE, the object is removed from the table. */
void (*db_finalize_dbterm)(int cret, DbUpdateHandle* handle);
void* (*db_eterm_to_dbterm)(int compress, int keypos, Eterm obj);
- void* (*db_dbterm_list_prepend)(void* list, void* db_term);
+ void* (*db_dbterm_list_append)(void* last_term, void* db_term);
void* (*db_dbterm_list_remove_first)(void** list);
int (*db_put_dbterm)(DbTable* tb, /* [in out] */
void* obj,
diff --git a/lib/stdlib/doc/src/ets.xml b/lib/stdlib/doc/src/ets.xml
index 507d27d8d6..005fda8089 100644
--- a/lib/stdlib/doc/src/ets.xml
+++ b/lib/stdlib/doc/src/ets.xml
@@ -825,6 +825,27 @@ Error: fun containing local Erlang function calls
<p>The entire operation is guaranteed to be
<seeerl marker="#concurrency">atomic and isolated</seeerl>,
even when a list of objects is inserted.</p>
+ <marker id="insert_list_order"></marker>
+ <p>
+ For <c>bag</c> and <c>duplicate_bag</c>, objects in the list with
+ identical keys will be inserted in list order (from head to tail). That
+ is, a subsequent call to <seemfa marker="#lookup/2"><c>lookup(T,Key)</c></seemfa>
+ will return them in that inserted order.
+ </p>
+ <note>
+ <p>
+ For <c>bag</c> the insertion order of indentical keys described above was
+ accidentally reverted in OTP 23.0 and later fixed in OTP 25.3. That
+ is, from OTP 23.0 up until OTP 25.3 the objects in a list are
+ inserted in reverse order (from tail to head).
+ </p>
+ <p>
+ For <c>duplicate_bag</c> the same faulty reverse insertion exist
+ from OTP 23.0 until OTP 25.3. However, it is unpredictable and may
+ or may not happen. A longer list will increase the probabiliy of the
+ insertion being done in reverse.
+ </p>
+ </note>
</desc>
</func>
diff --git a/lib/stdlib/test/ets_SUITE.erl b/lib/stdlib/test/ets_SUITE.erl
index 7f63a9903c..4cfc6933c8 100644
--- a/lib/stdlib/test/ets_SUITE.erl
+++ b/lib/stdlib/test/ets_SUITE.erl
@@ -51,7 +51,8 @@
-export([t_insert_list/1, t_insert_list_bag/1, t_insert_list_duplicate_bag/1,
t_insert_list_set/1, t_insert_list_delete_set/1,
t_insert_list_parallel/1, t_insert_list_delete_parallel/1,
- t_insert_list_kill_process/1]).
+ t_insert_list_kill_process/1,
+ t_insert_list_insert_order_preserved/1]).
-export([test_table_size_concurrency/1,test_table_memory_concurrency/1,
test_delete_table_while_size_snapshot/1, test_delete_table_while_size_snapshot_helper/1,
test_decentralized_counters_setting/1]).
@@ -222,7 +223,8 @@ groups() ->
[t_insert_list, t_insert_list_set, t_insert_list_bag,
t_insert_list_duplicate_bag, t_insert_list_delete_set,
t_insert_list_parallel, t_insert_list_delete_parallel,
- t_insert_list_kill_process]}].
+ t_insert_list_kill_process,
+ t_insert_list_insert_order_preserved]}].
init_per_suite(Config) ->
erts_debug:set_internal_state(available_internal_state, true),
@@ -1553,6 +1555,32 @@ t_insert_list_kill_process_do(Opts) ->
fun ets:insert_new/2]],
ok.
+t_insert_list_insert_order_preserved(Config) when is_list(Config) ->
+ insert_list_insert_order_preserved(bag),
+ insert_list_insert_order_preserved(duplicate_bag),
+ ok.
+
+insert_list_insert_order_preserved(Type) ->
+ Tab = ets:new(?FUNCTION_NAME, [Type]),
+ K = a,
+ Values1 = [{K, 1}, {K, 2}, {K, 3}],
+ Values2 = [{K, 4}, {K, 5}, {K, 6}],
+ ets:insert(Tab, Values1),
+ ets:insert(Tab, Values2),
+ [{K, 1}, {K, 2}, {K, 3}, {K, 4}, {K, 5}, {K, 6}] = ets:lookup(Tab, K),
+
+ ets:delete(Tab, K),
+ [] = ets:lookup(Tab, K),
+
+ %% Insert order in duplicate_bag depended on reductions left
+ ITERATIONS_PER_RED = 8,
+ NTuples = 4000 * ITERATIONS_PER_RED + 10,
+ LongList = [{K, V} || V <- lists:seq(1, NTuples)],
+ ets:insert(Tab, LongList),
+ LongList = ets:lookup(Tab, K),
+
+ ets:delete(Tab).
+
%% Test interface of ets:test_ms/2.
t_test_ms(Config) when is_list(Config) ->
EtsMem = etsmem(),
--
2.35.3
