25.8 Stable: Bump to 25.8.24

cmake/autogenerated_versions.txt

@@ -2,13 +2,13 @@
 
 # NOTE: VERSION_REVISION has nothing common with DBMS_TCP_PROTOCOL_VERSION,
 # only DBMS_TCP_PROTOCOL_VERSION should be incremented on protocol changes.
-SET(VERSION_REVISION 54523)
+SET(VERSION_REVISION 54524)
 SET(VERSION_MAJOR 25)
 SET(VERSION_MINOR 8)
-SET(VERSION_PATCH 23)
-SET(VERSION_GITHASH 6d29497525664acca46a1d1cd0d5787e9ad0857d)
-SET(VERSION_DESCRIBE v25.8.23.10001.altinitytest)
-SET(VERSION_STRING 25.8.23.10001.altinitytest)
+SET(VERSION_PATCH 24)
+SET(VERSION_GITHASH 443620d152ed64184d6f221c988c569334311055)
+SET(VERSION_DESCRIBE v25.8.24.10001.altinitytest)
+SET(VERSION_STRING 25.8.24.10001.altinitytest)
 # end of autochange
 
 SET(VERSION_TWEAK 10001)

contrib/libarchive-cmake/config.h

@@ -1031,6 +1031,9 @@ typedef uint64_t uintmax_t;
 /* Define to 1 if you have the <stdint.h> header file. */
 #define HAVE_STDINT_H 1
 
+/* Define to 1 if you have the <stdio.h> header file. */
+#define HAVE_STDIO_H 1
+
 /* Define to 1 if you have the <stdlib.h> header file. */
 #define HAVE_STDLIB_H 1
 

src/AggregateFunctions/QuantileTiming.h

@@ -391,6 +391,8 @@ namespace detail
                     readBinaryLittleEndian(index, buf);
                     if (index == BIG_THRESHOLD)
                         break;
+                    if (index - SMALL_THRESHOLD >= BIG_SIZE)
+                        throw Exception(ErrorCodes::INCORRECT_DATA, "Incorrect index {} in 'large' kind of quantileTiming deserialization", index);
 
                     UInt64 elem_count = 0;
                     readBinaryLittleEndian(elem_count, buf);

src/Columns/ColumnObject.cpp

@@ -531,6 +531,7 @@ bool ColumnObject::tryInsert(const Field & x)
         for (const auto & path : new_dynamic_paths)
         {
             dynamic_paths_ptrs.erase(path);
+            sorted_dynamic_paths.erase(path);
             dynamic_paths.erase(path);
         }
 
@@ -566,6 +567,7 @@ bool ColumnObject::tryInsert(const Field & x)
         }
         else if (auto * dynamic_path_column = tryToAddNewDynamicPath(path))
         {
+            new_dynamic_paths.insert(String(path));
             if (!dynamic_path_column->tryInsert(value_field))
             {
                 restore_sizes();

src/Columns/tests/gtest_column_object.cpp

@@ -444,3 +444,50 @@ TEST(ColumnObject, RepairDuplicatesInDynamicPathsAndSharedData)
     ASSERT_EQ((*column_object)[2], (Object{{"b", Field(1u)}, {"c", Field(1u)}, {"d", Field(1u)}}));
     ASSERT_EQ((*column_object)[3], (Object{{"d", Field(1u)}}));
 }
+
+TEST(ColumnObject, TryInsertRestoresSortedDynamicPaths)
+{
+    /// "b" is a typed UInt32 path; everything else becomes a dynamic path.
+    auto type = DataTypeFactory::instance().get("JSON(max_dynamic_types=10, max_dynamic_paths=10, b UInt32)");
+    auto col = type->createColumn();
+    auto & col_object = assert_cast<ColumnObject &>(*col);
+    const auto & dynamic_paths = col_object.getDynamicPaths();
+
+    /// One valid row so we have something to serialize later.
+    col_object.insert(Object{{"b", Field{5u}}});
+    ASSERT_EQ(col_object.size(), 1u);
+    ASSERT_EQ(dynamic_paths.size(), 0u);
+
+    /// tryInsert with Object{"a_new": 1, "b": "not_a_number"}.
+    /// Fields are processed in alphabetical order ("a_new" before "b"), so:
+    ///   1. "a_new" is new → tryToAddNewDynamicPath succeeds, "a_new" is added to all
+    ///      three structures including sorted_dynamic_paths.
+    ///   2. ColumnDynamic::tryInsert(1u) for "a_new" succeeds.
+    ///   3. ColumnUInt32::tryInsert(String) for "b" returns false.
+    ///   4. restore_sizes() is called.
+    ///      BUG 1: new_dynamic_paths was never populated, so the loop that removes
+    ///             newly-added paths is a no-op — "a_new" is left in dynamic_paths,
+    ///             dynamic_paths_ptrs, and sorted_dynamic_paths with a rolled-back size.
+    ///      BUG 2: even after populating new_dynamic_paths, the old code erased from
+    ///             dynamic_paths before sorted_dynamic_paths, leaving a dangling view.
+    ///      FIX:   record the path in new_dynamic_paths immediately after
+    ///             tryToAddNewDynamicPath succeeds, and erase sorted_dynamic_paths
+    ///             before dynamic_paths.
+    bool result = col_object.tryInsert(Object{{"a_new", Field{1u}}, {"b", Field{String("not_a_number")}}});
+    ASSERT_FALSE(result);
+
+    ASSERT_EQ(col_object.size(), 1u);
+    ASSERT_EQ(dynamic_paths.size(), 0u);   /// "a_new" must be fully rolled back
+
+    /// serializeValueIntoArena iterates sorted_dynamic_paths.
+    /// Without the fix the stale "a_new" entry causes undefined behavior here (see comment above).
+    Arena arena;
+    const char * begin = nullptr;
+    auto ref = col_object.serializeValueIntoArena(0, arena, begin, nullptr);
+
+    /// Round-trip sanity check.
+    ReadBufferFromMemory buf(ref.data, ref.size);
+    col_object.deserializeAndInsertFromArena(buf, nullptr);
+    ASSERT_EQ(col_object.size(), 2u);
+    ASSERT_EQ(col_object[1], col_object[0]);
+}

src/Coordination/KeeperStorage.cpp

@@ -1789,8 +1789,7 @@ std::list<KeeperStorageBase::Delta> preprocess(
     ProfileEvents::increment(ProfileEvents::KeeperGetRequest);
 
     if (zk_request.path == Coordination::keeper_api_feature_flags_path
-        || zk_request.path == Coordination::keeper_config_path
-        || zk_request.path == Coordination::keeper_availability_zone_path)
+        || zk_request.path == Coordination::keeper_config_path)
         return {};
 
     if (!storage.uncommitted_state.getNode(zk_request.path))

src/Core/PostgreSQL/insertPostgreSQLValue.cpp

@@ -149,6 +149,9 @@ void insertPostgreSQLValue(
                 {
                     max_dimension = std::max(max_dimension, dimension);
 
+                    if (dimension == 0)
+                        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unexpected array closing bracket");
+
                     --dimension;
                     if (dimension == 0)
                         break;

src/Core/ServerSettings.cpp

@@ -1283,7 +1283,7 @@ void ServerSettings::dumpToSystemServerSettingsColumns(ServerSettingColumnsParam
             {"mark_cache_size", {std::to_string(context->getMarkCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
             {"uncompressed_cache_size", {std::to_string(context->getUncompressedCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
             {"index_mark_cache_size", {std::to_string(context->getIndexMarkCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
-            {"index_uncompressed_cache_size", {std::to_string(context->getIndexUncompressedCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
+            {"index_uncompressed_cache_size", {std::to_string(context->getIndexUncompressedCache(/*only_if_enabled=*/ false)->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
             {"mmap_cache_size", {std::to_string(context->getMMappedFileCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
             {"query_condition_cache_size", {std::to_string(context->getQueryConditionCache()->maxSizeInBytes()), ChangeableWithoutRestart::Yes}},
 

src/Core/tests/gtest_insertPostgreSQLValue.cpp

@@ -0,0 +1,96 @@
+#include "config.h"
+
+#if USE_LIBPQXX
+
+#include <gtest/gtest.h>
+
+#include <Core/PostgreSQL/insertPostgreSQLValue.h>
+#include <Core/ExternalResultDescription.h>
+#include <Columns/ColumnArray.h>
+#include <Columns/ColumnsNumber.h>
+#include <DataTypes/DataTypeArray.h>
+#include <DataTypes/DataTypesNumber.h>
+
+
+using namespace DB;
+
+namespace DB::ErrorCodes
+{
+    extern const int BAD_ARGUMENTS;
+}
+
+/// Regression test for dimension underflow in PostgreSQL array parser.
+/// When pqxx::array_parser emits row_end before any row_start (e.g. malformed
+/// input starting with '}'), the dimension counter must not underflow from 0.
+/// See https://github.com/ClickHouse/clickhouse-core-incidents/issues/1693
+
+TEST(InsertPostgreSQLValue, MalformedArrayClosingBracketThrows)
+{
+    auto nested_type = std::make_shared<DataTypeInt32>();
+    auto array_type = std::make_shared<DataTypeArray>(nested_type);
+    auto column = ColumnArray::create(ColumnInt32::create());
+
+    std::unordered_map<size_t, PostgreSQLArrayInfo> array_info;
+    preparePostgreSQLArrayInfo(array_info, 0, array_type);
+
+    /// Input "}" causes row_end at dimension 0 — must throw BAD_ARGUMENTS,
+    /// not underflow size_t to SIZE_MAX and crash.
+    try
+    {
+        insertPostgreSQLValue(
+            *column, "}",
+            ExternalResultDescription::ValueType::vtArray,
+            array_type, array_info, 0);
+        FAIL() << "Expected BAD_ARGUMENTS exception for malformed array '}'";
+    }
+    catch (const Exception & e)
+    {
+        EXPECT_EQ(e.code(), ErrorCodes::BAD_ARGUMENTS);
+    }
+}
+
+TEST(InsertPostgreSQLValue, MalformedArrayClosingThenOpeningThrows)
+{
+    auto nested_type = std::make_shared<DataTypeInt32>();
+    auto array_type = std::make_shared<DataTypeArray>(nested_type);
+    auto column = ColumnArray::create(ColumnInt32::create());
+
+    std::unordered_map<size_t, PostgreSQLArrayInfo> array_info;
+    preparePostgreSQLArrayInfo(array_info, 0, array_type);
+
+    /// Input "}{" also starts with row_end at dimension 0.
+    try
+    {
+        insertPostgreSQLValue(
+            *column, "}{",
+            ExternalResultDescription::ValueType::vtArray,
+            array_type, array_info, 0);
+        FAIL() << "Expected BAD_ARGUMENTS exception for malformed array '}{'" ;
+    }
+    catch (const Exception & e)
+    {
+        EXPECT_EQ(e.code(), ErrorCodes::BAD_ARGUMENTS);
+    }
+}
+
+TEST(InsertPostgreSQLValue, WellFormedArraySucceeds)
+{
+    auto nested_type = std::make_shared<DataTypeInt32>();
+    auto array_type = std::make_shared<DataTypeArray>(nested_type);
+    auto column = ColumnArray::create(ColumnInt32::create());
+
+    std::unordered_map<size_t, PostgreSQLArrayInfo> array_info;
+    preparePostgreSQLArrayInfo(array_info, 0, array_type);
+
+    /// Well-formed "{1,2,3}" must succeed without exceptions.
+    EXPECT_NO_THROW(
+        insertPostgreSQLValue(
+            *column, "{1,2,3}",
+            ExternalResultDescription::ValueType::vtArray,
+            array_type, array_info, 0));
+
+    /// Verify the column now has one row with 3 elements.
+    ASSERT_EQ(column->size(), 1u);
+}
+
+#endif

src/Dictionaries/ClickHouseDictionarySource.cpp

@@ -142,10 +142,8 @@ bool ClickHouseDictionarySource::isModified() const
     if (!configuration.invalidate_query.empty())
     {
         auto response = doInvalidateQuery(configuration.invalidate_query);
-        LOG_TRACE(log, "Invalidate query has returned: {}, previous value: {}", response, invalidate_query_response);
-        if (invalidate_query_response == response)
-            return false;
-        invalidate_query_response = response;
+        LOG_TRACE(log, "Invalidate query has returned: {}", response);
+        return invalidate_query_response.updateAndCheckModified(response);
     }
     return true;
 }

src/Dictionaries/ClickHouseDictionarySource.h

@@ -7,6 +7,7 @@
 #include <Dictionaries/DictionaryStructure.h>
 #include <Dictionaries/ExternalQueryBuilder.h>
 #include <Dictionaries/IDictionarySource.h>
+#include <Dictionaries/InvalidateQueryResponse.h>
 
 
 namespace DB
@@ -81,7 +82,7 @@ class ClickHouseDictionarySource final : public IDictionarySource
     std::chrono::time_point<std::chrono::system_clock> update_time;
     const DictionaryStructure dict_struct;
     const Configuration configuration;
-    mutable std::string invalidate_query_response;
+    mutable InvalidateQueryResponse invalidate_query_response;
     ExternalQueryBuilderPtr query_builder;
     Block sample_block;
     ContextMutablePtr context;

src/Dictionaries/InvalidateQueryResponse.cpp

@@ -0,0 +1,21 @@
+#include <Dictionaries/InvalidateQueryResponse.h>
+
+namespace DB
+{
+
+InvalidateQueryResponse::InvalidateQueryResponse(const InvalidateQueryResponse & other)
+{
+    std::lock_guard lock(other.mutex);
+    response = other.response;
+}
+
+bool InvalidateQueryResponse::updateAndCheckModified(const std::string & new_response)
+{
+    std::lock_guard lock(mutex);
+    if (response == new_response)
+        return false;
+    response = new_response;
+    return true;
+}
+
+}

src/Dictionaries/InvalidateQueryResponse.h

@@ -0,0 +1,24 @@
+#pragma once
+
+#include <mutex>
+#include <string>
+
+namespace DB
+{
+
+/// Thread-safe holder for the last value returned by a dictionary source's invalidate query.
+class InvalidateQueryResponse
+{
+public:
+    InvalidateQueryResponse() = default;
+    InvalidateQueryResponse(const InvalidateQueryResponse & other);
+    InvalidateQueryResponse & operator=(const InvalidateQueryResponse &) = delete;
+
+    bool updateAndCheckModified(const std::string & new_response);
+
+private:
+    mutable std::mutex mutex;
+    std::string response;
+};
+
+}

src/Dictionaries/MySQLDictionarySource.cpp

@@ -268,11 +268,7 @@ bool MySQLDictionarySource::isModified() const
     {
         LOG_TRACE(log, "Executing invalidate query: {}", configuration.invalidate_query);
         auto response = doInvalidateQuery(configuration.invalidate_query);
-        if (response == invalidate_query_response)
-            return false;
-
-        invalidate_query_response = response;
-        return true;
+        return invalidate_query_response.updateAndCheckModified(response);
     }
 
     return true;

src/Dictionaries/MySQLDictionarySource.h

@@ -10,6 +10,7 @@
 #    include <Dictionaries/DictionaryStructure.h>
 #    include <Dictionaries/ExternalQueryBuilder.h>
 #    include <Dictionaries/IDictionarySource.h>
+#    include <Dictionaries/InvalidateQueryResponse.h>
 #    include <Processors/Sources/MySQLSource.h>
 
 namespace Poco
@@ -89,7 +90,7 @@ class MySQLDictionarySource final : public IDictionarySource
     Block sample_block;
     ExternalQueryBuilder query_builder;
     const std::string load_all_query;
-    mutable std::string invalidate_query_response;
+    mutable InvalidateQueryResponse invalidate_query_response;
     const StreamSettings settings;
 };
 

src/Dictionaries/PostgreSQLDictionarySource.cpp

@@ -130,9 +130,7 @@ bool PostgreSQLDictionarySource::isModified() const
     if (!configuration.invalidate_query.empty())
     {
         auto response = doInvalidateQuery(configuration.invalidate_query);
-        if (response == invalidate_query_response)
-            return false;
-        invalidate_query_response = response;
+        return invalidate_query_response.updateAndCheckModified(response);
     }
     return true;
 }

src/Dictionaries/PostgreSQLDictionarySource.h

@@ -3,6 +3,7 @@
 #include "config.h"
 #include <Dictionaries/DictionaryStructure.h>
 #include <Dictionaries/IDictionarySource.h>
+#include <Dictionaries/InvalidateQueryResponse.h>
 
 #if USE_LIBPQXX
 #include <Dictionaries/ExternalQueryBuilder.h>
@@ -65,7 +66,7 @@ class PostgreSQLDictionarySource final : public IDictionarySource
     ExternalQueryBuilder query_builder;
     const std::string load_all_query;
     std::chrono::time_point<std::chrono::system_clock> update_time;
-    mutable std::string invalidate_query_response;
+    mutable InvalidateQueryResponse invalidate_query_response;
 
 };
 

src/Dictionaries/XDBCDictionarySource.cpp

@@ -187,9 +187,7 @@ bool XDBCDictionarySource::isModified() const
     if (!configuration.invalidate_query.empty())
     {
         auto response = doInvalidateQuery(configuration.invalidate_query);
-        if (invalidate_query_response == response)
-            return false;
-        invalidate_query_response = response;
+        return invalidate_query_response.updateAndCheckModified(response);
     }
     return true;
 }

src/Dictionaries/XDBCDictionarySource.h

@@ -7,6 +7,7 @@
 #include <Dictionaries/DictionaryStructure.h>
 #include <Dictionaries/ExternalQueryBuilder.h>
 #include <Dictionaries/IDictionarySource.h>
+#include <Dictionaries/InvalidateQueryResponse.h>
 
 
 namespace Poco
@@ -83,7 +84,7 @@ class XDBCDictionarySource final : public IDictionarySource, WithContext
     Block sample_block;
     ExternalQueryBuilder query_builder;
     const std::string load_all_query;
-    mutable std::string invalidate_query_response;
+    mutable InvalidateQueryResponse invalidate_query_response;
 
     BridgeHelperPtr bridge_helper;
     Poco::URI bridge_url;