Repository: man-group/ArcticDB (language: python)

Issue:
Dynamic schema allows appending non-matching RangeIndexes
### Describe the bug

When dynamic schema is enabled it is possible to append dataframe with ranged index to symbol even if the ranges are not matching. This makes it possible to
1. Append ranged index with start different than the stop of the symbol
2. Append ranged index with step different than the step of the symbol
Both actions change the index even for the old rows and are forbidden with static_schema.

### Steps/Code to Reproduce

```python
>>> import arcticdb
>>> import pandas as pd
>>> import numpy as np
>>> ac = arcticdb.Arctic("lmdb://test")
>>> opts = arcticdb.LibraryOptions(dynamic_schema=True)
>>> lib = ac.get_library("test", create_if_missing=True, library_options=opts)
>>> lib.write("test", pd.DataFrame({"a": [1, 2, 3]}))
VersionedItem(symbol='test', library='test', data=n/a, version=0, metadata=None, host='LMDB(path=...test)', timestamp=1713515824376569000)
>>> lib.read("test").data
   a
0  1
1  2
2  3
>>> lib.append("test", pd.DataFrame({"a": [1]}, pd.RangeIndex(start=5, stop=6, step=1)))
VersionedItem(symbol='test', library='test', data=n/a, version=1, metadata=None, host='LMDB(path=...test)', timestamp=1713515904432580000)
>>> lib.read("test").data
   a
5  1
6  2
7  3
8  1
>>> lib.append("test", pd.DataFrame({"a": [1]}, pd.RangeIndex(start=5, stop=6, step=10)))
VersionedItem(symbol='test', library='test', data=n/a, version=6, metadata=None, host='LMDB(path=...test)', timestamp=1713517240251682100)
>>> lib.read("test").data
    a
5   1
15  2
25  3
35  1
45  1
```

### Expected Results

Do not allow appending RangeIndex-ed dataframes if the ranges are not matching.

### OS, Python Version and ArcticDB Version

Python: 3.10.11 (tags/v3.10.11:7d4cc5a, Apr 5 2023, 00:38:17) [MSC v.1929 64 bit (AMD64)]
OS: Windows-10-10.0.22631-SP0
ArcticDB: 4.4.0
Numpy: 1.26.3
Pandas: 2.1.4

### Backend storage used

_No response_

### Additional Context

_No response_


Discussion / hints:

Fix patch:
diff --git a/cpp/arcticdb/python/normalization_checks.cpp b/cpp/arcticdb/python/normalization_checks.cpp
index 109c3b4acc..17584bc941 100644
--- a/cpp/arcticdb/python/normalization_checks.cpp
+++ b/cpp/arcticdb/python/normalization_checks.cpp
@@ -122,17 +122,15 @@ void update_rowcount_normalization_data(
             );
 
             size_t new_start = new_index->start();
-            if (new_start != 0) {
-                auto stop = old_index->start() + old_length * old_index->step();
-                normalization::check<ErrorCode::E_INCOMPATIBLE_INDEX>(
-                    new_start == stop,
-                    "The appending data has a RangeIndex.start={} that is not contiguous with the {}"
-                    "stop ({}) of",
-                    error_suffix,
-                    new_start,
-                    stop
-                );
-            }
+            auto stop = old_index->start() + old_length * old_index->step();
+            normalization::check<ErrorCode::E_INCOMPATIBLE_INDEX>(
+                new_start == stop || (new_start == 0 && new_index->step() == 1),
+                "The appending data has a RangeIndex.start={} that is not contiguous with the "
+                "stop ({}) of {}",
+                new_start,
+                stop,
+                error_suffix
+            );
 
             new_pandas->get().mutable_index()->set_start(old_index->start());
         }
diff --git a/cpp/arcticdb/version/schema_checks.hpp b/cpp/arcticdb/version/schema_checks.hpp
index 9b81d54058..28a7e3ed1d 100644
--- a/cpp/arcticdb/version/schema_checks.hpp
+++ b/cpp/arcticdb/version/schema_checks.hpp
@@ -74,9 +74,10 @@ inline void check_normalization_index_match(
             // Having this will not allow appending RowCont indexed pd.DataFrames to DateTime indexed pd.DataFrames because they would
             // have different field size (the rowcount index is not stored as a field). This logic is bug prone and will become better
             // after we enable the empty index.
+            const bool input_frame_is_series = frame.norm_meta.has_series();
             normalization::check<ErrorCode::E_INCOMPATIBLE_INDEX>(
                 common_index_type != IndexDescriptor::UNKNOWN ||
-                    (old_idx_kind == IndexDescriptor::TIMESTAMP && new_idx_kind == IndexDescriptor::ROWCOUNT),
+                    (input_frame_is_series && old_idx_kind == IndexDescriptor::TIMESTAMP && new_idx_kind == IndexDescriptor::ROWCOUNT),
                 "Cannot append {} index to {} index",
                 index_type_to_str(new_idx_kind),
                 index_type_to_str(old_idx_kind)
@@ -121,12 +122,9 @@ inline void fix_descriptor_mismatch_or_throw(
     const auto &old_sd = existing_isr.tsd().as_stream_descriptor();
     check_normalization_index_match(operation, old_sd, new_frame, empty_types);
 
-    if (dynamic_schema)
-        return; // TODO: dynamic schema may need some of the checks as below
-
     fix_normalization_or_throw(operation == APPEND, existing_isr, new_frame);
 
-    if (!columns_match(old_sd, new_frame.desc)) {
+    if (!dynamic_schema && !columns_match(old_sd, new_frame.desc)) {
         throw StreamDescriptorMismatch(
             "The columns (names and types) in the argument are not identical to that of the existing version",
             StreamDescriptor{old_sd},


Test patch:
diff --git a/python/tests/unit/arcticdb/version_store/test_append.py b/python/tests/unit/arcticdb/version_store/test_append.py
index b1aaf988a2..84d6cac6b9 100644
--- a/python/tests/unit/arcticdb/version_store/test_append.py
+++ b/python/tests/unit/arcticdb/version_store/test_append.py
@@ -28,6 +28,39 @@ def test_append_simple(lmdb_version_store):
     assert_frame_equal(vit.data, expected)
 
 
+@pytest.mark.parametrize("empty_types", (True, False))
+@pytest.mark.parametrize("dynamic_schema", (True, False))
+def test_append_range_index(version_store_factory, empty_types, dynamic_schema):
+    lib = version_store_factory(empty_types=empty_types, dynamic_schema=dynamic_schema)
+    sym = "test_append_range_index"
+    df_0 = pd.DataFrame({"col": [0, 1]}, index=pd.RangeIndex(0, 4, 2))
+    lib.write(sym, df_0)
+
+    # Appending another range index following on from what is there should work
+    df_1 = pd.DataFrame({"col": [2, 3]}, index=pd.RangeIndex(4, 8, 2))
+    lib.append(sym, df_1)
+    expected = pd.concat([df_0, df_1])
+    received = lib.read(sym).data
+    assert_frame_equal(expected, received)
+
+    # Appending a range starting earlier or later, or with a different step size, should fail
+    for idx in [pd.RangeIndex(6, 10, 2), pd.RangeIndex(10, 14, 2), pd.RangeIndex(8, 14, 3)]:
+        with pytest.raises(NormalizationException):
+            lib.append(sym, pd.DataFrame({"col": [4, 5]}, index=idx))
+
+
+@pytest.mark.parametrize("empty_types", (True, False))
+@pytest.mark.parametrize("dynamic_schema", (True, False))
+def test_append_range_index_from_zero(version_store_factory, empty_types, dynamic_schema):
+    lib = version_store_factory(empty_types=empty_types, dynamic_schema=dynamic_schema)
+    sym = "test_append_range_index_from_zero"
+    df_0 = pd.DataFrame({"col": [0, 1]}, index=pd.RangeIndex(-6, -2, 2))
+    lib.write(sym, df_0)
+
+    with pytest.raises(NormalizationException):
+        lib.append(sym, pd.DataFrame({"col": [2, 3]}, index=pd.RangeIndex(0, 4, 2)))
+
+
 def test_append_indexed(s3_version_store):
     symbol = "test_append_simple"
     idx1 = np.arange(0, 10)
diff --git a/python/tests/unit/arcticdb/version_store/test_empty_column_type.py b/python/tests/unit/arcticdb/version_store/test_empty_column_type.py
index d15a406e36..0fa96ac2b2 100644
--- a/python/tests/unit/arcticdb/version_store/test_empty_column_type.py
+++ b/python/tests/unit/arcticdb/version_store/test_empty_column_type.py
@@ -13,7 +13,7 @@
 import pytest
 from packaging.version import Version
 from arcticdb.util._versions import PANDAS_VERSION
-import arcticdb
+from arcticdb_ext.exceptions import NormalizationException
 
 class DtypeGenerator:
     """
@@ -861,14 +861,9 @@ def test_no_cols(self, lmdb_version_store_static_and_dynamic):
         assert result.index.equals(pd.DatetimeIndex([]))
 
     def test_has_a_column(self, lmdb_version_store_static_and_dynamic):
-        if self.is_dynamic_schema(lmdb_version_store_static_and_dynamic):
-            pytest.xfail(
-                """In case of empty symbols dynamic schema allows appending indexes of different types.
-                See https://github.com/man-group/ArcticDB/issues/1507. This is fixed with the empty index type."""
-            )
         result = self.roundtrip(pd.DataFrame({"a": []}), lmdb_version_store_static_and_dynamic)
         assert result.index.equals(pd.DatetimeIndex([]))
-        with pytest.raises(Exception):
+        with pytest.raises(NormalizationException):
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": [1.0]}))
         to_append_successfuly = pd.DataFrame({"a": [1.0]}, index=pd.DatetimeIndex(["01/01/2024"])) 
         lmdb_version_store_static_and_dynamic.append(self.sym(), to_append_successfuly)
@@ -882,11 +877,6 @@ def test_explicit_row_range_no_columns(self, lmdb_version_store_static_and_dynam
         assert result.index.equals(pd.DatetimeIndex([]))
 
     def test_explicit_row_range_with_columns(self, lmdb_version_store_static_and_dynamic):
-        if self.is_dynamic_schema(lmdb_version_store_static_and_dynamic):
-            pytest.xfail(
-                """In case of empty symbols dynamic schema allows appending indexes of different types.
-                See https://github.com/man-group/ArcticDB/issues/1507. This is fixed with the empty index type."""
-            )
         result = self.roundtrip(pd.DataFrame({"a": []}, index=pd.RangeIndex(start=5, stop=5, step=100)), lmdb_version_store_static_and_dynamic)
         assert result.index.equals(pd.DatetimeIndex([]))
         with pytest.raises(Exception):
@@ -938,7 +928,7 @@ def test_no_cols(self, lmdb_version_store_static_and_dynamic):
     def test_has_a_column(self, lmdb_version_store_static_and_dynamic):
         result = self.roundtrip(pd.DataFrame({"a": []}), lmdb_version_store_static_and_dynamic)
         assert result.index.equals(pd.RangeIndex(start=0, stop=0, step=1))
-        with pytest.raises(arcticdb.exceptions.NormalizationException):
+        with pytest.raises(NormalizationException):
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": ["a"]}, index=pd.DatetimeIndex(["01/01/2024"])))
         to_append_successfuly = pd.DataFrame({"a": ["a"]})
         lmdb_version_store_static_and_dynamic.append(self.sym(), to_append_successfuly)
@@ -949,22 +939,17 @@ def test_explicit_row_range_no_columns(self, lmdb_version_store_static_and_dynam
         assert result.index.equals(pd.RangeIndex(start=0, stop=0, step=1))
 
     def test_explicit_row_range_with_columns(self, lmdb_version_store_static_and_dynamic):
-        if self.is_dynamic_schema(lmdb_version_store_static_and_dynamic):
-            pytest.xfail(
-                """Dynamic schema should not allow appending missmatching range indexes. See issue:
-                https://github.com/man-group/ArcticDB/issues/1509"""
-            )
         result = self.roundtrip(pd.DataFrame({"a": []}, index=pd.RangeIndex(start=5, stop=5, step=100)), lmdb_version_store_static_and_dynamic)
         assert result.index.equals(pd.RangeIndex(start=5, stop=5, step=100))
         # Cannot append datetime indexed df to empty rowrange index
-        with pytest.raises(arcticdb.exceptions.NormalizationException):
+        with pytest.raises(NormalizationException):
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": ["a"]}, index=pd.DatetimeIndex(["01/01/2024"])))
         # Cannot append rowrange indexed df if the start of the appended is not matching the stop of the empty
-        with pytest.raises(arcticdb.exceptions.NormalizationException):
+        with pytest.raises(NormalizationException):
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": ["a"]}, index=pd.RangeIndex(start=9, stop=109, step=100)))
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": ["a"]}, index=pd.RangeIndex(start=10, stop=110, step=100)))
         # Cannot append rowrange indexed df if the step is different
-        with pytest.raises(arcticdb.exceptions.NormalizationException):
+        with pytest.raises(NormalizationException):
             lmdb_version_store_static_and_dynamic.append(self.sym(), pd.DataFrame({"a": ["a"]}, index=pd.RangeIndex(start=5, stop=6, step=1)))
         to_append_successfuly = pd.DataFrame({"a": ["a"]}, index=pd.RangeIndex(start=5, stop=105, step=100))
         lmdb_version_store_static_and_dynamic.append(self.sym(), to_append_successfuly)
diff --git a/python/tests/unit/arcticdb/version_store/test_filtering_dynamic.py b/python/tests/unit/arcticdb/version_store/test_filtering_dynamic.py
index e762ba0040..6715e98a78 100644
--- a/python/tests/unit/arcticdb/version_store/test_filtering_dynamic.py
+++ b/python/tests/unit/arcticdb/version_store/test_filtering_dynamic.py
@@ -37,6 +37,7 @@
 
 
 def generic_dynamic_filter_test(version_store, symbol, df, arctic_query, pandas_query, dynamic_strings=True):
+    version_store.delete(symbol)
     expected, slices = make_dynamic(df)
     for df_slice in slices:
         version_store.append(symbol, df_slice, write_if_missing=True)