Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23552 )

Change subject: KUDU-1261 array columns aren't yet supported as key columns
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23552/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/23552/1/src/kudu/master/master-test.cc@2063
PS1, Line 2063: TEST_F(MasterTest, CreateTableWithArrayKeyType) {
              :   constexpr const char* const kTableName = 
"array_primary_key_column";
              :   constexpr const char* const kErrMsg =
              :       "key column may not have type of BOOL, FLOAT, DOUBLE, or 
NESTED";
              :
              :   const Schema table_schemas[] = {
              :     {
              :       {
              :         
ColumnSchemaBuilder().type(INT32).array(true).name("key"),
              :       }, 1
              :     },
              :     {
              :       {
              :         
ColumnSchemaBuilder().type(STRING).array(true).name("key"),
              :         
ColumnSchemaBuilder().type(INT64).name("c1").nullable(true),
              :       }, 1
              :     },
              :     {
              :       {
              :         
ColumnSchemaBuilder().type(INT16).array(true).name("key"),
              :         
ColumnSchemaBuilder().type(INT8).name("c1").nullable(true),
              :       }, 1
              :     },
              :     {
              :       {
              :         ColumnSchema("key", INT32),
              :         
ColumnSchemaBuilder().type(INT64).array(true).name("arr0"),
              :       }, 2
              :     },
              :     {
              :       {
              :         
ColumnSchemaBuilder().type(STRING).array(true).name("arr0"),
              :         ColumnSchema("key", INT32),
              :         
ColumnSchemaBuilder().type(INT64).array(true).name("arr1"),
              :       }, 3
              :     },
              :     {
              :       {
              :         
ColumnSchemaBuilder().type(BOOL).array(true).name("arr0"),
              :         ColumnSchema("key", BINARY),
              :       }, 2
              :     },
              :   };
              :
              :   const vector<KuduPartialRow> split_rows{};
              :   for (const auto& schema : table_schemas) {
              :     SCOPED_TRACE(schema.ToString(Schema::BASE_INFO));
              :     const auto s = CreateTable(kTableName, schema, split_rows);
              :     ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
              :     ASSERT_STR_CONTAINS(s.ToString(), kErrMsg);
              :   }
              : }
> nit: Is this possible to consolidate this into existing test of invalid key
It's possible, but I don't think it makes a lot of sense for two reasons: (a) 
this restriction on array columns is considered temporary (b) 
TestCreateTableInvalidKeyType is good on its own because of the way it uses the 
set of special scalar types.  Essentially, these are the reasons behind my 
decision to add this array-specific scenario on its own, not extending 
TestCreateTableInvalidKeyType.



--
To view, visit http://gerrit.cloudera.org:8080/23552
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If37a4dd2d1689aa51ab09e6cb71f01664dc2ee1a
Gerrit-Change-Number: 23552
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Xuebin Su <[email protected]>
Gerrit-Comment-Date: Thu, 16 Oct 2025 07:24:29 +0000
Gerrit-HasComments: Yes

Reply via email to