-
Notifications
You must be signed in to change notification settings - Fork 619
[15721] Sequence second PR #1345
Changes from all commits
30d48ac
086b493
59b85f6
282afb3
28b702e
ddb3dd7
26230e3
b273056
d90a649
c1a4e1d
f8ec42a
0d3521c
8fe7d6c
87680be
7d7955c
503c89f
7762354
b340c86
6897566
b8c7317
d665c94
e71a21c
ca8f8fe
85af574
7997a9a
35d6a5a
6b12584
bd4210e
80ad516
e6811f9
9106ba8
28045be
3d93dc6
fc4a385
527bfc5
836ffc0
5c760c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,8 +13,10 @@ | |
| #include <memory> | ||
|
|
||
| #include "catalog/catalog_cache.h" | ||
| #include <boost/functional/hash.hpp> | ||
|
|
||
| #include "catalog/database_catalog.h" | ||
| #include "catalog/sequence_catalog.h" | ||
| #include "common/logger.h" | ||
|
|
||
| namespace peloton { | ||
|
|
@@ -157,5 +159,76 @@ std::shared_ptr<IndexCatalogObject> CatalogCache::GetCachedIndexObject( | |
| return nullptr; | ||
| } | ||
|
|
||
| /*@brief insert sequence catalog object into cache | ||
| * @param sequence_object | ||
| * @return false only if sequence already exists in cache | ||
| */ | ||
| bool CatalogCache::InsertSequenceObject( | ||
| std::shared_ptr<SequenceCatalogObject> sequence_object) { | ||
| if (!sequence_object || sequence_object->seq_oid == INVALID_OID) { | ||
| return false; // invalid object | ||
| } | ||
|
|
||
| std::size_t hash_key = GetHashKey(sequence_object->seq_name, | ||
| sequence_object->db_oid); | ||
|
|
||
| // check if already in cache | ||
| if (sequence_objects_cache.find(hash_key) != | ||
| sequence_objects_cache.end()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to throw an exception for this case? |
||
| LOG_DEBUG("Sequence %s already exists in cache!", | ||
| sequence_object->seq_name.c_str()); | ||
| return false; | ||
| } | ||
|
|
||
| sequence_objects_cache.insert( | ||
| std::make_pair(hash_key, sequence_object)); | ||
| return true; | ||
| } | ||
|
|
||
| /*@brief evict sequence catalog object from cache | ||
| * @param sequence_name, database_oid | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each parameter should be put on a separate line. |
||
| * @return true if specified sequence is found and evicted; | ||
| * false if not found | ||
| */ | ||
| bool CatalogCache::EvictSequenceObject(const std::string & sequence_name, | ||
| oid_t database_oid) { | ||
| std::size_t hash_key = GetHashKey(sequence_name, database_oid); | ||
|
|
||
| auto it = sequence_objects_cache.find(hash_key); | ||
| if (it == sequence_objects_cache.end()) { | ||
| return false; // sequence not found in cache | ||
| } | ||
|
|
||
| auto sequence_object = it->second; | ||
| PELOTON_ASSERT(sequence_object); | ||
| sequence_objects_cache.erase(it); | ||
| return true; | ||
| } | ||
|
|
||
| /*@brief get sequence catalog object from cache | ||
| * @param sequence_name, database_oid | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each parameter should be put on a separate line. |
||
| * @return sequence catalog object; if not found return object with invalid oid | ||
| */ | ||
| std::shared_ptr<SequenceCatalogObject> CatalogCache::GetSequenceObject( | ||
| const std::string & sequence_name, oid_t database_oid) { | ||
| std::size_t hash_key = GetHashKey(sequence_name, database_oid); | ||
| auto it = sequence_objects_cache.find(hash_key); | ||
| if (it == sequence_objects_cache.end()) { | ||
| return nullptr; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It said it return object with invalid oid if not found. To make things consistent, instead of returning nullptr, we should probably return an SequenceCatalogObject with seq_oid = INVALID_OID. Or another easier way is to change the spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a consistent behavior with the existing methods (like GetTableObejct, etc.) in catalog_cache.cpp so I would like to keep it. |
||
| } | ||
| return it->second; | ||
| } | ||
|
|
||
| /*@brief get the hash key given the sequence information | ||
| * @param sequence_name, database_oid | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each parameter should be put on a separate line. |
||
| * @return hash key | ||
| */ | ||
| std::size_t CatalogCache::GetHashKey(const std::string sequence_name, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure but wouldn't using reference, const std::string &, avoid unnecessary copies of strings? |
||
| oid_t database_oid) { | ||
| std::tuple<std::string, size_t> key(sequence_name, database_oid); | ||
| boost::hash<std::tuple<std::string, size_t>> key_hash; | ||
| return key_hash(key); | ||
| } | ||
|
|
||
| } // namespace catalog | ||
| } // namespace peloton | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, false doesn't indicate the sequence exists in the cache. Probably need to change the @return spec or requires the sequence_object to be valid.