Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alter table add column #1186

Merged
merged 1 commit into from
Jan 19, 2023
Merged

Alter table add column #1186

merged 1 commit into from
Jan 19, 2023

Conversation

acquamarin
Copy link
Collaborator

@acquamarin acquamarin commented Jan 17, 2023

This PR adds support for adding column to an existing node/rel table.
The grammar for adding column is:
ALTER TABLE person ADD COLUMN count INTEGER DEFAULT 25;
If the user doesn't give a default value, the newly added column will be filled with null values.

Copy link
Contributor

@ray6080 ray6080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should start following the principle to separate query processing with storage details. So let's discuss and move towards that.

src/include/catalog/catalog.h Outdated Show resolved Hide resolved
src/include/catalog/catalog_structs.h Outdated Show resolved Hide resolved
src/include/storage/store/rel_table.h Outdated Show resolved Hide resolved
@@ -341,6 +341,11 @@ void RelTable::initEmptyRelsForNewNode(nodeID_t& nodeID) {
listsUpdatesStore->initNewlyAddedNodes(nodeID);
}

void RelTable::addProperty(Property property, TableSchema* tableSchema) {
fwdRelTableData->addProperty(property, tableSchema, wal, bufferManager);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass wal and bufferManager as private fields to DirectedRelTableData, so you don't need to pass them here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAL is also used by relTable, so we need to store WAL in relTable. I don't think it is a good idea to store it in DirectedRelTableData as well.

src/storage/store/rel_table.cpp Outdated Show resolved Hide resolved
src/include/storage/store/rels_statistics.h Show resolved Hide resolved
src/processor/operator/ddl/add_property.cpp Show resolved Hide resolved
src/processor/operator/ddl/add_property.cpp Outdated Show resolved Hide resolved
src/storage/storage_structure/in_mem_file.cpp Outdated Show resolved Hide resolved
@@ -222,6 +222,31 @@ void InMemOverflowFile::copyListOverflow(InMemOverflowFile* srcInMemOverflowFile
dstKUList->size * numBytesOfListElement);
}

void InMemOverflowFile::copyListOverflow(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has potential deadlocks. See #1112

src/antlr4/Cypher.g4 Outdated Show resolved Hide resolved
src/antlr4/Cypher.g4 Outdated Show resolved Hide resolved
src/antlr4/Cypher.g4 Show resolved Hide resolved
src/parser/transformer.cpp Outdated Show resolved Hide resolved
src/binder/bind/bind_ddl.cpp Outdated Show resolved Hide resolved
src/binder/bind/bind_ddl.cpp Outdated Show resolved Hide resolved
src/include/binder/binder.h Outdated Show resolved Hide resolved
src/include/processor/operator/ddl/add_property.h Outdated Show resolved Hide resolved
src/processor/operator/ddl/add_rel_property.cpp Outdated Show resolved Hide resolved
@acquamarin acquamarin force-pushed the add-property branch 3 times, most recently from 709e1a9 to 67dba6c Compare January 18, 2023 07:10
@acquamarin acquamarin merged commit e6b75db into master Jan 19, 2023
@acquamarin acquamarin deleted the add-property branch January 19, 2023 03:27
@ray6080 ray6080 changed the title support alter table add property Alter table add column Jan 29, 2023
@ray6080 ray6080 added the feature New features or missing components of existing features label Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features or missing components of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants