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

Default values #28

Merged
merged 21 commits into from May 5, 2019
Merged

Default values #28

merged 21 commits into from May 5, 2019

Conversation

saikiriti93
Copy link
Collaborator

Default values for MV tables. Fixes #24

PS: My C++ is pretty bad! So please feel free to suggest improvements.

@codecov-io
Copy link

codecov-io commented Apr 18, 2019

Codecov Report

Merging #28 into schema_change will increase coverage by 0.05%.
The diff coverage is 96.15%.

Impacted file tree graph

@@                Coverage Diff                @@
##           schema_change      #28      +/-   ##
=================================================
+ Coverage          88.11%   88.17%   +0.05%     
=================================================
  Files                190      190              
  Lines               7242     7287      +45     
=================================================
+ Hits                6381     6425      +44     
- Misses               861      862       +1
Impacted Files Coverage Δ
src/storage/projected_row.cpp 100% <ø> (ø) ⬆️
src/include/storage/sql_table.h 96.22% <ø> (ø) ⬆️
src/include/storage/storage_defs.h 94.44% <ø> (ø) ⬆️
src/storage/data_table.cpp 96.66% <ø> (-0.03%) ⬇️
src/storage/sql_table.cpp 95.97% <100%> (+1.01%) ⬆️
src/include/catalog/schema.h 96.66% <84.61%> (-3.34%) ⬇️
src/transaction/transaction_manager.cpp 95.27% <0%> (+0.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 614be3e...9384ea0. Read the comment docs.

Copy link
Collaborator

@jrolli jrolli left a comment

Choose a reason for hiding this comment

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

The code to apply default values looks clean, but I think the code applies default values in cases where it shouldn't. A test suite just for default values would help keep us on track.

src/include/catalog/schema.h Outdated Show resolved Hide resolved
src/include/catalog/schema.h Outdated Show resolved Hide resolved
src/include/catalog/schema.h Outdated Show resolved Hide resolved
src/include/catalog/schema.h Outdated Show resolved Hide resolved
src/include/storage/sql_table.h Outdated Show resolved Hide resolved
test/storage/sql_table_test.cpp Show resolved Hide resolved
test/storage/sql_table_test.cpp Show resolved Hide resolved
src/storage/sql_table.cpp Show resolved Hide resolved
src/storage/sql_table.cpp Outdated Show resolved Hide resolved
src/storage/sql_table.cpp Outdated Show resolved Hide resolved
@jrolli jrolli requested review from yangjuns and yashNaN April 18, 2019 15:26
@jrolli jrolli added the In Progress This PR is in progress and not ready to be reviewed label Apr 25, 2019
@saikiriti93 saikiriti93 added ReadyForReview This PR is ready for review and removed In Progress This PR is in progress and not ready to be reviewed labels May 4, 2019
@saikiriti93 saikiriti93 requested a review from jrolli May 4, 2019 23:59
@saikiriti93 saikiriti93 merged commit 3562388 into schema_change May 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyForReview This PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Default Values
3 participants