Change type from VARBINARY to ROW(HUGEINT, TINYINT)

facebookincubator · Oct 3, 2024 · 24ccc33 · 24ccc33
1 parent 7b8af4b
commit 24ccc33
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/velox/docs/develop/types.rst b/velox/docs/develop/types.rst
@@ -184,9 +184,9 @@ create IPPREFIX networks as well as to check IPADDRESS validity within
 IPPREFIX networks.
 
 IPPREFIX represents an IPv6 or IPv4 formatted IPv6 address along with a one byte
-prefix length. Its physical type is VARBINARY but has a fixed length of 17 bytes.
-The format that the address is stored in is defined as part of `RFC 4291#section-2.5.5.2 <https://datatracker.ietf.org/doc/html/rfc4291.html#section-2.5.5.2>`_.
-The prefix length is stored in the last byte of the VARBINARY array.
+prefix length. Its physical type is ROW(HUGEINT, TINYINT). The IPADDRESS is stored in
+the HUGEINT and is in the form defined in `RFC 4291#section-2.5.5.2 <https://datatracker.ietf.org/doc/html/rfc4291.html#section-2.5.5.2>`_.
+The prefix length is stored in the TINYINT.
 The IP address stored is the canonical(smallest) IP address in the
 subnet range. This type can be used in IP subnet functions.
 

diff --git a/velox/functions/prestosql/types/IPPrefixType.h b/velox/functions/prestosql/types/IPPrefixType.h
@@ -20,8 +20,8 @@
 
 namespace facebook::velox {
 
-class IPPrefixType : public VarbinaryType {
-  IPPrefixType() = default;
+class IPPrefixType : public RowType {
+  IPPrefixType() : RowType({"ip", "prefix_len"}, {HUGEINT(), TINYINT()}) {}
 
  public:
   static const std::shared_ptr<const IPPrefixType>& get() {
@@ -50,6 +50,11 @@ class IPPrefixType : public VarbinaryType {
     obj["type"] = name();
     return obj;
   }
+
+  const std::vector<TypeParameter>& parameters() const override {
+    static const std::vector<TypeParameter> kEmpty = {};
+    return kEmpty;
+  }
 };
 
 FOLLY_ALWAYS_INLINE bool isIPPrefixType(const TypePtr& type) {

diff --git a/velox/functions/prestosql/types/tests/IPPrefixTypeTest.cpp b/velox/functions/prestosql/types/tests/IPPrefixTypeTest.cpp
@@ -27,7 +27,7 @@ class IPPrefixTypeTest : public testing::Test, public TypeTestBase {
 
 TEST_F(IPPrefixTypeTest, basic) {
   ASSERT_STREQ(IPPREFIX()->name(), "IPPREFIX");
-  ASSERT_STREQ(IPPREFIX()->kindName(), "VARBINARY");
+  ASSERT_STREQ(IPPREFIX()->kindName(), "ROW");
   ASSERT_EQ(IPPREFIX()->name(), "IPPREFIX");
   ASSERT_TRUE(IPPREFIX()->parameters().empty());