feat: update LockRequestQueue::request_queue_ & fix typos & ensure format consistency (#576)

* feat: Add std::shared_ptr for the request_queue_ in LockManager::LockRequestQueue

* fix: Fix typos & Ensure format consistency

* fix: Fix typos & Ensure format consistency

* feat: Change TableOid() to GetTableOid(), ensure the consistency of function names

* feat: Add book keeping note to give a hint for students on PJ4, T3

Author: xzhseh

src/include/buffer/buffer_pool_manager.h

@@ -35,7 +35,7 @@ class BufferPoolManager {
    * @brief Creates a new BufferPoolManager.
    * @param pool_size the size of the buffer pool
    * @param disk_manager the disk manager
-   * @param replacer_k the lookback constant k for the LRU-K replacer
+   * @param replacer_k the LookBack constant k for the LRU-K replacer
    * @param log_manager the log manager (for testing only: nullptr = disable logging). Please ignore this for P1.
    */
   BufferPoolManager(size_t pool_size, DiskManager *disk_manager, size_t replacer_k = LRUK_REPLACER_K,

src/include/buffer/lru_k_replacer.h

@@ -44,7 +44,7 @@ class LRUKNode {
  * current timestamp and the timestamp of kth previous access.
  *
  * A frame with less than k historical references is given
- * +inf as its backward k-distance. When multipe frames have +inf backward k-distance,
+ * +inf as its backward k-distance. When multiple frames have +inf backward k-distance,
  * classical LRU algorithm is used to choose victim.
  */
 class LRUKReplacer {

src/include/concurrency/lock_manager.h

@@ -65,7 +65,7 @@ class LockManager {
   class LockRequestQueue {
    public:
     /** List of lock requests for the same resource (table or row) */
-    std::list<LockRequest *> request_queue_;
+    std::list<std::shared_ptr<LockRequest>> request_queue_;
     /** For notifying blocked transactions on this rid */
     std::condition_variable cv_;
     /** txn_id of an upgrading transaction (if any) */
@@ -154,8 +154,12 @@ class LockManager {
    *    - If requested lock mode is the same as that of the lock presently held,
    *      Lock() should return true since it already has the lock.
    *    - If requested lock mode is different, Lock() should upgrade the lock held by the transaction.
+   *    - Basically there should be three steps to perform a lock upgrade in general
+   *      - 1. Check the precondition of upgrade
+   *      - 2. Drop the current lock, reserve the upgrade position
+   *      - 3. Wait to get the new lock granted
    *
-   *    A lock request being upgraded should be prioritised over other waiting lock requests on the same resource.
+   *    A lock request being upgraded should be prioritized over other waiting lock requests on the same resource.
    *
    *    While upgrading, only the following transitions should be allowed:
    *        IS -> [S, X, IX, SIX]
@@ -173,6 +177,9 @@ class LockManager {
    * BOOK KEEPING:
    *    If a lock is granted to a transaction, lock manager should update its
    *    lock sets appropriately (check transaction.h)
+   *
+   *    You probably want to consider which type of lock to directly apply on table
+   *    when implementing executor later
    */
 
   /**

src/include/container/disk/hash/disk_extendible_hash_table.h

@@ -90,7 +90,7 @@ class DiskExtendibleHashTable {
    * for extendible hashing.
    *
    * @param key the key to hash
-   * @return the downcasted 32-bit hash
+   * @return the down-casted 32-bit hash
    */
   inline auto Hash(KeyType key) -> uint32_t;
 

src/include/execution/executors/aggregation_executor.h

@@ -198,10 +198,13 @@ class AggregationExecutor : public AbstractExecutor {
  private:
   /** The aggregation plan node */
   const AggregationPlanNode *plan_;
+
   /** The child executor that produces tuples over which the aggregation is computed */
   std::unique_ptr<AbstractExecutor> child_executor_;
+
   /** Simple aggregation hash table */
   // TODO(Student): Uncomment SimpleAggregationHashTable aht_;
+
   /** Simple aggregation hash table iterator */
   // TODO(Student): Uncomment SimpleAggregationHashTable::Iterator aht_iterator_;
 };

src/include/execution/executors/delete_executor.h

@@ -58,6 +58,7 @@ class DeleteExecutor : public AbstractExecutor {
  private:
   /** The delete plan node to be executed */
   const DeletePlanNode *plan_;
+
   /** The child executor from which RIDs for deleted tuples are pulled */
   std::unique_ptr<AbstractExecutor> child_executor_;
 };

src/include/execution/executors/limit_executor.h

@@ -51,6 +51,7 @@ class LimitExecutor : public AbstractExecutor {
  private:
   /** The limit plan node to be executed */
   const LimitPlanNode *plan_;
+
   /** The child executor from which tuples are obtained */
   std::unique_ptr<AbstractExecutor> child_executor_;
 };

src/include/execution/executors/nested_index_join_executor.h

@@ -34,8 +34,8 @@ class NestIndexJoinExecutor : public AbstractExecutor {
  public:
   /**
    * Creates a new nested index join executor.
-   * @param exec_ctx the context that the hash join should be performed in
-   * @param plan the nested index join plan node
+   * @param exec_ctx the context that the nested index join should be performed in
+   * @param plan the nested index join plan to be executed
    * @param child_executor the outer table
    */
   NestIndexJoinExecutor(ExecutorContext *exec_ctx, const NestedIndexJoinPlanNode *plan,

src/include/execution/executors/nested_loop_join_executor.h

@@ -30,7 +30,7 @@ class NestedLoopJoinExecutor : public AbstractExecutor {
   /**
    * Construct a new NestedLoopJoinExecutor instance.
    * @param exec_ctx The executor context
-   * @param plan The NestedLoop join plan to be executed
+   * @param plan The nested loop join plan to be executed
    * @param left_executor The child executor that produces tuple for the left side of join
    * @param right_executor The child executor that produces tuple for the right side of join
    */

src/include/execution/executors/topn_check_executor.h

@@ -22,15 +22,15 @@
 namespace bustub {
 
 /**
- * TopNCheckExecutor checks the number of items in topn executor container
+ * TopNCheckExecutor checks the number of items in TopN executor container
  */
 class TopNCheckExecutor : public AbstractExecutor {
  public:
   /**
    * Construct a new TopNCheckExecutor instance.
    * @param exec_ctx The executor context
    * @param plan The TopN plan to be executed
-   * @param child_executor The topn child executor
+   * @param child_executor The TopN child executor
    */
   TopNCheckExecutor(ExecutorContext *exec_ctx, const TopNPlanNode *plan,
                     std::unique_ptr<AbstractExecutor> &&child_executor, TopNExecutor *topn_executor);