Added reverted files and fixes for failing fuzzer tests

2024-11-24 08:32:02 +00:00 · 2022-10-28 12:37:59 +02:00 · 2022-10-28 12:37:59 +02:00 · c37b154254
commit c37b154254
parent 348e153455
6 changed files with 219 additions and 28 deletions
--- a/src/Functions/FunctionsJSON.cpp
+++ b/src/Functions/FunctionsJSON.cpp
@ -20,18 +20,19 @@
 #include <Columns/ColumnArray.h>
 #include <Columns/ColumnTuple.h>

-#include <DataTypes/Serializations/SerializationDecimal.h>
-#include <DataTypes/DataTypesNumber.h>
-#include <DataTypes/DataTypeLowCardinality.h>
-#include <DataTypes/DataTypeString.h>
-#include <DataTypes/DataTypesDecimal.h>
-#include <DataTypes/DataTypeUUID.h>
+#include <DataTypes/DataTypeArray.h>
 #include <DataTypes/DataTypeEnum.h>
 #include <DataTypes/DataTypeFactory.h>
+#include <DataTypes/DataTypeFixedString.h>
+#include <DataTypes/DataTypeLowCardinality.h>
 #include <DataTypes/DataTypeNothing.h>
 #include <DataTypes/DataTypeNullable.h>
-#include <DataTypes/DataTypeArray.h>
+#include <DataTypes/DataTypeString.h>
 #include <DataTypes/DataTypeTuple.h>
+#include <DataTypes/DataTypeUUID.h>
+#include <DataTypes/DataTypesDecimal.h>
+#include <DataTypes/DataTypesNumber.h>
+#include <DataTypes/Serializations/SerializationDecimal.h>

 #include <Functions/FunctionFactory.h>
 #include <Functions/IFunction.h>
@ -695,8 +696,16 @@ public:
        else
            return false;

-        auto & col_vec = assert_cast<ColumnVector<NumberType> &>(dest);
-        col_vec.insertValue(value);
+        if (dest.getDataType() == TypeIndex::LowCardinality)
+        {
+            ColumnLowCardinality & col_low = assert_cast<ColumnLowCardinality &>(dest);
+            col_low.insertData(reinterpret_cast<const char *>(&value), sizeof(value));
+        }
+        else
+        {
+            auto & col_vec = assert_cast<ColumnVector<NumberType> &>(dest);
+            col_vec.insertValue(value);
+        }
        return true;
    }
 };
@ -773,8 +782,17 @@ public:
            return JSONExtractRawImpl<JSONParser>::insertResultToColumn(dest, element, {});

        auto str = element.getString();
-        ColumnString & col_str = assert_cast<ColumnString &>(dest);
-        col_str.insertData(str.data(), str.size());
+
+        if (dest.getDataType() == TypeIndex::LowCardinality)
+        {
+            ColumnLowCardinality & col_low = assert_cast<ColumnLowCardinality &>(dest);
+            col_low.insertData(str.data(), str.size());
+        }
+        else
+        {
+            ColumnString & col_str = assert_cast<ColumnString &>(dest);
+            col_str.insertData(str.data(), str.size());
+        }
        return true;
    }
 };
@ -803,25 +821,33 @@ struct JSONExtractTree
        }
    };

-    class LowCardinalityNode : public Node
+    class LowCardinalityFixedStringNode : public Node
    {
    public:
-        LowCardinalityNode(DataTypePtr dictionary_type_, std::unique_ptr<Node> impl_)
-            : dictionary_type(dictionary_type_), impl(std::move(impl_)) {}
+        explicit LowCardinalityFixedStringNode(const size_t fixed_length_) : fixed_length(fixed_length_) { }
        bool insertResultToColumn(IColumn & dest, const Element & element) override
        {
-            auto from_col = dictionary_type->createColumn();
-            if (impl->insertResultToColumn(*from_col, element))
-            {
-                std::string_view value = from_col->getDataAt(0).toView();
-                assert_cast<ColumnLowCardinality &>(dest).insertData(value.data(), value.size());
-                return true;
-            }
-            return false;
+            // If element is an object we delegate the insertion to JSONExtractRawImpl
+            if (element.isObject())
+                return JSONExtractRawImpl<JSONParser>::insertResultToLowCardinalityFixedStringColumn(dest, element, fixed_length);
+            else if (!element.isString())
+                return false;
+
+            auto str = element.getString();
+            if (str.size() > fixed_length)
+                return false;
+
+            // For the non low cardinality case of FixedString, the padding is done in the FixedString Column implementation.
+            // In order to avoid having to pass the data to a FixedString Column and read it back (which would slow down the execution)
+            // the data is padded here and written directly to the Low Cardinality Column
+            auto padded_str = str.data() + std::string(fixed_length - std::min(fixed_length, str.length()), '\0');
+
+            assert_cast<ColumnLowCardinality &>(dest).insertData(padded_str.data(), padded_str.size());
+            return true;
        }
+
    private:
-        DataTypePtr dictionary_type;
-        std::unique_ptr<Node> impl;
+        const size_t fixed_length;
    };

    class UUIDNode : public Node
@ -833,7 +859,15 @@ struct JSONExtractTree
                return false;

            auto uuid = parseFromString<UUID>(element.getString());
-            assert_cast<ColumnUUID &>(dest).insert(uuid);
+            if (dest.getDataType() == TypeIndex::LowCardinality)
+            {
+                ColumnLowCardinality & col_low = assert_cast<ColumnLowCardinality &>(dest);
+                col_low.insertData(reinterpret_cast<const char *>(&uuid), sizeof(uuid));
+            }
+            else
+            {
+                assert_cast<ColumnUUID &>(dest).insert(uuid);
+            }
            return true;
        }
    };
@ -853,6 +887,7 @@ struct JSONExtractTree
            assert_cast<ColumnDecimal<DecimalType> &>(dest).insert(result);
            return true;
        }
+
    private:
        DataTypePtr data_type;
    };
@ -871,13 +906,18 @@ struct JSONExtractTree
    public:
        bool insertResultToColumn(IColumn & dest, const Element & element) override
        {
-            if (!element.isString())
+            if (element.isNull())
                return false;
-            auto & col_str = assert_cast<ColumnFixedString &>(dest);
+
+            if (!element.isString())
+                return JSONExtractRawImpl<JSONParser>::insertResultToFixedStringColumn(dest, element, {});
+
            auto str = element.getString();
+            auto & col_str = assert_cast<ColumnFixedString &>(dest);
            if (str.size() > col_str.getN())
                return false;
            col_str.insertData(str.data(), str.size());
+
            return true;
        }
    };
@ -1099,9 +1139,19 @@ struct JSONExtractTree
            case TypeIndex::UUID: return std::make_unique<UUIDNode>();
            case TypeIndex::LowCardinality:
            {
+                // The low cardinality case is treated in two different ways:
+                // For FixedString type, an especial class is implemented for inserting the data in the destination column,
+                // as the string length must be passed in order to check and pad the incoming data.
+                // For the rest of low cardinality types, the insertion is done in their corresponding class, adapting the data
+                // as needed for the insertData function of the ColumnLowCardinality.
                auto dictionary_type = typeid_cast<const DataTypeLowCardinality *>(type.get())->getDictionaryType();
+                if ((*dictionary_type).getTypeId() == TypeIndex::FixedString)
+                {
+                    auto fixed_length = typeid_cast<const DataTypeFixedString *>(dictionary_type.get())->getN();
+                    return std::make_unique<LowCardinalityFixedStringNode>(fixed_length);
+                }
                auto impl = build(function_name, dictionary_type);
-                return std::make_unique<LowCardinalityNode>(dictionary_type, std::move(impl));
+                return impl;
            }
            case TypeIndex::Decimal256: return std::make_unique<DecimalNode<Decimal256>>(type);
            case TypeIndex::Decimal128: return std::make_unique<DecimalNode<Decimal128>>(type);
--- a/tests/performance/low_cardinality_from_json.xml
+++ b/tests/performance/low_cardinality_from_json.xml
@ -0,0 +1,73 @@
+<test>
+
+    <substitutions>
+        <substitution>
+            <name>string_json</name>
+            <values>
+                <value>'{"a": "hi", "b": "hello", "c": "hola", "d": "see you, bye, bye"}'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>int_json</name>
+            <values>
+                <value>'{"a": 11, "b": 2222, "c": 33333333, "d": 4444444444444444}'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>uuid_json</name>
+            <values>
+                <value>'{"a": "2d49dc6e-ddce-4cd0-afb8-790956df54c4", "b": "2d49dc6e-ddce-4cd0-afb8-790956df54c3", "c": "2d49dc6e-ddce-4cd0-afb8-790956df54c1", "d": "2d49dc6e-ddce-4cd0-afb8-790956df54c1"}'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_string</name>
+            <values>
+                <value>'Tuple(a LowCardinality(String), b LowCardinality(String), c LowCardinality(String), d LowCardinality(String) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_fixed_string</name>
+            <values>
+                <value>'Tuple(a LowCardinality(FixedString(20)), b LowCardinality(FixedString(20)), c LowCardinality(FixedString(20)), d LowCardinality(FixedString(20)) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_int8</name>
+            <values>
+                <value>'Tuple(a LowCardinality(Int8), b LowCardinality(Int8), c LowCardinality(Int8), d LowCardinality(Int8) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_int16</name>
+            <values>
+                <value>'Tuple(a LowCardinality(Int16), b LowCardinality(Int16), c LowCardinality(Int16), d LowCardinality(Int16) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_int32</name>
+            <values>
+                <value>'Tuple(a LowCardinality(Int32), b LowCardinality(Int32), c LowCardinality(Int32), d LowCardinality(Int32) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_int64</name>
+            <values>
+                <value>'Tuple(a LowCardinality(Int64), b LowCardinality(Int64), c LowCardinality(Int64), d LowCardinality(Int64) )'</value>
+            </values>
+        </substitution>
+        <substitution>
+            <name>low_cardinality_tuple_uuid</name>
+            <values>
+                <value>'Tuple(a LowCardinality(UUID), b LowCardinality(UUID), c LowCardinality(UUID), d LowCardinality(UUID) )'</value>
+            </values>
+        </substitution>
+    </substitutions>
+
+    <query>SELECT 'fixed_string_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({string_json}), {low_cardinality_tuple_fixed_string})) FORMAT Null </query>
+    <query>SELECT 'string_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({string_json}), {low_cardinality_tuple_string})) FORMAT Null </query>
+    <query>SELECT 'int8_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({int_json}), {low_cardinality_tuple_int8})) FORMAT Null </query>
+    <query>SELECT 'int16_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({int_json}), {low_cardinality_tuple_int16})) FORMAT Null </query>
+    <query>SELECT 'int32_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({int_json}), {low_cardinality_tuple_int32})) FORMAT Null </query>
+    <query>SELECT 'int64_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({int_json}), {low_cardinality_tuple_int64})) FORMAT Null </query>
+    <query>SELECT 'uuid_json' FROM zeros(500000) WHERE NOT ignore(JSONExtract(materialize({uuid_json}), {low_cardinality_tuple_uuid})) FORMAT Null </query>
+</test>
--- a/tests/queries/0_stateless/02452_check_low_cardinality.reference
+++ b/tests/queries/0_stateless/02452_check_low_cardinality.reference
@ -0,0 +1,7 @@
+('hi','hello','hola','see you, bye, bye')
+('hi\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0','hello\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0','hola\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0','see you, bye, bye\0\0\0')
+(11,0,0,0)
+(11,2222,0,0)
+(11,2222,33333333,0)
+(11,2222,33333333,4444444444444444)
+('2d49dc6e-ddce-4cd0-afb8-790956df54c4','2d49dc6e-ddce-4cd0-afb8-790956df54c3','2d49dc6e-ddce-4cd0-afb8-790956df54c1','2d49dc6e-ddce-4cd0-afb8-790956df54c1')
--- a/tests/queries/0_stateless/02452_check_low_cardinality.sql
+++ b/tests/queries/0_stateless/02452_check_low_cardinality.sql
@ -0,0 +1,54 @@
+DROP TABLE IF EXISTS test_low_cardinality_string;
+DROP TABLE IF EXISTS test_low_cardinality_uuid;
+DROP TABLE IF EXISTS test_low_cardinality_int;
+CREATE TABLE test_low_cardinality_string (data String) ENGINE MergeTree ORDER BY data;
+CREATE TABLE test_low_cardinality_uuid (data String) ENGINE MergeTree ORDER BY data;
+CREATE TABLE test_low_cardinality_int (data String) ENGINE MergeTree ORDER BY data;
+INSERT INTO test_low_cardinality_string (data) VALUES ('{"a": "hi", "b": "hello", "c": "hola", "d": "see you, bye, bye"}');
+INSERT INTO test_low_cardinality_int (data) VALUES ('{"a": 11, "b": 2222, "c": 33333333, "d": 4444444444444444}');
+INSERT INTO test_low_cardinality_uuid (data) VALUES ('{"a": "2d49dc6e-ddce-4cd0-afb8-790956df54c4", "b": "2d49dc6e-ddce-4cd0-afb8-790956df54c3", "c": "2d49dc6e-ddce-4cd0-afb8-790956df54c1", "d": "2d49dc6e-ddce-4cd0-afb8-790956df54c1"}');
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(String),
+                            b LowCardinality(String),
+                            c LowCardinality(String),
+                            d LowCardinality(String)
+                            )') AS json FROM test_low_cardinality_string;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(FixedString(20)),
+                            b LowCardinality(FixedString(20)),
+                            c LowCardinality(FixedString(20)),
+                            d LowCardinality(FixedString(20))
+                            )') AS json FROM test_low_cardinality_string;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(Int8),
+                            b LowCardinality(Int8),
+                            c LowCardinality(Int8),
+                            d LowCardinality(Int8)
+                            )') AS json FROM test_low_cardinality_int;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(Int16),
+                            b LowCardinality(Int16),
+                            c LowCardinality(Int16),
+                            d LowCardinality(Int16)
+                            )') AS json FROM test_low_cardinality_int;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(Int32),
+                            b LowCardinality(Int32),
+                            c LowCardinality(Int32),
+                            d LowCardinality(Int32)
+                            )') AS json FROM test_low_cardinality_int;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(Int64),
+                            b LowCardinality(Int64),
+                            c LowCardinality(Int64),
+                            d LowCardinality(Int64)
+                            )') AS json FROM test_low_cardinality_int;
+SELECT JSONExtract(data, 'Tuple(
+                            a LowCardinality(UUID),
+                            b LowCardinality(UUID),
+                            c LowCardinality(UUID),
+                            d LowCardinality(UUID)
+                            )') AS json FROM test_low_cardinality_uuid;
+DROP TABLE test_low_cardinality_string;
+DROP TABLE test_low_cardinality_uuid;
+DROP TABLE test_low_cardinality_int;
--- a/tests/queries/0_stateless/02455_extract_fixed_string_from_nested_json.reference
+++ b/tests/queries/0_stateless/02455_extract_fixed_string_from_nested_json.reference
@ -0,0 +1 @@
+('{"b":{"c":1,"d":"str"}}\0')
--- a/tests/queries/0_stateless/02455_extract_fixed_string_from_nested_json.sql
+++ b/tests/queries/0_stateless/02455_extract_fixed_string_from_nested_json.sql
@ -0,0 +1,6 @@
+-- Tags: no-fasttest
+DROP TABLE IF EXISTS test_fixed_string_nested_json;
+CREATE TABLE test_fixed_string_nested_json (data String) ENGINE MergeTree ORDER BY data;
+INSERT INTO test_fixed_string_nested_json (data) VALUES ('{"a" : {"b" : {"c" : 1, "d" : "str"}}}');
+SELECT JSONExtract(data, 'Tuple(a FixedString(24))') AS json FROM test_fixed_string_nested_json;
+DROP TABLE test_fixed_string_nested_json;