Fix create with serial #1812
Merged
Fix create with serial #1812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
acquamarin
commented
Jul 13, 2023
•
acquamarin
commented
- Fix create node with null property bug.
- Disable updates to struct property column.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1812 +/- ##
=======================================
Coverage 91.20% 91.21%
=======================================
Files 777 777
Lines 28449 28463 +14
=======================================
+ Hits 25948 25963 +15
+ Misses 2501 2500 -1
☔ View full report in Codecov by Sentry. |
ray6080
reviewed
Jul 14, 2023
| @@ -102,9 +102,11 @@ class CatalogContent { | |||
| const Property& getRelProperty( | |||
| common::table_id_t tableID, const std::string& propertyName) const; | |||
|
|
|||
| std::vector<Property> getAllNodeProperties(common::table_id_t tableID) const; | |||
| inline const std::vector<Property>& getNodeProperties(common::table_id_t tableID) const { | |||
There was a problem hiding this comment.
I'm thinking if we should enforce a naming style for returning reference of a value, like getNodePropertiesRef, etc. What do you think?
| @@ -86,6 +86,9 @@ bool Column::isNull(common::offset_t nodeOffset, transaction::Transaction* trans | |||
|
|
|||
| void Column::setNull(common::offset_t nodeOffset) { | |||
| nullColumn->setValue(nodeOffset); | |||
| auto walPageInfo = createWALVersionOfPageIfNecessaryForElement(nodeOffset, numElementsPerPage); | |||
There was a problem hiding this comment.
hmm I think we only need these newly added lines when we are adding a new node, right? Can we change CreateNode to go through Column::write? So we can gradually get rid of setNull interface, whose functionality is overlapped with write.
Let's discuss this a bit more when you are online.
ray6080
approved these changes
Jul 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.