Merge branch 'master' into create-index-unique-noop

.github/workflows/master.yml

@@ -3643,7 +3643,7 @@ jobs:
           cat >> "$GITHUB_ENV" << 'EOF'
           TEMP_PATH=${{runner.temp}}/unit_tests_asan
           REPORTS_PATH=${{runner.temp}}/reports_dir
-          CHECK_NAME=Unit tests (release-clang)
+          CHECK_NAME=Unit tests (release)
           REPO_COPY=${{runner.temp}}/unit_tests_asan/ClickHouse
           EOF
       - name: Download json reports

.github/workflows/pull_request.yml

@@ -4541,7 +4541,7 @@ jobs:
           cat >> "$GITHUB_ENV" << 'EOF'
           TEMP_PATH=${{runner.temp}}/unit_tests_asan
           REPORTS_PATH=${{runner.temp}}/reports_dir
-          CHECK_NAME=Unit tests (release-clang)
+          CHECK_NAME=Unit tests (release)
           REPO_COPY=${{runner.temp}}/unit_tests_asan/ClickHouse
           EOF
       - name: Download json reports

.gitmodules

@@ -340,3 +340,6 @@
 [submodule "contrib/c-ares"]
 	path = contrib/c-ares
 	url = https://github.com/c-ares/c-ares.git
+[submodule "contrib/incbin"]
+	path = contrib/incbin
+	url = https://github.com/graphitemaster/incbin.git

CMakeLists.txt

@@ -165,8 +165,14 @@ elseif(GLIBC_COMPATIBILITY)
     message (${RECONFIGURE_MESSAGE_LEVEL} "Glibc compatibility cannot be enabled in current configuration")
 endif ()
 
-# Make sure the final executable has symbols exported
-set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -rdynamic")
+if (OS_LINUX)
+    # We should not export dynamic symbols, because:
+    # - The main clickhouse binary does not use dlopen,
+    #   and whatever is poisoning it by LD_PRELOAD should not link to our symbols.
+    # - The clickhouse-odbc-bridge and clickhouse-library-bridge binaries
+    #   should not expose their symbols to ODBC drivers and libraries.
+    set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-export-dynamic")
+endif ()
 
 if (OS_DARWIN)
     # The `-all_load` flag forces loading of all symbols from all libraries,

SECURITY.md

@@ -13,9 +13,10 @@ The following versions of ClickHouse server are currently being supported with s
 
 | Version | Supported |
 |:-|:-|
+| 23.7 | ✔️ |
 | 23.6 | ✔️ |
 | 23.5 | ✔️ |
-| 23.4 | ✔️ |
+| 23.4 | ❌ |
 | 23.3 | ✔️ |
 | 23.2 | ❌ |
 | 23.1 | ❌ |

base/base/StringRef.h

@@ -8,6 +8,7 @@
 #include <functional>
 #include <iosfwd>
 
+#include <base/defines.h>
 #include <base/types.h>
 #include <base/unaligned.h>
 
@@ -274,6 +275,8 @@ struct CRC32Hash
         if (size == 0)
             return 0;
 
+        chassert(pos);
+
         if (size < 8)
         {
             return static_cast<unsigned>(hashLessThan8(x.data, x.size));

base/base/defines.h

@@ -115,8 +115,15 @@
 /// because SIGABRT is easier to debug than SIGTRAP (the second one makes gdb crazy)
 #if !defined(chassert)
     #if defined(ABORT_ON_LOGICAL_ERROR)
+        // clang-format off
+        #include <base/types.h>
+        namespace DB
+        {
+            void abortOnFailedAssertion(const String & description);
+        }
         #define chassert(x) static_cast<bool>(x) ? void(0) : ::DB::abortOnFailedAssertion(#x)
         #define UNREACHABLE() abort()
+        // clang-format off
     #else
         /// Here sizeof() trick is used to suppress unused warning for result,
         /// since simple "(void)x" will evaluate the expression, while

base/poco/Foundation/include/Poco/Message.h

@@ -67,6 +67,8 @@ class Foundation_API Message
 
     Message(
         const std::string & source, const std::string & text, Priority prio, const char * file, int line, std::string_view fmt_str = {});
+    Message(
+        std::string && source, std::string && text, Priority prio, const char * file, int line, std::string_view fmt_str);
     /// Creates a Message with the given source, text, priority,
     /// source file path and line.
     ///

base/poco/Foundation/include/Poco/URI.h

@@ -57,7 +57,7 @@ class Foundation_API URI
     URI();
     /// Creates an empty URI.
 
-    explicit URI(const std::string & uri);
+    explicit URI(const std::string & uri, bool enable_url_encoding = true);
     /// Parses an URI from the given string. Throws a
     /// SyntaxException if the uri is not valid.
 
@@ -350,13 +350,19 @@ class Foundation_API URI
     static const std::string ILLEGAL;
 
 private:
+    void encodePath(std::string & encodedStr) const;
+    void decodePath(const std::string & encodedStr);
+
+
     std::string _scheme;
     std::string _userInfo;
     std::string _host;
     unsigned short _port;
     std::string _path;
     std::string _query;
     std::string _fragment;
+
+    bool _enable_url_encoding = true;
 };
 
 

base/poco/Foundation/src/Message.cpp

@@ -60,6 +60,19 @@ Message::Message(const std::string& source, const std::string& text, Priority pr
 }
 
 
+Message::Message(std::string && source, std::string && text, Priority prio, const char * file, int line, std::string_view fmt_str):
+    _source(std::move(source)),
+    _text(std::move(text)),
+    _prio(prio),
+    _tid(0),
+    _file(file),
+    _line(line),
+    _pMap(0),
+    _fmt_str(fmt_str)
+{
+    init();
+}
+
 Message::Message(const Message& msg):
 	_source(msg._source),
 	_text(msg._text),

base/poco/Foundation/src/URI.cpp

@@ -36,8 +36,8 @@ URI::URI():
 }
 
 
-URI::URI(const std::string& uri):
-	_port(0)
+URI::URI(const std::string& uri, bool enable_url_encoding):
+	_port(0), _enable_url_encoding(enable_url_encoding)
 {
 	parse(uri);
 }
@@ -107,7 +107,8 @@ URI::URI(const URI& uri):
 	_port(uri._port),
 	_path(uri._path),
 	_query(uri._query),
-	_fragment(uri._fragment)
+	_fragment(uri._fragment),
+    _enable_url_encoding(uri._enable_url_encoding)
 {
 }
 
@@ -119,7 +120,8 @@ URI::URI(const URI& baseURI, const std::string& relativeURI):
 	_port(baseURI._port),
 	_path(baseURI._path),
 	_query(baseURI._query),
-	_fragment(baseURI._fragment)
+	_fragment(baseURI._fragment),
+    _enable_url_encoding(baseURI._enable_url_encoding)
 {
 	resolve(relativeURI);
 }
@@ -151,6 +153,7 @@ URI& URI::operator = (const URI& uri)
 		_path     = uri._path;
 		_query    = uri._query;
 		_fragment = uri._fragment;
+        _enable_url_encoding = uri._enable_url_encoding;
 	}
 	return *this;
 }
@@ -181,6 +184,7 @@ void URI::swap(URI& uri)
 	std::swap(_path, uri._path);
 	std::swap(_query, uri._query);
 	std::swap(_fragment, uri._fragment);
+    std::swap(_enable_url_encoding, uri._enable_url_encoding);
 }
 
 
@@ -201,7 +205,7 @@ std::string URI::toString() const
 	std::string uri;
 	if (isRelative())
 	{
-		encode(_path, RESERVED_PATH, uri);
+		encodePath(uri);
 	}
 	else
 	{
@@ -217,7 +221,7 @@ std::string URI::toString() const
 		{
 			if (!auth.empty() && _path[0] != '/')
 				uri += '/';
-			encode(_path, RESERVED_PATH, uri);
+            encodePath(uri);
 		}
 		else if (!_query.empty() || !_fragment.empty())
 		{
@@ -313,7 +317,7 @@ void URI::setAuthority(const std::string& authority)
 void URI::setPath(const std::string& path)
 {
 	_path.clear();
-	decode(path, _path);
+    decodePath(path);
 }
 
 
@@ -418,7 +422,7 @@ void URI::setPathEtc(const std::string& pathEtc)
 std::string URI::getPathEtc() const
 {
 	std::string pathEtc;
-	encode(_path, RESERVED_PATH, pathEtc);
+	encodePath(pathEtc);
 	if (!_query.empty())
 	{
 		pathEtc += '?';
@@ -436,7 +440,7 @@ std::string URI::getPathEtc() const
 std::string URI::getPathAndQuery() const
 {
 	std::string pathAndQuery;
-	encode(_path, RESERVED_PATH, pathAndQuery);
+	encodePath(pathAndQuery);
 	if (!_query.empty())
 	{
 		pathAndQuery += '?';
@@ -681,6 +685,21 @@ void URI::decode(const std::string& str, std::string& decodedStr, bool plusAsSpa
 	}
 }
 
+void URI::encodePath(std::string & encodedStr) const
+{
+    if (_enable_url_encoding)
+        encode(_path, RESERVED_PATH, encodedStr);
+    else
+        encodedStr = _path;
+}
+
+void URI::decodePath(const std::string & encodedStr)
+{
+    if (_enable_url_encoding)
+        decode(encodedStr, _path);
+    else
+        _path = encodedStr;
+}
 
 bool URI::isWellKnownPort() const
 {
@@ -820,7 +839,7 @@ void URI::parsePath(std::string::const_iterator& it, const std::string::const_it
 {
 	std::string path;
 	while (it != end && *it != '?' && *it != '#') path += *it++;
-	decode(path, _path);
+    decodePath(path);
 }
 
 

cmake/autogenerated_versions.txt

@@ -2,11 +2,11 @@
 
 # NOTE: has nothing common with DBMS_TCP_PROTOCOL_VERSION,
 # only DBMS_TCP_PROTOCOL_VERSION should be incremented on protocol changes.
-SET(VERSION_REVISION 54476)
+SET(VERSION_REVISION 54477)
 SET(VERSION_MAJOR 23)
-SET(VERSION_MINOR 7)
+SET(VERSION_MINOR 8)
 SET(VERSION_PATCH 1)
-SET(VERSION_GITHASH d1c7e13d08868cb04d3562dcced704dd577cb1df)
-SET(VERSION_DESCRIBE v23.7.1.1-testing)
-SET(VERSION_STRING 23.7.1.1)
+SET(VERSION_GITHASH a70127baecc451f1f7073bad7b6198f6703441d8)
+SET(VERSION_DESCRIBE v23.8.1.1-testing)
+SET(VERSION_STRING 23.8.1.1)
 # end of autochange

cmake/split_debug_symbols.cmake

@@ -22,8 +22,9 @@ macro(clickhouse_split_debug_symbols)
        # Splits debug symbols into separate file, leaves the binary untouched:
        COMMAND "${OBJCOPY_PATH}" --only-keep-debug "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}" "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug"
        COMMAND chmod 0644 "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug"
-       # Strips binary, sections '.note' & '.comment' are removed in line with Debian's stripping policy: www.debian.org/doc/debian-policy/ch-files.html, section '.clickhouse.hash' is needed for integrity check:
-       COMMAND "${STRIP_PATH}" --remove-section=.comment --remove-section=.note --keep-section=.clickhouse.hash "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
+       # Strips binary, sections '.note' & '.comment' are removed in line with Debian's stripping policy: www.debian.org/doc/debian-policy/ch-files.html, section '.clickhouse.hash' is needed for integrity check.
+       # Also, after we disabled the export of symbols for dynamic linking, we still to keep a static symbol table for good stack traces.
+       COMMAND "${STRIP_PATH}" --strip-debug --remove-section=.comment --remove-section=.note "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
        # Associate stripped binary with debug symbols:
        COMMAND "${OBJCOPY_PATH}" --add-gnu-debuglink "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug" "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
        COMMENT "Stripping clickhouse binary" VERBATIM

contrib/CMakeLists.txt

@@ -164,13 +164,13 @@ add_contrib (libpq-cmake libpq)
 add_contrib (nuraft-cmake NuRaft)
 add_contrib (fast_float-cmake fast_float)
 add_contrib (datasketches-cpp-cmake datasketches-cpp)
+add_contrib (incbin-cmake incbin)
 
 option(ENABLE_NLP "Enable NLP functions support" ${ENABLE_LIBRARIES})
 if (ENABLE_NLP)
     add_contrib (libstemmer-c-cmake libstemmer_c)
     add_contrib (wordnet-blast-cmake wordnet-blast)
     add_contrib (lemmagen-c-cmake lemmagen-c)
-    add_contrib (nlp-data-cmake nlp-data)
     add_contrib (cld2-cmake cld2)
 endif()
 

contrib/arrow-cmake/CMakeLists.txt

@@ -502,9 +502,10 @@ target_include_directories(_parquet SYSTEM BEFORE
         "${ClickHouse_SOURCE_DIR}/contrib/arrow/cpp/src"
         "${CMAKE_CURRENT_SOURCE_DIR}/cpp/src")
 target_link_libraries(_parquet
-    PUBLIC _arrow
-    PRIVATE
+    PUBLIC
+        _arrow
         ch_contrib::thrift
+    PRIVATE
         boost::headers_only
         boost::regex
         OpenSSL::Crypto OpenSSL::SSL)