Merge pull request #41431 from ClickHouse/remove-method-simplification

Remove `-WithTerminatingZero` methods

programs/client/Client.cpp

@@ -150,7 +150,7 @@ std::vector<String> Client::loadWarningMessages()
 
                     size_t rows = packet.block.rows();
                     for (size_t i = 0; i < rows; ++i)
-                        messages.emplace_back(column.getDataAt(i).toString());
+                        messages.emplace_back(column[i].get<String>());
                 }
                 continue;
 

programs/library-bridge/CatBoostLibraryHandler.cpp

@@ -98,7 +98,7 @@ void placeStringColumn(const ColumnString & column, const char ** buffer, size_t
     size_t size = column.size();
     for (size_t i = 0; i < size; ++i)
     {
-        *buffer = const_cast<char *>(column.getDataAtWithTerminatingZero(i).data);
+        *buffer = const_cast<char *>(column.getDataAt(i).data);
         buffer += features_count;
     }
 }

src/AggregateFunctions/AggregateFunctionMinMaxAny.h

@@ -492,7 +492,7 @@ public:
     void insertResultInto(IColumn & to) const
     {
         if (has())
-            assert_cast<ColumnString &>(to).insertDataWithTerminatingZero(getData(), size);
+            assert_cast<ColumnString &>(to).insertData(getData(), size);
         else
             assert_cast<ColumnString &>(to).insertDefault();
     }
@@ -569,7 +569,7 @@ public:
 
     void change(const IColumn & column, size_t row_num, Arena * arena)
     {
-        changeImpl(assert_cast<const ColumnString &>(column).getDataAtWithTerminatingZero(row_num), arena);
+        changeImpl(assert_cast<const ColumnString &>(column).getDataAt(row_num), arena);
     }
 
     void change(const Self & to, Arena * arena)
@@ -618,7 +618,7 @@ public:
 
     bool changeIfLess(const IColumn & column, size_t row_num, Arena * arena)
     {
-        if (!has() || assert_cast<const ColumnString &>(column).getDataAtWithTerminatingZero(row_num) < getStringRef())
+        if (!has() || assert_cast<const ColumnString &>(column).getDataAt(row_num) < getStringRef())
         {
             change(column, row_num, arena);
             return true;
@@ -640,7 +640,7 @@ public:
 
     bool changeIfGreater(const IColumn & column, size_t row_num, Arena * arena)
     {
-        if (!has() || assert_cast<const ColumnString &>(column).getDataAtWithTerminatingZero(row_num) > getStringRef())
+        if (!has() || assert_cast<const ColumnString &>(column).getDataAt(row_num) > getStringRef())
         {
             change(column, row_num, arena);
             return true;
@@ -667,7 +667,7 @@ public:
 
     bool isEqualTo(const IColumn & column, size_t row_num) const
     {
-        return has() && assert_cast<const ColumnString &>(column).getDataAtWithTerminatingZero(row_num) == getStringRef();
+        return has() && assert_cast<const ColumnString &>(column).getDataAt(row_num) == getStringRef();
     }
 
     static bool allocatesMemoryInArena()

src/Client/Suggest.cpp

@@ -187,9 +187,8 @@ void Suggest::fillWordsFromBlock(const Block & block)
     Words new_words;
     new_words.reserve(rows);
     for (size_t i = 0; i < rows; ++i)
-    {
-        new_words.emplace_back(column.getDataAt(i).toString());
-    }
+        new_words.emplace_back(column[i].get<String>());
+
     addWords(std::move(new_words));
 }
 

src/Columns/ColumnArray.cpp

@@ -151,23 +151,24 @@ void ColumnArray::get(size_t n, Field & res) const
 
 StringRef ColumnArray::getDataAt(size_t n) const
 {
+    assert(n < size());
+
     /** Returns the range of memory that covers all elements of the array.
       * Works for arrays of fixed length values.
-      * For arrays of strings and arrays of arrays, the resulting chunk of memory may not be one-to-one correspondence with the elements,
-      *  since it contains only the data laid in succession, but not the offsets.
       */
 
-    size_t offset_of_first_elem = offsetAt(n);
-    StringRef first = getData().getDataAtWithTerminatingZero(offset_of_first_elem);
+    /// We are using pointer arithmetic on the addresses of the array elements.
+    if (!data->isFixedAndContiguous())
+        throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method getDataAt is not supported for {}", getName());
 
     size_t array_size = sizeAt(n);
     if (array_size == 0)
-        return StringRef(first.data, 0);
+        return StringRef(nullptr, 0);
 
-    size_t offset_of_last_elem = getOffsets()[n] - 1;
-    StringRef last = getData().getDataAtWithTerminatingZero(offset_of_last_elem);
+    size_t offset_of_first_elem = offsetAt(n);
+    StringRef first = getData().getDataAt(offset_of_first_elem);
 
-    return StringRef(first.data, last.data + last.size - first.data);
+    return StringRef(first.data, first.size * array_size);
 }
 
 
@@ -183,7 +184,7 @@ void ColumnArray::insertData(const char * pos, size_t length)
     /** Similarly - only for arrays of fixed length values.
       */
     if (!data->isFixedAndContiguous())
-        throw Exception("Method insertData is not supported for " + getName(), ErrorCodes::NOT_IMPLEMENTED);
+        throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method insertData is not supported for {}", getName());
 
     size_t field_size = data->sizeOfValueIfFixed();
 

src/Columns/ColumnConst.h

@@ -81,11 +81,6 @@ public:
         return data->getDataAt(0);
     }
 
-    StringRef getDataAtWithTerminatingZero(size_t) const override
-    {
-        return data->getDataAtWithTerminatingZero(0);
-    }
-
     UInt64 get64(size_t) const override
     {
         return data->get64(0);

src/Columns/ColumnLowCardinality.h

@@ -59,10 +59,6 @@ public:
     void get(size_t n, Field & res) const override { getDictionary().get(getIndexes().getUInt(n), res); }
 
     StringRef getDataAt(size_t n) const override { return getDictionary().getDataAt(getIndexes().getUInt(n)); }
-    StringRef getDataAtWithTerminatingZero(size_t n) const override
-    {
-        return getDictionary().getDataAtWithTerminatingZero(getIndexes().getUInt(n));
-    }
 
     bool isDefaultAt(size_t n) const override { return getDictionary().isDefaultAt(getIndexes().getUInt(n)); }
     UInt64 get64(size_t n) const override { return getDictionary().get64(getIndexes().getUInt(n)); }

src/Columns/ColumnString.h

@@ -108,12 +108,6 @@ public:
         return StringRef(&chars[offsetAt(n)], sizeAt(n) - 1);
     }
 
-    StringRef getDataAtWithTerminatingZero(size_t n) const override
-    {
-        assert(n < size());
-        return StringRef(&chars[offsetAt(n)], sizeAt(n));
-    }
-
     bool isDefaultAt(size_t n) const override
     {
         assert(n < size());
@@ -177,17 +171,6 @@ public:
         offsets.push_back(new_size);
     }
 
-    /// Like getData, but inserting data should be zero-ending (i.e. length is 1 byte greater than real string size).
-    void insertDataWithTerminatingZero(const char * pos, size_t length)
-    {
-        const size_t old_size = chars.size();
-        const size_t new_size = old_size + length;
-
-        chars.resize(new_size);
-        memcpy(chars.data() + old_size, pos, length);
-        offsets.push_back(new_size);
-    }
-
     void popBack(size_t n) override
     {
         size_t nested_n = offsets.back() - offsetAt(offsets.size() - n);

src/Columns/ColumnUnique.h

@@ -70,10 +70,6 @@ public:
     void get(size_t n, Field & res) const override { getNestedColumn()->get(n, res); }
     bool isDefaultAt(size_t n) const override { return n == 0; }
     StringRef getDataAt(size_t n) const override { return getNestedColumn()->getDataAt(n); }
-    StringRef getDataAtWithTerminatingZero(size_t n) const override
-    {
-        return getNestedColumn()->getDataAtWithTerminatingZero(n);
-    }
     UInt64 get64(size_t n) const override { return getNestedColumn()->get64(n); }
     UInt64 getUInt(size_t n) const override { return getNestedColumn()->getUInt(n); }
     Int64 getInt(size_t n) const override { return getNestedColumn()->getInt(n); }

src/Columns/IColumn.h

@@ -106,13 +106,6 @@ public:
     /// Is used to optimize some computations (in aggregation, for example).
     [[nodiscard]] virtual StringRef getDataAt(size_t n) const = 0;
 
-    /// Like getData, but has special behavior for columns that contain variable-length strings.
-    /// Returns zero-ending memory chunk (i.e. its size is 1 byte longer).
-    [[nodiscard]] virtual StringRef getDataAtWithTerminatingZero(size_t n) const
-    {
-        return getDataAt(n);
-    }
-
     /// If column stores integers, it returns n-th element transformed to UInt64 using static_cast.
     /// If column stores floating point numbers, bits of n-th elements are copied to lower bits of UInt64, the remaining bits are zeros.
     /// Is used to optimize some computations (in aggregation, for example).

src/Functions/FunctionsEmbeddedDictionaries.h

@@ -649,7 +649,7 @@ public:
             for (unsigned int region_id : region_ids)
             {
                 const StringRef & name_ref = dict.getRegionName(region_id, language);
-                col_to->insertDataWithTerminatingZero(name_ref.data, name_ref.size + 1);
+                col_to->insertData(name_ref.data, name_ref.size);
             }
 
             return col_to;

src/Functions/addressToSymbol.cpp

@@ -83,7 +83,7 @@ public:
         for (size_t i = 0; i < input_rows_count; ++i)
         {
             if (const auto * symbol = symbol_index.findSymbol(reinterpret_cast<const void *>(data[i])))
-                result_column->insertDataWithTerminatingZero(symbol->name, strlen(symbol->name) + 1);
+                result_column->insertData(symbol->name, strlen(symbol->name));
             else
                 result_column->insertDefault();
         }

src/Functions/demange.cpp

@@ -78,15 +78,15 @@ public:
 
         for (size_t i = 0; i < input_rows_count; ++i)
         {
-            StringRef source = column_concrete->getDataAtWithTerminatingZero(i);
+            StringRef source = column_concrete->getDataAt(i);
             auto demangled = tryDemangle(source.data);
             if (demangled)
             {
-                result_column->insertDataWithTerminatingZero(demangled.get(), strlen(demangled.get()) + 1);
+                result_column->insertData(demangled.get(), strlen(demangled.get()));
             }
             else
             {
-                result_column->insertDataWithTerminatingZero(source.data, source.size);
+                result_column->insertData(source.data, source.size);
             }
         }
 
@@ -102,4 +102,3 @@ REGISTER_FUNCTION(Demangle)
 }
 
 }
-

src/Processors/Transforms/MongoDBSource.cpp

@@ -250,13 +250,13 @@ namespace
                 if (value.type() == Poco::MongoDB::ElementTraits<ObjectId::Ptr>::TypeId)
                 {
                     std::string string_id = value.toString();
-                    assert_cast<ColumnString &>(column).insertDataWithTerminatingZero(string_id.data(), string_id.size() + 1);
+                    assert_cast<ColumnString &>(column).insertData(string_id.data(), string_id.size());
                     break;
                 }
                 else if (value.type() == Poco::MongoDB::ElementTraits<String>::TypeId)
                 {
                     String string = static_cast<const Poco::MongoDB::ConcreteElement<String> &>(value).value();
-                    assert_cast<ColumnString &>(column).insertDataWithTerminatingZero(string.data(), string.size() + 1);
+                    assert_cast<ColumnString &>(column).insertData(string.data(), string.size());
                     break;
                 }
 

tests/queries/0_stateless/02423_multidimensional_array_get_data_at.sql

@@ -0,0 +1,7 @@
+SELECT formatRow('RawBLOB', [[[33]], []]); -- { serverError 48 }
+SELECT formatRow('RawBLOB', [[[]], []]); -- { serverError 48 }
+SELECT formatRow('RawBLOB', [[[[[[[0x48, 0x65, 0x6c, 0x6c, 0x6f]]]]]], []]); -- { serverError 48 }
+SELECT formatRow('RawBLOB', []::Array(Array(Nothing))); -- { serverError 48 }
+SELECT formatRow('RawBLOB', [[], [['Hello']]]); -- { serverError 48 }
+SELECT formatRow('RawBLOB', [[['World']], []]); -- { serverError 48 }
+SELECT formatRow('RawBLOB', []::Array(String)); -- { serverError 48 }