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

Change subject: KUDU-2671 update partition schema in catalog when dropping range
......................................................................


Patch Set 8:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc
File src/kudu/common/partition-test.cc:

http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1492
PS8, Line 1492: RangeSchemaPB,
> nit: RangeWithHashSchemaPB
actually, that's about PartitionSchemaPB and its field custom_hash_schema_ranges


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1494
PS8, Line 1494: tablet
> nit: table
Done


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@1497
PS8, Line 1497: RangeSchemaPB
> nit: RangeWithHashSchemaPB
Done


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition-test.cc@2163
PS8, Line 2163: truly
> nit: remove 'truly'
Done


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h
File src/kudu/common/partition.h:

http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition.h@310
PS8, Line 310: with
> nit: remove extra with
Done


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc
File src/kudu/common/partition_pruner-test.cc:

http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/common/partition_pruner-test.cc@1124
PS8, Line 1124:  PartitionSchema::RangesWithHashSchemas ranges;
              :   ASSERT_OK(PartitionSchema::FromPB(pb, schema, 
&partition_schema, &ranges));
              :
              :   vector<Partition> partitions;
              :   ASSERT_OK(partition_schema.CreatePartitions(ranges, schema, 
&partitions));
> So to understand the usage of 'ranges'. Now since we're only storing the ra
Yes, that's right: I found this way it would be cleaner.


http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/18642/8/src/kudu/master/catalog_manager.cc@1939
PS8, Line 1939: based on specified ranges with custom hash schemas.
> Doesn't this method create partitions for each range, regardless of custom
Right.  As you can see, there have been too many iterations on this relatively 
small feature, and comments are falling behind :)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78afdd1a358751dca43f564c5d8e69191f165d4
Gerrit-Change-Number: 18642
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Comment-Date: Wed, 29 Jun 2022 02:56:46 +0000
Gerrit-HasComments: Yes

Reply via email to