From 47f6156ad8042dc2b6915ff17cc2605e1a281735 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 19 Jan 2020 03:01:49 +0300 Subject: [PATCH 1/3] Correct configuration of replxx --- contrib/replxx-cmake/CMakeLists.txt | 8 +++++--- dbms/programs/client/Client.cpp | 3 ++- libs/libcommon/include/common/config_common.h.in | 1 + libs/libcommon/src/LineReader.cpp | 11 ++++++----- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/contrib/replxx-cmake/CMakeLists.txt b/contrib/replxx-cmake/CMakeLists.txt index fed70558b07..389c1a7fb58 100644 --- a/contrib/replxx-cmake/CMakeLists.txt +++ b/contrib/replxx-cmake/CMakeLists.txt @@ -1,6 +1,6 @@ -option (ENABLE_READLINE "Enable readline support" ${ENABLE_LIBRARIES}) +option (ENABLE_REPLXX "Enable replxx support" ${ENABLE_LIBRARIES}) -if (ENABLE_READLINE) +if (ENABLE_REPLXX) option (USE_INTERNAL_REPLXX "Use internal replxx library" ${NOT_UNBUNDLED}) if (USE_INTERNAL_REPLXX) @@ -47,7 +47,9 @@ if (ENABLE_READLINE) endif () endif () - target_compile_definitions(replxx PUBLIC USE_REPLXX) + set(USE_REPLXX 1) message (STATUS "Using replxx") +else () + set(USE_REPLXX 0) endif () diff --git a/dbms/programs/client/Client.cpp b/dbms/programs/client/Client.cpp index 99d8fb3bdb0..7f014dcea79 100644 --- a/dbms/programs/client/Client.cpp +++ b/dbms/programs/client/Client.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -545,7 +546,7 @@ private: /// This is intended for testing purposes. if (config().getBool("always_load_suggestion_data", false)) { -#ifdef USE_REPLXX +#if USE_REPLXX SCOPE_EXIT({ Suggest::instance().finalize(); }); Suggest::instance().load(connection_parameters, config().getInt("suggestion_limit")); #else diff --git a/libs/libcommon/include/common/config_common.h.in b/libs/libcommon/include/common/config_common.h.in index 41999bb5cde..6cee84a5b32 100644 --- a/libs/libcommon/include/common/config_common.h.in +++ b/libs/libcommon/include/common/config_common.h.in @@ -3,5 +3,6 @@ // .h autogenerated by cmake ! #cmakedefine01 USE_JEMALLOC +#cmakedefine01 USE_REPLXX #cmakedefine01 UNBUNDLED #cmakedefine01 WITH_COVERAGE diff --git a/libs/libcommon/src/LineReader.cpp b/libs/libcommon/src/LineReader.cpp index 1f06373577f..17d69f91d15 100644 --- a/libs/libcommon/src/LineReader.cpp +++ b/libs/libcommon/src/LineReader.cpp @@ -1,6 +1,7 @@ +#include #include -#ifdef USE_REPLXX +#if USE_REPLXX # include #endif @@ -60,7 +61,7 @@ LineReader::Suggest::WordsRange LineReader::Suggest::getCompletions(const String LineReader::LineReader(const Suggest * suggest, const String & history_file_path_, char extender_, char delimiter_) : history_file_path(history_file_path_), extender(extender_), delimiter(delimiter_) { -#ifdef USE_REPLXX +#if USE_REPLXX impl = new replxx::Replxx; auto & rx = *(replxx::Replxx*)(impl); @@ -85,7 +86,7 @@ LineReader::LineReader(const Suggest * suggest, const String & history_file_path LineReader::~LineReader() { -#ifdef USE_REPLXX +#if USE_REPLXX auto & rx = *(replxx::Replxx*)(impl); if (!history_file_path.empty()) rx.history_save(history_file_path); @@ -141,7 +142,7 @@ LineReader::InputStatus LineReader::readOneLine(const String & prompt) { input.clear(); -#ifdef USE_REPLXX +#if USE_REPLXX auto & rx = *(replxx::Replxx*)(impl); const char* cinput = rx.input(prompt); if (cinput == nullptr) @@ -160,7 +161,7 @@ LineReader::InputStatus LineReader::readOneLine(const String & prompt) void LineReader::addToHistory(const String & line) { -#ifdef USE_REPLXX +#if USE_REPLXX auto & rx = *(replxx::Replxx*)(impl); rx.history_add(line); #endif From 4dd7bb7c50fedd4b1b687acf010d03b5776896bf Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 19 Jan 2020 03:18:25 +0300 Subject: [PATCH 2/3] Fixed configuration of replxx --- CMakeLists.txt | 1 + cmake/find/replxx.cmake | 40 +++++++++++++++++ contrib/CMakeLists.txt | 4 +- contrib/replxx-cmake/CMakeLists.txt | 70 +++++++---------------------- 4 files changed, 61 insertions(+), 54 deletions(-) create mode 100644 cmake/find/replxx.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 7c8ccb6e17c..d37cdfc3af8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -352,6 +352,7 @@ include (cmake/find/simdjson.cmake) include (cmake/find/rapidjson.cmake) include (cmake/find/fastops.cmake) include (cmake/find/orc.cmake) +include (cmake/find/replxx.cmake) find_contrib_lib(cityhash) find_contrib_lib(farmhash) diff --git a/cmake/find/replxx.cmake b/cmake/find/replxx.cmake new file mode 100644 index 00000000000..13df104515e --- /dev/null +++ b/cmake/find/replxx.cmake @@ -0,0 +1,40 @@ +option (ENABLE_REPLXX "Enable replxx support" ${ENABLE_LIBRARIES}) + +if (ENABLE_REPLXX) + option (USE_INTERNAL_REPLXX "Use internal replxx library" ${NOT_UNBUNDLED}) + + if (USE_INTERNAL_REPLXX AND NOT EXISTS "${ClickHouse_SOURCE_DIR}/contrib/replxx/README.md") + message (WARNING "submodule contrib/replxx is missing. to fix try run: \n git submodule update --init --recursive") + set (USE_INTERNAL_REPLXX 0) + endif () + + if (NOT USE_INTERNAL_REPLXX) + find_library(LIBRARY_REPLXX NAMES replxx replxx-static) + find_path(INCLUDE_REPLXX replxx.hxx) + + add_library(replxx UNKNOWN IMPORTED) + set_property(TARGET replxx PROPERTY IMPORTED_LOCATION ${LIBRARY_REPLXX}) + target_include_directories(replxx PUBLIC ${INCLUDE_REPLXX}) + + set(CMAKE_REQUIRED_LIBRARIES replxx) + check_cxx_source_compiles( + " + #include + int main() { + replxx::Replxx rx; + } + " + EXTERNAL_REPLXX_WORKS + ) + + if (NOT EXTERNAL_REPLXX_WORKS) + message (FATAL_ERROR "replxx is unusable: ${LIBRARY_REPLXX} ${INCLUDE_REPLXX}") + endif () + endif () + + set(USE_REPLXX 1) + + message (STATUS "Using replxx") +else () + set(USE_REPLXX 0) +endif () diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index fe3e4e83f03..f81d616cddd 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -332,4 +332,6 @@ if (USE_FASTOPS) add_subdirectory (fastops-cmake) endif() -add_subdirectory (replxx-cmake) +if (USE_INTERNAL_REPLXX) + add_subdirectory (replxx-cmake) +endif() diff --git a/contrib/replxx-cmake/CMakeLists.txt b/contrib/replxx-cmake/CMakeLists.txt index 389c1a7fb58..1e08b72f89a 100644 --- a/contrib/replxx-cmake/CMakeLists.txt +++ b/contrib/replxx-cmake/CMakeLists.txt @@ -1,55 +1,19 @@ -option (ENABLE_REPLXX "Enable replxx support" ${ENABLE_LIBRARIES}) +set (LIBRARY_DIR "${ClickHouse_SOURCE_DIR}/contrib/replxx") -if (ENABLE_REPLXX) - option (USE_INTERNAL_REPLXX "Use internal replxx library" ${NOT_UNBUNDLED}) +set(SRCS + ${LIBRARY_DIR}/src/conversion.cxx + ${LIBRARY_DIR}/src/ConvertUTF.cpp + ${LIBRARY_DIR}/src/escape.cxx + ${LIBRARY_DIR}/src/history.cxx + ${LIBRARY_DIR}/src/io.cxx + ${LIBRARY_DIR}/src/prompt.cxx + ${LIBRARY_DIR}/src/replxx.cxx + ${LIBRARY_DIR}/src/replxx_impl.cxx + ${LIBRARY_DIR}/src/util.cxx + ${LIBRARY_DIR}/src/wcwidth.cpp + ${LIBRARY_DIR}/src/windows.cxx +) - if (USE_INTERNAL_REPLXX) - set (LIBRARY_DIR "${ClickHouse_SOURCE_DIR}/contrib/replxx") - - set(SRCS - ${LIBRARY_DIR}/src/conversion.cxx - ${LIBRARY_DIR}/src/ConvertUTF.cpp - ${LIBRARY_DIR}/src/escape.cxx - ${LIBRARY_DIR}/src/history.cxx - ${LIBRARY_DIR}/src/io.cxx - ${LIBRARY_DIR}/src/prompt.cxx - ${LIBRARY_DIR}/src/replxx.cxx - ${LIBRARY_DIR}/src/replxx_impl.cxx - ${LIBRARY_DIR}/src/util.cxx - ${LIBRARY_DIR}/src/wcwidth.cpp - ${LIBRARY_DIR}/src/windows.cxx - ) - - add_library(replxx ${SRCS}) - target_include_directories(replxx PUBLIC ${LIBRARY_DIR}/include) - target_compile_options(replxx PUBLIC -Wno-documentation) - else () - find_library(LIBRARY_REPLXX NAMES replxx replxx-static) - find_path(INCLUDE_REPLXX replxx.hxx) - - add_library(replxx UNKNOWN IMPORTED) - set_property(TARGET replxx PROPERTY IMPORTED_LOCATION ${LIBRARY_REPLXX}) - target_include_directories(replxx PUBLIC ${INCLUDE_REPLXX}) - - set(CMAKE_REQUIRED_LIBRARIES replxx) - check_cxx_source_compiles( - " - #include - int main() { - replxx::Replxx rx; - } - " - EXTERNAL_REPLXX_WORKS - ) - - if (NOT EXTERNAL_REPLXX_WORKS) - message (FATAL_ERROR "replxx is unusable: ${LIBRARY_REPLXX} ${INCLUDE_REPLXX}") - endif () - endif () - - set(USE_REPLXX 1) - - message (STATUS "Using replxx") -else () - set(USE_REPLXX 0) -endif () +add_library(replxx ${SRCS}) +target_include_directories(replxx PUBLIC ${LIBRARY_DIR}/include) +target_compile_options(replxx PUBLIC -Wno-documentation) From db1bb630e04835b9b636c91305822ebe27d91613 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 19 Jan 2020 03:23:35 +0300 Subject: [PATCH 3/3] Fixed configuration of replxx --- dbms/programs/client/Client.cpp | 20 +------------------ dbms/programs/client/Suggest.h | 4 ++-- ...StorageSystemBuildOptions.generated.cpp.in | 1 + libs/libcommon/include/common/LineReader.h | 2 +- libs/libcommon/src/LineReader.cpp | 15 ++++++-------- 5 files changed, 11 insertions(+), 31 deletions(-) diff --git a/dbms/programs/client/Client.cpp b/dbms/programs/client/Client.cpp index 7f014dcea79..c73c87885b1 100644 --- a/dbms/programs/client/Client.cpp +++ b/dbms/programs/client/Client.cpp @@ -479,7 +479,7 @@ private: if (server_revision >= Suggest::MIN_SERVER_REVISION && !config().getBool("disable_suggestion", false)) /// Load suggestion data from the server. - Suggest::instance()->load(connection_parameters, config().getInt("suggestion_limit")); + Suggest::instance().load(connection_parameters, config().getInt("suggestion_limit")); /// Load command history if present. if (config().has("history_file")) @@ -543,17 +543,6 @@ private: } else { - /// This is intended for testing purposes. - if (config().getBool("always_load_suggestion_data", false)) - { -#if USE_REPLXX - SCOPE_EXIT({ Suggest::instance().finalize(); }); - Suggest::instance().load(connection_parameters, config().getInt("suggestion_limit")); -#else - throw Exception("Command line suggestions cannot work without line editing library", ErrorCodes::BAD_ARGUMENTS); -#endif - } - query_id = config().getString("query_id", ""); nonInteractive(); @@ -1823,13 +1812,6 @@ public: server_logs_file = options["server_logs_file"].as(); if (options.count("disable_suggestion")) config().setBool("disable_suggestion", true); - if (options.count("always_load_suggestion_data")) - { - if (options.count("disable_suggestion")) - throw Exception("Command line parameters disable_suggestion (-A) and always_load_suggestion_data cannot be specified simultaneously", - ErrorCodes::BAD_ARGUMENTS); - config().setBool("always_load_suggestion_data", true); - } if (options.count("suggestion_limit")) config().setInt("suggestion_limit", options["suggestion_limit"].as()); diff --git a/dbms/programs/client/Suggest.h b/dbms/programs/client/Suggest.h index 2fea534a986..bd4f239ddc7 100644 --- a/dbms/programs/client/Suggest.h +++ b/dbms/programs/client/Suggest.h @@ -18,10 +18,10 @@ namespace ErrorCodes class Suggest : public LineReader::Suggest, boost::noncopyable { public: - static Suggest * instance() + static Suggest & instance() { static Suggest instance; - return &instance; + return instance; } void load(const ConnectionParameters & connection_parameters, size_t suggestion_limit); diff --git a/dbms/src/Storages/System/StorageSystemBuildOptions.generated.cpp.in b/dbms/src/Storages/System/StorageSystemBuildOptions.generated.cpp.in index 65c4f19b7cb..550ead28996 100644 --- a/dbms/src/Storages/System/StorageSystemBuildOptions.generated.cpp.in +++ b/dbms/src/Storages/System/StorageSystemBuildOptions.generated.cpp.in @@ -61,6 +61,7 @@ const char * auto_config_build[] "USE_HYPERSCAN", "@USE_HYPERSCAN@", "USE_SIMDJSON", "@USE_SIMDJSON@", "USE_POCO_REDIS", "@USE_POCO_REDIS@", + "USE_REPLXX", "@USE_REPLXX@", nullptr, nullptr }; diff --git a/libs/libcommon/include/common/LineReader.h b/libs/libcommon/include/common/LineReader.h index 120ff76dac6..3661c6ef316 100644 --- a/libs/libcommon/include/common/LineReader.h +++ b/libs/libcommon/include/common/LineReader.h @@ -22,7 +22,7 @@ public: WordsRange getCompletions(const String & prefix, size_t prefix_length) const; }; - LineReader(const Suggest * suggest, const String & history_file_path, char extender, char delimiter = 0); /// if delimiter != 0, then it's multiline mode + LineReader(const Suggest & suggest, const String & history_file_path, char extender, char delimiter = 0); /// if delimiter != 0, then it's multiline mode ~LineReader(); /// Reads the whole line until delimiter (in multiline mode) or until the last line without extender. diff --git a/libs/libcommon/src/LineReader.cpp b/libs/libcommon/src/LineReader.cpp index 17d69f91d15..bab03dedbe5 100644 --- a/libs/libcommon/src/LineReader.cpp +++ b/libs/libcommon/src/LineReader.cpp @@ -58,7 +58,7 @@ LineReader::Suggest::WordsRange LineReader::Suggest::getCompletions(const String }); } -LineReader::LineReader(const Suggest * suggest, const String & history_file_path_, char extender_, char delimiter_) +LineReader::LineReader(const Suggest & suggest, const String & history_file_path_, char extender_, char delimiter_) : history_file_path(history_file_path_), extender(extender_), delimiter(delimiter_) { #if USE_REPLXX @@ -68,18 +68,15 @@ LineReader::LineReader(const Suggest * suggest, const String & history_file_path if (!history_file_path.empty()) rx.history_load(history_file_path); - auto callback = [suggest] (const String & context, size_t context_size) + auto callback = [&suggest] (const String & context, size_t context_size) { - auto range = suggest->getCompletions(context, context_size); + auto range = suggest.getCompletions(context, context_size); return replxx::Replxx::completions_t(range.first, range.second); }; - if (suggest) - { - rx.set_completion_callback(callback); - rx.set_complete_on_empty(false); - rx.set_word_break_characters(word_break_characters); - } + rx.set_completion_callback(callback); + rx.set_complete_on_empty(false); + rx.set_word_break_characters(word_break_characters); #endif /// FIXME: check extender != delimiter }