Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Add Validation to Column Constructor #1627

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

turingcompl33t
Copy link
Contributor

The CTE PR added a new constructor for the Column type but neglected to include a call to Column::Validate() to validate instances upon construction. This PR adds this missing validation step.

@turingcompl33t turingcompl33t added in-progress This PR is being actively worked on and not ready to be reviewed or merged. Mark PRs with this. ready-for-ci Indicate that this build should be run through CI. bug Something isn't working (correctness). Mark issues with this. labels Jul 15, 2021
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-2.64% tpcc RAM disk
Detailsmaster tps=23290.82, commit tps=22676.77, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
-1.5% tpcc None
Detailsmaster tps=29928.3, commit tps=29478.48, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
0.01% tpcc HDD
Detailsmaster tps=22270.74, commit tps=22274.05, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
4.57% tatp RAM disk
Detailsmaster tps=6708.41, commit tps=7015.21, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
1.52% tatp None
Detailsmaster tps=7614.91, commit tps=7730.55, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.64% tatp HDD
Detailsmaster tps=6553.99, commit tps=6923.8, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #1627 (9b0512c) into master (ab85c96) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1627      +/-   ##
==========================================
+ Coverage   80.76%   80.82%   +0.05%     
==========================================
  Files         774      774              
  Lines       55360    55360              
==========================================
+ Hits        44713    44742      +29     
+ Misses      10647    10618      -29     
Impacted Files Coverage Δ
src/include/catalog/schema.h 97.08% <100.00%> (ø)
src/execution/sema/sema_decl.cpp 79.24% <0.00%> (-1.89%) ⬇️
src/storage/index/hash_index.cpp 91.13% <0.00%> (ø)
src/execution/sema/sema_builtin.cpp 60.54% <0.00%> (ø)
src/execution/vm/bytecode_generator.cpp 84.00% <0.00%> (+0.04%) ⬆️
src/include/storage/index/bplustree.h 92.06% <0.00%> (+0.10%) ⬆️
src/traffic_cop/traffic_cop.cpp 68.91% <0.00%> (+0.64%) ⬆️
src/execution/sema/sema_checking.cpp 72.84% <0.00%> (+0.66%) ⬆️
src/storage/garbage_collector.cpp 94.21% <0.00%> (+0.82%) ⬆️
...xecution/compiler/operator/operator_translator.cpp 62.71% <0.00%> (+0.84%) ⬆️
... and 6 more

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 ab85c96...9b0512c. Read the comment docs.

@mbutrovich mbutrovich added ready-to-merge This PR is ready to be merged. Mark PRs with this. and removed in-progress This PR is being actively worked on and not ready to be reviewed or merged. Mark PRs with this. labels Jul 16, 2021
@mbutrovich mbutrovich merged commit a628efe into cmu-db:master Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working (correctness). Mark issues with this. ready-for-ci Indicate that this build should be run through CI. ready-to-merge This PR is ready to be merged. Mark PRs with this.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants