fix codes according to the reviews

contrib/CMakeLists.txt

@@ -156,7 +156,7 @@ add_contrib (nuraft-cmake NuRaft)
 add_contrib (fast_float-cmake fast_float)
 add_contrib (datasketches-cpp-cmake datasketches-cpp)
 add_contrib (incbin-cmake incbin)
-add_contrib (sqids-cmake sqids-cpp)
+add_contrib (squids-cpp-cmake sqids-cpp)
 
 option(ENABLE_NLP "Enable NLP functions support" ${ENABLE_LIBRARIES})
 if (ENABLE_NLP)

contrib/squids-cpp-cmake/CMakeLists.txt

@@ -0,0 +1,18 @@
+option(ENABLE_SQIDS "Enable sqids support" ${ENABLE_LIBRARIES})
+if ((NOT ENABLE_SQIDS))
+    message (STATUS "Not using sqids")
+    return()
+endif()
+
+set (LIBRARY_DIR "${ClickHouse_SOURCE_DIR}/contrib/sqids-cpp")
+
+set (HDRS
+    "${LIBRARY_DIR}/include/sqids/blocklist.hpp"
+    "${LIBRARY_DIR}/include/sqids/sqids.hpp"
+)
+
+add_library(_sqids ${HDRS})
+target_include_directories(_sqids SYSTEM PUBLIC "${LIBRARY_DIR}/include")
+
+add_library(ch_contrib::squids ALIAS _sqids)
+target_compile_definitions(_sqids INTERFACE ENABLE_SQIDS)

docker/test/fasttest/run.sh

@@ -155,7 +155,6 @@ function clone_submodules
             contrib/libfiu
             contrib/incbin
             contrib/yaml-cpp
-            contrib/sqids
         )
 
         git submodule sync

docs/en/sql-reference/functions/hash-functions.md

@@ -1776,3 +1776,33 @@ Result:
 │ (('queries','database','analytical'),('oriented','processing','DBMS')) │
 └────────────────────────────────────────────────────────────────────────┘
 ```
+
+## sqid
+
+[sqid](https://sqids.org/) generates YouTube-looking IDs from numbers. It's good for link shortening, fast & URL-safe ID generation and decoding back into numbers for quicker database lookups.
+
+**Syntax**
+
+```sql
+sqid(number1,...)
+```
+
+**Arguments**
+
+The function takes a variable number of input parameters. Arguments can be any of UInt8, UInt16, UInt32, UInt64. Hashing function 'sqid' is experimental. Set `allow_experimental_hash_functions` setting to enable it.
+
+**Returned Value**
+
+- A [String](/docs/en/sql-reference/data-types/string.md).
+
+**Example**
+
+```sql
+SELECT sqid(1, 2, 3, 4, 5);
+```
+
+```response
+┌─sqid(1, 2, 3, 4, 5)─┐
+│ gXHfJ1C6dN          │
+└─────────────────────┘
+```

src/Functions/CMakeLists.txt

@@ -36,7 +36,6 @@ list (APPEND PUBLIC_LIBS
         ch_contrib::metrohash
         ch_contrib::murmurhash
         ch_contrib::morton_nd
-        ch_contrib::sqids
 )
 
 list (APPEND PRIVATE_LIBS
@@ -104,6 +103,10 @@ if (TARGET ch_contrib::crc32-vpmsum)
     list (APPEND PUBLIC_LIBS ch_contrib::crc32-vpmsum)
 endif()
 
+if (TARGET ch_contrib::squids)
+    list (APPEND PUBLIC_LIBS ch_contrib::squids)
+endif()
+
 add_subdirectory(GatherUtils)
 list (APPEND PRIVATE_LIBS clickhouse_functions_gatherutils)
 

src/Functions/FunctionSqid.cpp

@@ -1,5 +1,3 @@
-#pragma once
-
 #include <sqids/blocklist.hpp>
 #include <sqids/sqids.hpp>
 
@@ -26,11 +24,11 @@ namespace ErrorCodes
     extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
 }
 
-// sqids(numbers, alphabet, minLength, blocklist)
+// sqid(number1,...)
-class FunctionSqids : public IFunction
+class FunctionSqid : public IFunction
 {
 public:
-    static constexpr auto name = "sqids";
+    static constexpr auto name = "sqid";
 
     static FunctionPtr create(ContextPtr context)
     {
@@ -40,7 +38,7 @@ public:
                 "Hashing function '{}' is experimental. Set `allow_experimental_hash_functions` setting to enable it",
                 name);
 
-        return std::make_shared<FunctionSqids>();
+        return std::make_shared<FunctionSqid>();
     }
 
     String getName() const override { return name; }
@@ -56,7 +54,7 @@ public:
         if (arguments.empty())
             throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, "Function {} requires at least one argument.", getName());
 
-        for (auto i : collections::range(0, arguments.size()))
+        for (size_t i = 0; i < arguments.size(); ++i)
         {
             if (!checkDataTypes<
                     DataTypeUInt8,
@@ -80,18 +78,24 @@ public:
         auto col_res = ColumnString::create();
 
         sqidscxx::Sqids<> sqids;
+        std::vector<UInt64> numbers(num_args);
         for (size_t i = 0; i < input_rows_count; ++i)
         {
-            std::vector<UInt64> numbers(num_args);
             for (size_t j = 0; j < num_args; ++j)
             {
                 const ColumnWithTypeAndName & arg = arguments[j];
                 ColumnPtr current_column = arg.column;
                 numbers[j] = current_column->getUInt(i);
             }
-            col_res->insert(sqids.encode(numbers));
+            auto id = sqids.encode(numbers);
+            col_res->insert(id);
         }
         return col_res;
     }
 };
+
+REGISTER_FUNCTION(Sqid)
+{
+    factory.registerFunction<FunctionSqid>();
+}
 }

src/Functions/FunctionSqids.cpp

@@ -1,12 +0,0 @@
-#include "FunctionSqids.h"
-#include <Functions/FunctionFactory.h>
-
-namespace DB
-{
-
-REGISTER_FUNCTION(Sqids)
-{
-    factory.registerFunction<FunctionSqids>();
-}
-
-}

tests/queries/0_stateless/02724_sqids.reference

@@ -1,6 +0,0 @@
-86Rf07
-0	rSCtlB
-1	86Rf07
-2	0dT3tt
-3	GKU5cI
-4	IivTBt

tests/queries/0_stateless/02724_sqids.sql

@@ -1,4 +0,0 @@
-SET allow_experimental_hash_functions = 1;
-
-select sqids(1,2,3);
-select number, sqids(number, number+1, number+2) from system.numbers limit 5;

tests/queries/0_stateless/02933_sqid.reference

@@ -0,0 +1,17 @@
+gXHfJ1C6dN
+gXHfJ1C6dN
+0	rSCtlB
+1	86Rf07
+2	0dT3tt
+3	GKU5cI
+4	IivTBt
+oildrzxYUo
+gXHfJ1C6dN
+lNiIG1kKDO
+SoIvsIqL45
+zhcrwBm7k8
+oildrzxYUo
+gXHfJ1C6dN
+lNiIG1kKDO
+SoIvsIqL45
+zhcrwBm7k8

tests/queries/0_stateless/02933_sqid.sql

@@ -0,0 +1,29 @@
+-- Tags: no-fasttest
+SET allow_experimental_hash_functions = 1;
+SET allow_suspicious_low_cardinality_types = 1;
+
+select sqid(1, 2, 3, 4, 5);
+select sqid(1, 2, 3, 4, materialize(5));
+select number, sqid(number, number+1, number+2) from system.numbers limit 5;
+
+CREATE TABLE t_sqid
+(
+    id UInt64,
+    a LowCardinality(UInt8),
+    b LowCardinality(UInt16),
+    c LowCardinality(UInt32),
+    d LowCardinality(UInt64),
+    e Nullable(UInt8),
+    f Nullable(UInt16),
+    g Nullable(UInt32),
+    h Nullable(UInt64)
+)
+ENGINE = MergeTree ORDER BY id;
+
+INSERT INTO t_sqid select number, number+1, number+2, number+3, number+4, number+1, number+2, number+3, number+4 from system.numbers limit 5;
+
+select sqid(id, a, b, c, d) from t_sqid;
+select sqid(id, e, f, g, h) from t_sqid;
+
+select sqid('1'); -- { serverError 43}
+select sqid(); -- { serverError 42 }