fix device-node cache aliasing null tag with literal string "null"

TsFileIOReader::get_cached_device_node keyed device_node_cache_ by
IDeviceID::get_device_name(), which renders a null tag segment as the
literal text "null".  A device with a real null tag (e.g. tags
(null, b, c)) and a device whose tag value is the string "null"
(("null", b, c)) therefore produce the identical name "a.null.b.c" and
collide in the cache: whichever device is queried first populates the
entry, and every later query for the other device on the same reused
reader gets the first device's cached MetaIndexNode and silently reads
its chunks.

This surfaced through the Python dataset API, where one long-lived
TsFileReader answers many per-device queries: the pytest
test_dataset_null_tag_positions_and_string_null_are_distinct read
(null,b,c)'s data for the ("null",b,c) device.  The device metadata
binary search (DeviceIDComparable, segment-based) was always correct;
only the string cache key was lossy.

Key the cache by a collision-free, length-prefixed encoding of the
segment vector that flags null segments explicitly, so a null tag can
never alias the string "null".  Add a device_id unit test pinning the
invariant (names collide, segment equality distinguishes them).

Verified: C++ suite 707/707 (Release+ASan+UBSan), Python suite 150/150.

Author: ColinLeeo

cpp/src/file/tsfile_io_reader.cc

@@ -384,10 +384,29 @@ int TsFileIOReader::load_tsfile_meta() {
     return ret;
 }
 
+std::string TsFileIOReader::device_node_cache_key(
+    const std::shared_ptr<IDeviceID>& device_id) {
+    // Length-prefixed, null-flagged encoding: for each segment emit either
+    // "N;" (null) or "<len>:<bytes>;".  Distinct segment sequences always map
+    // to distinct keys, so a real null tag never aliases the literal "null".
+    std::string key;
+    for (const std::string* seg : device_id->get_segments()) {
+        if (seg == nullptr) {
+            key += "N;";
+        } else {
+            key += std::to_string(seg->size());
+            key += ':';
+            key += *seg;
+            key += ';';
+        }
+    }
+    return key;
+}
+
 int TsFileIOReader::get_cached_device_node(std::shared_ptr<IDeviceID> device_id,
                                            common::PageArena& pa,
                                            CachedDeviceNode& out) {
-    std::string dev_name = device_id->get_device_name();
+    std::string dev_name = device_node_cache_key(device_id);
 
     {
         std::lock_guard<std::mutex> lk(device_node_cache_mu_);

cpp/src/file/tsfile_io_reader.h

@@ -195,6 +195,15 @@ class TsFileIOReader {
                                common::PageArena& pa, CachedDeviceNode& out);
 
    private:
+    // Build a collision-free key for device_node_cache_.  get_device_name()
+    // renders a null tag segment as the literal "null", so a device with a
+    // real null tag and one whose tag value is the string "null" produce the
+    // same name and would alias in the cache — the second device would read
+    // the first device's chunks.  Encode each segment length-prefixed and
+    // flag null segments explicitly so the two can never collide.
+    static std::string device_node_cache_key(
+        const std::shared_ptr<IDeviceID>& device_id);
+
     ReadFile* read_file_;
     common::PageArena tsfile_meta_page_arena_;
     TsFileMeta tsfile_meta_;

cpp/test/common/device_id_test.cc

@@ -71,4 +71,32 @@ TEST(DeviceIdTest, TabletDeviceId) {
     ASSERT_EQ("test_device0.null.t2.t3",
               tablet.get_device_id(2)->get_device_name());
 }
+
+// Regression: a device whose first tag is a real null and a device whose first
+// tag is the literal string "null" render to the SAME get_device_name()
+// ("t.null.b"), so anything that keys a per-device map/cache by the device name
+// aliases the two — the second device silently reads the first device's chunks.
+// The device-node cache in TsFileIOReader hit exactly this, conflating the two
+// devices' data on a reused reader.  The reliable discriminator is the segment
+// vector (operator==), which keeps nullptr distinct from the string "null".
+TEST(DeviceIdTest, NullTagVsLiteralNullAreDistinct) {
+    // Real null first tag: segment pointer is nullptr.
+    std::vector<std::string*> null_first_segs{new std::string("t"), nullptr,
+                                              new std::string("b")};
+    StringArrayDeviceID null_first(null_first_segs);
+    for (auto* s : null_first_segs) delete s;
+
+    // Literal string "null" as the first tag value.
+    StringArrayDeviceID literal_null(
+        std::vector<std::string>({"t", "null", "b"}));
+
+    // The names collide — this is the trap the cache used to fall into.
+    ASSERT_EQ(null_first.get_device_name(), literal_null.get_device_name());
+    ASSERT_EQ("t.null.b", null_first.get_device_name());
+
+    // But the devices are genuinely different, and the segment-based equality
+    // used by DeviceIDComparable / the cache key must reflect that.
+    ASSERT_FALSE(null_first == literal_null);
+    ASSERT_TRUE(null_first != literal_null);
+}
 }  // namespace storage