Ipv4 and ipv6 domains #3669

Enmk · 2018-11-26T14:30:27Z

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Added:

IDataTypeDomain interface;
method DataTypeFactory::registerDataTypeDomain for registering domains;
DataTypeDomainWithSimpleSerialization domain base class with simple serialization/deserialization;
Concrete IPv4 and IPv6 domain implementations: DataTypeDomanIPv6 and DataTypeDomanIPv4;

Updated:

IDataType text serialization/deserialization methods;
IDataType implementation to use domain for text serialization/deserialization;
Refactored implementation of the IPv4/IPv6 functions to use formatIPv4/v6 and parseIPv4/v6 from Common/formatIPv6.h;

Tests:

Added test cases for IPv4 and IPv6 domains.
Updated IPv4/v6 functions tests to validate more cases;
Added performance tests for IPv4 and IPv6 related functions;

alexey-milovidov · 2018-11-26T14:35:11Z

dbms/src/DataTypes/DataTypeDomainIPv4AndIPv6.cpp

+
+} // namespace
+
+void registerDataTypeDomainIPv4AndIPv5(DataTypeFactory & factory)


alexey-milovidov · 2018-11-26T14:36:21Z

dbms/src/DataTypes/DataTypeDomainWithSimpleSerialization.h

+    /** Text serialization for the CSV format.
+      */
+    void serializeTextCSV(const IColumn & column, size_t row_num, WriteBuffer & ostr, const FormatSettings &) const override;
+    /** delimiter - the delimiter we expect when reading a string value that is not double-quoted


Obsolete comment.

alexey-milovidov · 2018-11-26T14:36:30Z

dbms/src/DataTypes/DataTypeDomainWithSimpleSerialization.h

+    void deserializeTextCSV(IColumn & column, ReadBuffer & istr, const FormatSettings &) const override;
+
+    /** Text serialization intended for using in JSON format.
+      * force_quoting_64bit_integers parameter forces to brace UInt64 and Int64 types into quotes.


Obsolete comment.

alexey-milovidov · 2018-11-26T14:38:18Z

dbms/src/DataTypes/DataTypeFactory.cpp

@@ -128,6 +115,48 @@ void DataTypeFactory::registerSimpleDataType(const String & name, SimpleCreator
    }, case_sensitiveness);
 }

+void DataTypeFactory::registerDataTypeDomain(const String & domain_name, const String & family_name, DataTypeDomainUniquePtr domain, CaseSensitiveness case_sensitiveness)
+{
+    const auto& data_type_creator = findCreatorByName(family_name);


alexey-milovidov · 2018-11-26T14:39:55Z

dbms/src/DataTypes/DataTypeFactory.cpp

+    const auto& data_type_creator = findCreatorByName(family_name);
+    all_domains.reserve(all_domains.size() + 1);
+
+    // We can't move the pointer to the Creator since creator is copied later.


shared_ptr is Ok.

Simplified that piece of code, shared_ptr is not needed anymore.

alexey-milovidov · 2018-11-26T14:41:43Z

dbms/src/DataTypes/IDataTypeDomain.h

+struct FormatSettings;
+class IColumn;
+
+class IDataTypeDomain


Missing comment.

alexey-milovidov · 2018-11-26T14:41:58Z

dbms/src/DataTypes/IDataTypeDomain.h

+    virtual void deserializeTextCSV(IColumn & column, ReadBuffer & istr, const FormatSettings &) const = 0;
+
+    /** Text serialization intended for using in JSON format.
+      * force_quoting_64bit_integers parameter forces to brace UInt64 and Int64 types into quotes.


Obsolete comment.

alexey-milovidov · 2018-11-26T14:42:10Z

dbms/src/DataTypes/IDataTypeDomain.h

+      */
+    virtual void serializeTextCSV(const IColumn & column, size_t row_num, WriteBuffer & ostr, const FormatSettings &) const = 0;
+
+    /** delimiter - the delimiter we expect when reading a string value that is not double-quoted


Obsolete comment.

champtar · 2018-11-26T14:57:04Z

Hi @Enmk @alexey-milovidov is there any plans to have an IP datatype where you can have both IPv4 and IPv6 ? Right now I'm storing all IPs as a FixedString(16) so it's way simpler when querying

Enmk · 2019-01-14T14:49:31Z

Performance comparison result.

Performance was measured on my machine against base version (without these modifications) using performance tests included in the PR. Negative numbers represent degraded performance, positive - improved, see compare.py for details.

Please note that I've observed +/-1% variance of the results from run to run, so I believe there numbers here are within +/-2%.

ipv4 (ALL CASES)

Queries: 444
Stats:
avg: -12.1179753049
min: -33.0634032212
max: 12.7247256601
stdev: 6.32970567083

ipv4 IPv4StringToNum

Queries: 100
Stats:
avg: -16.9865447285
min: -23.5455008443
max: -12.0256227406
stdev: 2.44602672946

ipv4 IPv4NumToString+IPv4StringToNum

Queries: 100
Stats:
avg: -9.63764803906
min: -21.3878474019
max: -5.97267144694
stdev: 1.66649394699

ipv4 IPv4NumToStringClassC+IPv4StringToNum

Queries: 100
Stats:
avg: -8.24220705267
min: -13.3396067461
max: -3.68402493364
stdev: 1.42471396986

ipv4 error

Queries: 44
Stats:
avg: -1.78364095218
min: -12.4710148976
max: 12.7247256601
stdev: 7.59229876574

ipv6 (ALL CASES)

Queries: 426
Stats:
avg: -5.88968274932
min: -22.8925432598
max: 10.2782631191
stdev: 5.88792824151

ipv6 IPv6StringToNum

Queries: 100
Stats:
avg: -15.0270332058
min: -22.8925432598
max: -10.0665889641
stdev: 2.67577871908

ipv6 IPv6NumToString+IPv6StringToNum

Queries: 100
Stats:
avg: -5.09307059004
min: -10.7523961077
max: -2.51185092654
stdev: 1.6369094322

ipv6 error

Queries: 26
Stats:
avg: -4.67112698211
min: -13.9460662489
max: 4.58204584396
stdev: 5.82377752186

ipv6 mapped

Queries: 200
Stats:
avg: -1.87772585045
min: -18.7335566474
max: 10.2782631191
stdev: 2.49766966381

s-mx · 2019-01-15T17:47:58Z

dbms/tests/queries/0_stateless/00725_ipv4_ipv6_domains.sql

@@ -0,0 +1,59 @@
+DROP TABLE IF EXISTS ipv4_test;


In stateless tests tables should be created in test database.

s-mx · 2019-01-16T11:41:51Z

dbms/src/Common/formatIPv6.cpp

 {
    const auto limit = IPV4_BINARY_LENGTH - zeroed_tail_bytes_count;

-    for (const auto i : ext::range(0, IPV4_BINARY_LENGTH))
+    for (const auto i : ext::range(0, limit))
    {
        UInt8 byte = (i < limit) ? src[i] : 0;


useless ternary operator

useless ternary operator

fixed

s-mx · 2019-01-16T11:52:56Z

dbms/src/Common/formatIPv6.cpp

@@ -136,4 +145,134 @@ void formatIPv6(const unsigned char * src, char *& dst, UInt8 zeroed_tail_bytes_
    *dst++ = '\0';
 }

+//UInt32 parseIPv4_fast(const char * src)


Do we need this function?

Nope, forgot to remove refactoring leftovers from commit. It is gone now.

s-mx · 2019-01-27T21:24:32Z

dbms/programs/performance-test/PerformanceTest.cpp

            }
        }
    }

-    std::vector<String> formatQueries(const String & query, StringToVector substitutions_to_generate)
+    Queries formatQueries(const Query & query, const SubstitutionsMap& substitutions_to_generate)


const SubstitutionsMap &

Reverted all changes that I've maid to PerformaceTest.cpp to reduce the scope of this PR.

s-mx · 2019-01-27T21:47:58Z

dbms/src/Common/formatIPv6.cpp

@@ -46,18 +48,24 @@ static void printInteger(char *& out, T value)
 }

 /// print IPv4 address as %u.%u.%u.%u
-static void formatIPv4(const unsigned char * src, char *& dst, UInt8 zeroed_tail_bytes_count)
+static void doFormatIPv4(const unsigned char * src, char *& dst, UInt8 zeroed_tail_bytes_count)


Do we need UInt8 zeroed_tail_bytes_count ?
There is only only one invocation, and there zeroed_tail_bytes_count equals IPV4_BINARY_LENGTH.

That particular function was used by formatIPv6, used by both CutIPv6 and IPv6NumToString. I've removed that implementation and unified it with the rest of the code, so formatIPv4 it is used by formatIPv6, IPv4NumToString and IPv4NumToStringClassC.

s-mx · 2019-01-27T22:40:30Z

dbms/tests/queries/0_stateless/00076_ip_coding_functions.sql

@@ -6,6 +6,8 @@ select IPv4StringToNum('127.0.0.1' as p) == (0x7f000001 as n), IPv4NumToString(n
 select IPv4StringToNum(materialize('127.0.0.1') as p) == (materialize(0x7f000001) as n), IPv4NumToString(n) == p;
 select IPv4NumToString(toUInt32(0)) == '0.0.0.0';
 select IPv4NumToString(materialize(toUInt32(0))) == materialize('0.0.0.0');
+select IPv4NumToString(toUInt32(0x7f000001)) == '127.0.0.1';
+select IPv4NumToString(materialize(toUInt32(0x7f000001))) == materialize('127.0.0.1');


Unfortunately, this test fails on my pc. Here is the diff https://gist.github.com/s-mx/83ed11ed435525470869cf2edc33da3d

Added: * IDataTypeDomain interface; * method DataTypeFactory::registerDataTypeDomain for registering domains; * DataTypeDomainWithSimpleSerialization domain base class with simple serialization/deserialization; * Concrete IPv4 and IPv6 domain implementations: DataTypeDomanIPv6 and DataTypeDomanIPv4; Updated: * IDataType text serialization/deserialization methods; * IDataType implementation to use domain for text serialization/deserialization; * Refactored implementation of the IPv4/IPv6 functions to use formatIPv4/v6 and parseIPv4/v6 from Common/formatIPv6.h; Tests: * Added test cases for IPv4 and IPv6 domains. * Updated IPv4/v6 functions tests to validate more cases; * Added performance tests for IPv4 and IPv6 related functions;

The-Alchemist · 2019-03-01T17:53:42Z

@Enmk : Out of curiosity, why is IPv6 represented using FixedString(16) instead of UInt128?

@KochetovNicolai : Would a PR changing it to UInt128 be considered?

Enmk · 2019-03-02T12:16:18Z

@Enmk : Out of curiosity, why is IPv6 represented using FixedString(16) instead of UInt128?

Well, the major reason is compatibility. Existing IPv6-functions work with FixedString(16) and hence work with IPv6 domain too, also that allows for seamless comparing of old FixedString(16)-IPv6 column values with IPv6.

alexey-milovidov reviewed Nov 26, 2018

View reviewed changes

Enmk force-pushed the ipv4_and_ipv6_domains branch from ec3298b to eed1527 Compare December 7, 2018 08:55

alesapin force-pushed the master branch from 0b4d097 to 5498ed5 Compare December 7, 2018 11:01

Enmk force-pushed the ipv4_and_ipv6_domains branch from eed1527 to f19e284 Compare December 13, 2018 13:47

Enmk force-pushed the ipv4_and_ipv6_domains branch 2 times, most recently from 31b74e8 to 21abf95 Compare December 28, 2018 11:43

Enmk force-pushed the ipv4_and_ipv6_domains branch from 21abf95 to 4060285 Compare January 14, 2019 14:37

s-mx reviewed Jan 15, 2019

View reviewed changes

s-mx reviewed Jan 16, 2019

View reviewed changes

Enmk force-pushed the ipv4_and_ipv6_domains branch 4 times, most recently from aaf70e8 to 9b7818a Compare January 25, 2019 09:26

s-mx reviewed Jan 27, 2019

View reviewed changes

Enmk force-pushed the ipv4_and_ipv6_domains branch from 9b7818a to 59910eb Compare February 4, 2019 09:32

Enmk force-pushed the ipv4_and_ipv6_domains branch from 59910eb to 2716df8 Compare February 4, 2019 09:38

KochetovNicolai merged commit 9f971c2 into ClickHouse:master Feb 6, 2019

AchilleAsh mentioned this pull request Feb 19, 2019

Add native IPv4 and IPv6 types support mymarilyn/clickhouse-driver#73

Merged

Matt-Texier mentioned this pull request Feb 25, 2019

Extended support of IPv4 and IPv6 column types mymarilyn/clickhouse-driver#43

Closed

hczhcz mentioned this pull request Apr 24, 2019

Add DataTypeDomainBool #5071

Closed

filimonov added the altinity label Jun 10, 2019

Enmk deleted the ipv4_and_ipv6_domains branch December 8, 2020 14:36

alexey-milovidov mentioned this pull request Jan 16, 2021

Misleading indentation of IPv4 addresses in Pretty formats #19184

Closed

alexey-milovidov mentioned this pull request Mar 31, 2021

Refactoring of data types serialization #21562

Merged

alexey-milovidov mentioned this pull request Apr 19, 2021

IPv4OrZero or toIPv4OrNull does not exist. #22825

Closed

This was referenced Aug 15, 2021

IPv4/IPv6 "data type domains" are barely usable #27491

Closed

Wrong exception message about toIPv4OrZero function. #27490

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ipv4 and ipv6 domains #3669

Ipv4 and ipv6 domains #3669

Enmk commented Nov 26, 2018 •

edited

Loading

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 12, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

alexey-milovidov Nov 26, 2018

Enmk Dec 6, 2018

champtar commented Nov 26, 2018

Enmk commented Jan 14, 2019 •

edited

Loading

s-mx Jan 15, 2019

Enmk Jan 23, 2019

s-mx Jan 16, 2019

Enmk Jan 23, 2019

s-mx Jan 16, 2019

Enmk Feb 4, 2019 •

edited

Loading

s-mx Jan 27, 2019

Enmk Feb 4, 2019

s-mx Jan 27, 2019

Enmk Feb 4, 2019 •

edited

Loading

s-mx Jan 27, 2019

Enmk Feb 4, 2019

The-Alchemist commented Mar 1, 2019 •

edited

Loading

Enmk commented Mar 2, 2019


		} // namespace

		void registerDataTypeDomainIPv4AndIPv5(DataTypeFactory & factory)

Ipv4 and ipv6 domains #3669

Ipv4 and ipv6 domains #3669

Conversation

Enmk commented Nov 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

champtar commented Nov 26, 2018

Enmk commented Jan 14, 2019 • edited Loading

ipv4 (ALL CASES)

ipv4 IPv4StringToNum

ipv4 IPv4NumToString+IPv4StringToNum

ipv4 IPv4NumToStringClassC+IPv4StringToNum

ipv4 error

ipv6 (ALL CASES)

ipv6 IPv6StringToNum

ipv6 IPv6NumToString+IPv6StringToNum

ipv6 error

ipv6 mapped

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Enmk Feb 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Enmk Feb 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The-Alchemist commented Mar 1, 2019 • edited Loading

Enmk commented Mar 2, 2019

Enmk commented Nov 26, 2018 •

edited

Loading

Enmk commented Jan 14, 2019 •

edited

Loading

Enmk Feb 4, 2019 •

edited

Loading

Enmk Feb 4, 2019 •

edited

Loading

The-Alchemist commented Mar 1, 2019 •

edited

Loading