This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 708fdff19 [refactor] Fix some lint and tidy errors
708fdff19 is described below

commit 708fdff19073dbe9dc4eb9492551443aa38111f9
Author: Yingchun Lai <acelyc1112...@gmail.com>
AuthorDate: Wed Jul 13 23:56:02 2022 +0800

    [refactor] Fix some lint and tidy errors
    
    This patch has no functional changes, it fixes some lint and
    tidy errors, it's the first step to implement immutable column
    feature, aiming to reduce the diff code when we review that
    patch in https://gerrit.cloudera.org/c/18241/.
    
    Change-Id: I89b0f3b9ace02f380ce1193bc1facf2aaf135096
    Reviewed-on: http://gerrit.cloudera.org:8080/18724
    Tested-by: Alexey Serbin <ale...@apache.org>
    Reviewed-by: Alexey Serbin <ale...@apache.org>
---
 src/kudu/client/write_op.h                  | 43 ++++++++++++++---------------
 src/kudu/common/row.h                       |  1 +
 src/kudu/common/row_changelist.cc           |  8 ++----
 src/kudu/common/row_changelist.h            |  4 +--
 src/kudu/common/schema-test.cc              | 14 +++++-----
 src/kudu/tablet/delta_applier.cc            | 12 ++++----
 src/kudu/tablet/delta_applier.h             | 30 ++++++++++----------
 src/kudu/tablet/delta_iterator_merger.cc    | 16 ++++-------
 src/kudu/tablet/delta_iterator_merger.h     |  2 +-
 src/kudu/tablet/delta_store.h               |  6 ++--
 src/kudu/tablet/deltafile.cc                |  2 +-
 src/kudu/tablet/deltafile.h                 |  2 +-
 src/kudu/tablet/deltamemstore-test.cc       |  4 +--
 src/kudu/tablet/deltamemstore.cc            |  2 +-
 src/kudu/tablet/deltamemstore.h             |  2 +-
 src/kudu/tablet/ops/op.h                    |  3 +-
 src/kudu/tablet/rowset.cc                   |  1 -
 src/kudu/tablet/rowset.h                    |  2 +-
 src/kudu/tablet/tablet-decoder-eval-test.cc |  6 ++--
 src/kudu/tablet/tablet.cc                   |  7 ++---
 20 files changed, 78 insertions(+), 89 deletions(-)

diff --git a/src/kudu/client/write_op.h b/src/kudu/client/write_op.h
index a1e9e30a6..e5b76488e 100644
--- a/src/kudu/client/write_op.h
+++ b/src/kudu/client/write_op.h
@@ -30,7 +30,6 @@
 
 #ifdef KUDU_HEADERS_NO_STUBS
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/port.h"
 #else
 #include "kudu/client/stubs.h"
 #endif
@@ -141,16 +140,16 @@ class KUDU_EXPORT KuduWriteOperation {
 ///   columns which do not have default values.
 class KUDU_EXPORT KuduInsert : public KuduWriteOperation {
  public:
-  virtual ~KuduInsert();
+  ~KuduInsert() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "INSERT " + 
row_.ToString(); }
+  std::string ToString() const override { return "INSERT " + row_.ToString(); }
 
  protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return INSERT;
   }
 
@@ -168,16 +167,16 @@ class KUDU_EXPORT KuduInsert : public KuduWriteOperation {
 ///   columns which do not have default values.
 class KUDU_EXPORT KuduInsertIgnore : public KuduWriteOperation {
  public:
-  virtual ~KuduInsertIgnore();
+  ~KuduInsertIgnore() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "INSERT IGNORE " + 
row_.ToString(); }
+  std::string ToString() const override { return "INSERT IGNORE " + 
row_.ToString(); }
 
  protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return INSERT_IGNORE;
   }
 
@@ -194,16 +193,16 @@ class KUDU_EXPORT KuduInsertIgnore : public 
KuduWriteOperation {
 /// See KuduInsert for more details.
 class KUDU_EXPORT KuduUpsert : public KuduWriteOperation {
  public:
-  virtual ~KuduUpsert();
+  ~KuduUpsert() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "UPSERT " + 
row_.ToString(); }
+  std::string ToString() const override { return "UPSERT " + row_.ToString(); }
 
  protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return UPSERT;
   }
 
@@ -221,16 +220,16 @@ class KUDU_EXPORT KuduUpsert : public KuduWriteOperation {
 ///   in the schema to be set in the embedded KuduPartialRow object.
 class KUDU_EXPORT KuduUpdate : public KuduWriteOperation {
  public:
-  virtual ~KuduUpdate();
+  ~KuduUpdate() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "UPDATE " + 
row_.ToString(); }
+  std::string ToString() const override { return "UPDATE " + row_.ToString(); }
 
  protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return UPDATE;
   }
 
@@ -247,16 +246,16 @@ class KUDU_EXPORT KuduUpdate : public KuduWriteOperation {
 ///   in the schema to be set in the embedded KuduPartialRow object.
 class KUDU_EXPORT KuduUpdateIgnore : public KuduWriteOperation {
 public:
-  virtual ~KuduUpdateIgnore();
+  ~KuduUpdateIgnore() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "UPDATE IGNORE " + 
row_.ToString(); }
+  std::string ToString() const override { return "UPDATE IGNORE " + 
row_.ToString(); }
 
 protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return UPDATE_IGNORE;
   }
 
@@ -273,16 +272,16 @@ private:
 ///   KuduPartialRow object.
 class KUDU_EXPORT KuduDelete : public KuduWriteOperation {
  public:
-  virtual ~KuduDelete();
+  ~KuduDelete() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "DELETE " + 
row_.ToString(); }
+  std::string ToString() const override { return "DELETE " + row_.ToString(); }
 
  protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return DELETE;
   }
 
@@ -299,16 +298,16 @@ class KUDU_EXPORT KuduDelete : public KuduWriteOperation {
 ///   KuduPartialRow object.
 class KUDU_EXPORT KuduDeleteIgnore : public KuduWriteOperation {
 public:
-  virtual ~KuduDeleteIgnore();
+  ~KuduDeleteIgnore() override;
 
   /// @copydoc KuduWriteOperation::ToString()
-  virtual std::string ToString() const OVERRIDE { return "DELETE IGNORE " + 
row_.ToString(); }
+  std::string ToString() const override { return "DELETE IGNORE " + 
row_.ToString(); }
 
 protected:
   /// @cond PROTECTED_MEMBERS_DOCUMENTED
 
   /// @copydoc KuduWriteOperation::type()
-  virtual Type type() const OVERRIDE {
+  Type type() const override {
     return DELETE_IGNORE;
   }
 
diff --git a/src/kudu/common/row.h b/src/kudu/common/row.h
index aa6324122..f040edf47 100644
--- a/src/kudu/common/row.h
+++ b/src/kudu/common/row.h
@@ -251,6 +251,7 @@ class RowProjector {
  private:
   DISALLOW_COPY_AND_ASSIGN(RowProjector);
 
+  // projection_ column index -> base_schema_ index
   std::vector<ProjectionIdxMapping> base_cols_mapping_;
   std::vector<size_t> projection_defaults_;
 
diff --git a/src/kudu/common/row_changelist.cc 
b/src/kudu/common/row_changelist.cc
index c20fce107..480085a06 100644
--- a/src/kudu/common/row_changelist.cc
+++ b/src/kudu/common/row_changelist.cc
@@ -263,11 +263,9 @@ Status 
RowChangeListDecoder::MutateRowAndCaptureChanges(RowBlockRow* dst_row,
   return Status::OK();
 }
 
-
-
 Status RowChangeListDecoder::ApplyToOneColumn(size_t row_idx, ColumnBlock* 
dst_col,
                                               const Schema& dst_schema,
-                                              int col_idx, Arena *arena) {
+                                              int col_idx, Arena* arena) {
   DCHECK(is_reinsert() || is_update());
 
   const ColumnSchema& col_schema = dst_schema.column(col_idx);
@@ -281,14 +279,14 @@ Status RowChangeListDecoder::ApplyToOneColumn(size_t 
row_idx, ColumnBlock* dst_c
     }
 
     int junk_col_idx;
-    const void* new_val;
+    const void* new_val = nullptr;
     RETURN_NOT_OK(dec.Validate(dst_schema, &junk_col_idx, &new_val));
     DCHECK_EQ(junk_col_idx, col_idx);
 
     SimpleConstCell src(&col_schema, new_val);
     ColumnBlock::Cell dst_cell = dst_col->cell(row_idx);
     RETURN_NOT_OK(CopyCell(src, &dst_cell, arena));
-    // TODO: could potentially break; here if we're guaranteed to only have 
one update
+    // TODO(unknown): could potentially break; here if we're guaranteed to 
only have one update
     // per column in a RowChangeList (which would make sense!)
   }
   return Status::OK();
diff --git a/src/kudu/common/row_changelist.h b/src/kudu/common/row_changelist.h
index e648d3dc1..08df9a87e 100644
--- a/src/kudu/common/row_changelist.h
+++ b/src/kudu/common/row_changelist.h
@@ -251,9 +251,7 @@ void RowChangeListEncoder::SetToReinsert(const RowType& 
src_row) {
   DCHECK_EQ(RowChangeList::kUninitialized, type_);
   SetType(RowChangeList::kReinsert);
   const Schema* schema = src_row.schema();
-  for (int i = 0; i < schema->num_columns(); ++i) {
-    // Reinserts don't need to store the keys.
-    if (schema->is_key_column(i)) continue;
+  for (int i = schema->num_key_columns(); i < schema->num_columns(); ++i) {
     ColumnId col_id = schema->column_id(i);
     const ColumnSchema& col_schema = schema->column(i);
     if (col_schema.is_nullable() && src_row.is_null(i)) {
diff --git a/src/kudu/common/schema-test.cc b/src/kudu/common/schema-test.cc
index 14817a0ee..6a78cbd46 100644
--- a/src/kudu/common/schema-test.cc
+++ b/src/kudu/common/schema-test.cc
@@ -233,13 +233,13 @@ TEST_P(ParameterizedSchemaTest, TestCopyAndMove) {
 TEST_F(TestSchema, TestSchemaWithDecimal) {
   ColumnSchema col1("key", STRING);
   ColumnSchema col2("decimal32val", DECIMAL32, false,
-                    NULL, NULL, ColumnStorageAttributes(),
+                    nullptr, nullptr, ColumnStorageAttributes(),
                     ColumnTypeAttributes(9, 4));
   ColumnSchema col3("decimal64val", DECIMAL64, true,
-                    NULL, NULL, ColumnStorageAttributes(),
+                    nullptr, nullptr, ColumnStorageAttributes(),
                     ColumnTypeAttributes(18, 10));
   ColumnSchema col4("decimal128val", DECIMAL128, true,
-                    NULL, NULL, ColumnStorageAttributes(),
+                    nullptr, nullptr, ColumnStorageAttributes(),
                     ColumnTypeAttributes(38, 2));
 
   vector<ColumnSchema> cols = { col1, col2, col3, col4 };
@@ -267,16 +267,16 @@ TEST_F(TestSchema, TestSchemaWithDecimal) {
 TEST_F(TestSchema, TestSchemaEqualsWithDecimal) {
   ColumnSchema col1("key", STRING);
   ColumnSchema col_18_10("decimal64val", DECIMAL64, true,
-                         NULL, NULL, ColumnStorageAttributes(),
+                         nullptr, nullptr, ColumnStorageAttributes(),
                          ColumnTypeAttributes(18, 10));
   ColumnSchema col_18_9("decimal64val", DECIMAL64, true,
-                        NULL, NULL, ColumnStorageAttributes(),
+                        nullptr, nullptr, ColumnStorageAttributes(),
                         ColumnTypeAttributes(18, 9));
   ColumnSchema col_17_10("decimal64val", DECIMAL64, true,
-                         NULL, NULL, ColumnStorageAttributes(),
+                         nullptr, nullptr, ColumnStorageAttributes(),
                          ColumnTypeAttributes(17, 10));
   ColumnSchema col_17_9("decimal64val", DECIMAL64, true,
-                        NULL, NULL, ColumnStorageAttributes(),
+                        nullptr, nullptr, ColumnStorageAttributes(),
                         ColumnTypeAttributes(17, 9));
 
   Schema schema_18_10({ col1, col_18_10 }, 1);
diff --git a/src/kudu/tablet/delta_applier.cc b/src/kudu/tablet/delta_applier.cc
index cd567c88f..deb75ffad 100644
--- a/src/kudu/tablet/delta_applier.cc
+++ b/src/kudu/tablet/delta_applier.cc
@@ -43,7 +43,7 @@ struct IteratorStats;
 
 namespace tablet {
 
-  // Construct. The base_iter and delta_iter should not be Initted.
+// Construct. The base_iter and delta_iter should not be Initted.
 DeltaApplier::DeltaApplier(RowIteratorOptions opts,
                            shared_ptr<CFileSet::Iterator> base_iter,
                            unique_ptr<DeltaIterator> delta_iter)
@@ -55,7 +55,7 @@ DeltaApplier::DeltaApplier(RowIteratorOptions opts,
 DeltaApplier::~DeltaApplier() {
 }
 
-Status DeltaApplier::Init(ScanSpec *spec) {
+Status DeltaApplier::Init(ScanSpec* spec) {
   RETURN_NOT_OK(base_iter_->Init(spec));
   RETURN_NOT_OK(delta_iter_->Init(spec));
   return Status::OK();
@@ -72,7 +72,7 @@ string DeltaApplier::ToString() const {
   return s;
 }
 
-const Schema &DeltaApplier::schema() const {
+const Schema& DeltaApplier::schema() const {
   return base_iter_->schema();
 }
 
@@ -84,7 +84,7 @@ bool DeltaApplier::HasNext() const {
   return base_iter_->HasNext();
 }
 
-Status DeltaApplier::PrepareBatch(size_t *nrows) {
+Status DeltaApplier::PrepareBatch(size_t* nrows) {
   // The initial seek is deferred from Init() into the first PrepareBatch()
   // because it requires a loaded delta file, and we don't want to require
   // that at Init() time.
@@ -106,7 +106,7 @@ Status DeltaApplier::FinishBatch() {
   return base_iter_->FinishBatch();
 }
 
-Status DeltaApplier::InitializeSelectionVector(SelectionVector *sel_vec) {
+Status DeltaApplier::InitializeSelectionVector(SelectionVector* sel_vec) {
   DCHECK(!first_prepare_) << "PrepareBatch() must be called at least once";
 
   // A diff scan will set both 'snap_to_exclude' and 'include_deleted_rows'.
@@ -129,7 +129,7 @@ Status 
DeltaApplier::InitializeSelectionVector(SelectionVector *sel_vec) {
   return Status::OK();
 }
 
-Status DeltaApplier::MaterializeColumn(ColumnMaterializationContext *ctx) {
+Status DeltaApplier::MaterializeColumn(ColumnMaterializationContext* ctx) {
   DCHECK(!first_prepare_) << "PrepareBatch() must be called at least once";
   // Data with updates cannot be evaluated at the decoder-level.
   if (delta_iter_->MayHaveDeltas()) {
diff --git a/src/kudu/tablet/delta_applier.h b/src/kudu/tablet/delta_applier.h
index cdd99aac5..e0608340b 100644
--- a/src/kudu/tablet/delta_applier.h
+++ b/src/kudu/tablet/delta_applier.h
@@ -26,7 +26,6 @@
 
 #include "kudu/common/iterator.h"
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/port.h"
 #include "kudu/tablet/cfile_set.h"
 #include "kudu/tablet/rowset.h"
 #include "kudu/util/status.h"
@@ -47,42 +46,41 @@ class DeltaIterator;
 // Delta-applying iterators
 ////////////////////////////////////////////////////////////
 
-// A DeltaApplier takes in a base ColumnwiseIterator along with a a
+// A DeltaApplier takes in a base ColumnwiseIterator along with a
 // DeltaIterator. It is responsible for applying the updates coming
 // from the delta iterator to the results of the base iterator.
-class DeltaApplier : public ColumnwiseIterator {
+class DeltaApplier final : public ColumnwiseIterator {
  public:
-  virtual Status Init(ScanSpec *spec) OVERRIDE;
-  Status PrepareBatch(size_t *nrows) OVERRIDE;
+  Status Init(ScanSpec* spec) override;
+  Status PrepareBatch(size_t* nrows) override;
 
-  Status FinishBatch() OVERRIDE;
+  Status FinishBatch() override;
 
-  bool HasNext() const OVERRIDE;
+  bool HasNext() const override;
 
-  std::string ToString() const OVERRIDE;
+  std::string ToString() const override;
 
-  const Schema &schema() const OVERRIDE;
+  const Schema& schema() const override;
 
-  virtual void GetIteratorStats(std::vector<IteratorStats>* stats) const 
OVERRIDE;
+  void GetIteratorStats(std::vector<IteratorStats>* stats) const override;
 
   // Initialize the selection vector for the current batch.
   // This processes DELETEs -- any deleted rows are set to 0 in 'sel_vec'.
   // All other rows are set to 1.
-  virtual Status InitializeSelectionVector(SelectionVector *sel_vec) OVERRIDE;
+  Status InitializeSelectionVector(SelectionVector* sel_vec) override;
+
+  Status MaterializeColumn(ColumnMaterializationContext* ctx) override;
 
-  Status MaterializeColumn(ColumnMaterializationContext *ctx) override;
  private:
   friend class DeltaTracker;
 
   FRIEND_TEST(TestMajorDeltaCompaction, TestCompact);
 
-  DISALLOW_COPY_AND_ASSIGN(DeltaApplier);
-
   // Construct. The base_iter and delta_iter should not be Initted.
   DeltaApplier(RowIteratorOptions opts,
                std::shared_ptr<CFileSet::Iterator> base_iter,
                std::unique_ptr<DeltaIterator> delta_iter);
-  virtual ~DeltaApplier();
+  ~DeltaApplier() override;
 
   const RowIteratorOptions opts_;
 
@@ -90,6 +88,8 @@ class DeltaApplier : public ColumnwiseIterator {
   std::unique_ptr<DeltaIterator> delta_iter_;
 
   bool first_prepare_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeltaApplier);
 };
 
 } // namespace tablet
diff --git a/src/kudu/tablet/delta_iterator_merger.cc 
b/src/kudu/tablet/delta_iterator_merger.cc
index e3d0af005..1afb2f722 100644
--- a/src/kudu/tablet/delta_iterator_merger.cc
+++ b/src/kudu/tablet/delta_iterator_merger.cc
@@ -19,6 +19,7 @@
 
 #include <algorithm>
 
+#include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tablet/delta_key.h"
 
@@ -118,7 +119,7 @@ Status DeltaIteratorMerger::FilterColumnIdsAndCollectDeltas(
   return Status::OK();
 }
 
-bool DeltaIteratorMerger::HasNext() {
+bool DeltaIteratorMerger::HasNext() const {
   for (const unique_ptr<DeltaIterator>& iter : iters_) {
     if (iter->HasNext()) {
       return true;
@@ -140,16 +141,9 @@ bool DeltaIteratorMerger::MayHaveDeltas() const {
 string DeltaIteratorMerger::ToString() const {
   string ret;
   ret.append("DeltaIteratorMerger(");
-
-  bool first = true;
-  for (const unique_ptr<DeltaIterator> &iter : iters_) {
-    if (!first) {
-      ret.append(", ");
-    }
-    first = false;
-
-    ret.append(iter->ToString());
-  }
+  ret.append(JoinMapped(iters_, [](const unique_ptr<DeltaIterator> &iter) {
+        return iter->ToString();
+      }, ", "));
   ret.append(")");
   return ret;
 }
diff --git a/src/kudu/tablet/delta_iterator_merger.h 
b/src/kudu/tablet/delta_iterator_merger.h
index 4c87e4de2..13547c8d5 100644
--- a/src/kudu/tablet/delta_iterator_merger.h
+++ b/src/kudu/tablet/delta_iterator_merger.h
@@ -81,7 +81,7 @@ class DeltaIteratorMerger : public DeltaIterator {
                                          std::vector<DeltaKeyAndUpdate>* out,
                                          Arena* arena) override;
 
-  bool HasNext() override;
+  bool HasNext() const override;
 
   bool MayHaveDeltas() const override;
 
diff --git a/src/kudu/tablet/delta_store.h b/src/kudu/tablet/delta_store.h
index ce59f54be..3b30ea0db 100644
--- a/src/kudu/tablet/delta_store.h
+++ b/src/kudu/tablet/delta_store.h
@@ -340,7 +340,7 @@ class DeltaIterator : public PreparedDeltas {
     // Prepare a batch of deltas for applying. All deltas in the batch will be
     // decoded. Operations affecting row data (i.e. UPDATEs and REINSERTs) will
     // be coalesced into a column-major data structure suitable for
-    // ApplyUpdates. Operations affecting row lifecycle (i.e. DELETES and
+    // ApplyUpdates. Operations affecting row lifecycle (i.e. DELETEs and
     // REINSERTs) will be coalesced into a row-major data structure suitable 
for ApplyDeletes.
     //
     // On success, ApplyUpdates and ApplyDeltas will be callable.
@@ -362,7 +362,7 @@ class DeltaIterator : public PreparedDeltas {
   virtual Status PrepareBatch(size_t nrows, int prepare_flags) = 0;
 
   // Returns true if there are any more rows left in this iterator.
-  virtual bool HasNext() = 0;
+  virtual bool HasNext() const = 0;
 
   // Return a string representation suitable for debug printouts.
   virtual std::string ToString() const = 0;
@@ -558,7 +558,7 @@ class DeltaPreparer : public PreparedDeltas {
 
   // Used for PREPARED_FOR_APPLY mode.
   //
-  // Set to true in all of the spots where deleted, reinserted_, and 
updates_by_col_
+  // Set to true in all of the spots where deleted_, reinserted_, and 
updates_by_col_
   // are modified.
   bool may_have_deltas_;
 
diff --git a/src/kudu/tablet/deltafile.cc b/src/kudu/tablet/deltafile.cc
index 5d6f6999a..cdc87f86f 100644
--- a/src/kudu/tablet/deltafile.cc
+++ b/src/kudu/tablet/deltafile.cc
@@ -733,7 +733,7 @@ Status 
DeltaFileIterator<Type>::CollectMutations(vector<Mutation*>* dst, Arena*
 }
 
 template<DeltaType Type>
-bool DeltaFileIterator<Type>::HasNext() {
+bool DeltaFileIterator<Type>::HasNext() const {
   return !exhausted_ || !delta_blocks_.empty();
 }
 
diff --git a/src/kudu/tablet/deltafile.h b/src/kudu/tablet/deltafile.h
index 44ab7d8cc..57156557a 100644
--- a/src/kudu/tablet/deltafile.h
+++ b/src/kudu/tablet/deltafile.h
@@ -267,7 +267,7 @@ class DeltaFileIterator : public DeltaIterator {
 
   std::string ToString() const override;
 
-  bool HasNext() override;
+  bool HasNext() const override;
 
   bool MayHaveDeltas() const override;
 
diff --git a/src/kudu/tablet/deltamemstore-test.cc 
b/src/kudu/tablet/deltamemstore-test.cc
index 7c4946e2d..9cbeda980 100644
--- a/src/kudu/tablet/deltamemstore-test.cc
+++ b/src/kudu/tablet/deltamemstore-test.cc
@@ -597,7 +597,7 @@ TEST_F(TestDeltaMemStore, TestCollectMutations) {
   opts.projection = &schema_;
   opts.snap_to_include = MvccSnapshot(mvcc_);
   unique_ptr<DeltaIterator> iter;
-  Status s =  dms_->NewDeltaIterator(opts, &iter);
+  Status s = dms_->NewDeltaIterator(opts, &iter);
   if (s.IsNotFound()) {
     FAIL() << "Iterator fell outside of the range of the snapshot";
   }
@@ -636,7 +636,7 @@ TEST_F(TestDeltaMemStore, TestCollectMutations) {
   }
 }
 
-// Generates a series of random deltas,  writes them to a DMS, reads them back
+// Generates a series of random deltas, writes them to a DMS, reads them back
 // using a DMSIterator, and verifies the results.
 TEST_F(TestDeltaMemStore, TestFuzz) {
   // Arbitrary constants to control the running time and coverage of the test.
diff --git a/src/kudu/tablet/deltamemstore.cc b/src/kudu/tablet/deltamemstore.cc
index a8460f530..3b7b48991 100644
--- a/src/kudu/tablet/deltamemstore.cc
+++ b/src/kudu/tablet/deltamemstore.cc
@@ -302,7 +302,7 @@ Status DMSIterator::FilterColumnIdsAndCollectDeltas(const 
vector<ColumnId>& col_
   return preparer_.FilterColumnIdsAndCollectDeltas(col_ids, out, arena);
 }
 
-bool DMSIterator::HasNext() {
+bool DMSIterator::HasNext() const {
   return iter_->IsValid();
 }
 
diff --git a/src/kudu/tablet/deltamemstore.h b/src/kudu/tablet/deltamemstore.h
index 318c0fd6d..2fd4da36d 100644
--- a/src/kudu/tablet/deltamemstore.h
+++ b/src/kudu/tablet/deltamemstore.h
@@ -239,7 +239,7 @@ class DMSIterator : public DeltaIterator {
 
   std::string ToString() const override;
 
-  bool HasNext() override;
+  bool HasNext() const override;
 
   bool MayHaveDeltas() const override;
 
diff --git a/src/kudu/tablet/ops/op.h b/src/kudu/tablet/ops/op.h
index 024b53d3e..155b003f1 100644
--- a/src/kudu/tablet/ops/op.h
+++ b/src/kudu/tablet/ops/op.h
@@ -28,6 +28,7 @@
 #include <google/protobuf/arena.h>
 
 #include "kudu/common/common.pb.h"
+#include "kudu/common/schema.h"
 #include "kudu/common/timestamp.h"
 #include "kudu/common/wire_protocol.h"
 #include "kudu/consensus/consensus.pb.h"
@@ -49,8 +50,6 @@ class Message;
 
 namespace kudu {
 
-class Schema;
-
 namespace tablet {
 class OpCompletionCallback;
 class OpState;
diff --git a/src/kudu/tablet/rowset.cc b/src/kudu/tablet/rowset.cc
index 18192aa12..11b70989c 100644
--- a/src/kudu/tablet/rowset.cc
+++ b/src/kudu/tablet/rowset.cc
@@ -143,7 +143,6 @@ Status DuplicatingRowSet::NewCompactionInput(const Schema* 
/*projection*/,
   return Status::OK();
 }
 
-
 Status DuplicatingRowSet::MutateRow(Timestamp timestamp,
                                     const RowSetKeyProbe &probe,
                                     const RowChangeList &update,
diff --git a/src/kudu/tablet/rowset.h b/src/kudu/tablet/rowset.h
index efb68af05..f759cb584 100644
--- a/src/kudu/tablet/rowset.h
+++ b/src/kudu/tablet/rowset.h
@@ -33,7 +33,6 @@
 #include "kudu/common/rowid.h"
 #include "kudu/common/timestamp.h"
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/port.h"
 #include "kudu/tablet/mvcc.h"
 #include "kudu/util/bloom_filter.h"
 #include "kudu/util/status.h"
@@ -41,6 +40,7 @@
 
 namespace kudu {
 
+class Arena;
 class MonoTime; // IWYU pragma: keep
 class RowChangeList;
 class RowwiseIterator;
diff --git a/src/kudu/tablet/tablet-decoder-eval-test.cc 
b/src/kudu/tablet/tablet-decoder-eval-test.cc
index fbd92a9d8..86f5b02b3 100644
--- a/src/kudu/tablet/tablet-decoder-eval-test.cc
+++ b/src/kudu/tablet/tablet-decoder-eval-test.cc
@@ -75,10 +75,12 @@ class TabletDecoderEvalTest : public KuduTabletTest,
 public:
   TabletDecoderEvalTest()
           : KuduTabletTest(Schema({ColumnSchema("key", INT32),
-                                   ColumnSchema("string_val_a", STRING, true, 
NULL, NULL,
+                                   ColumnSchema("string_val_a", STRING, true,
+                                                nullptr, nullptr,
                                                 
ColumnStorageAttributes(DICT_ENCODING,
                                                                         
DEFAULT_COMPRESSION)),
-                                   ColumnSchema("string_val_b", STRING, true, 
NULL, NULL,
+                                   ColumnSchema("string_val_b", STRING, true,
+                                                nullptr, nullptr,
                                                 
ColumnStorageAttributes(DICT_ENCODING,
                                                                         
DEFAULT_COMPRESSION))}, 1))
   {}
diff --git a/src/kudu/tablet/tablet.cc b/src/kudu/tablet/tablet.cc
index 1290d280f..bf691c2d4 100644
--- a/src/kudu/tablet/tablet.cc
+++ b/src/kudu/tablet/tablet.cc
@@ -701,7 +701,7 @@ Status Tablet::ValidateInsertOrUpsertUnlocked(const RowOp& 
op) {
 Status Tablet::ValidateMutateUnlocked(const RowOp& op) {
   RowChangeListDecoder rcl_decoder(op.decoded_op.changelist);
   RETURN_NOT_OK(rcl_decoder.Init());
-  if (rcl_decoder.is_reinsert()) {
+  if (PREDICT_FALSE(rcl_decoder.is_reinsert())) {
     // REINSERT mutations are the byproduct of an INSERT on top of a ghost
     // row, not something the user is allowed to specify on their own.
     return Status::InvalidArgument("User may not specify REINSERT mutations");
@@ -748,6 +748,7 @@ Status Tablet::InsertOrUpsertUnlocked(const IOContext* 
io_context,
     }
   }
 
+  DCHECK(!op->present_in_rowset);
   Timestamp ts = op_state->timestamp();
   ConstContiguousRow row(schema().get(), op->decoded_op.row_data);
 
@@ -828,9 +829,7 @@ Status Tablet::ApplyUpsertAsUpdate(const IOContext* 
io_context,
   ConstContiguousRow row(schema, upsert->decoded_op.row_data);
   faststring buf;
   RowChangeListEncoder enc(&buf);
-  for (int i = 0; i < schema->num_columns(); i++) {
-    if (schema->is_key_column(i)) continue;
-
+  for (int i = schema->num_key_columns(); i < schema->num_columns(); i++) {
     // If the user didn't explicitly set this column in the UPSERT, then we 
should
     // not turn it into an UPDATE. This prevents the UPSERT from updating
     // values back to their defaults when unset.

Reply via email to