noisepage-checks[bot]
commented
3 years ago Major Decrease in Performance
STOP: this PR has a major negative performance impact
| tps (%change) | benchmark_type | wal_device | details |
|---|---|---|---|
| 0.01% | tpcc | RAM disk | Detailsmaster tps=22075.29, commit tps=22077.46, 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=29682.29, commit tps=29236.31, 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 |
| -1.84% | tpcc | HDD | Detailsmaster tps=21863.97, commit tps=21462.16, 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 |
| -0.89% | tatp | RAM disk | Detailsmaster tps=6691.93, commit tps=6632.34, 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 |
| -8.34% | tatp | None | Detailsmaster tps=7611.21, commit tps=6976.28, 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 |
| -4.03% | tatp | HDD | Detailsmaster tps=6718.74, commit tps=6447.76, 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[bot]
mbutrovich
Background:
type::TypeIdhas been another bee in my bonnet likeTransientValuewas, which was also a stopgap as other parts of the system were built without the execution engine or optimizer yet. As of right now we have about 5 different type definitions in the system:PostgresValueTypein the network layer, Peloton-liketype::TypeIdin/include/type, other Postgres types in the parser, andSqlTypeIdandTypeIdin/include/execution/sql.Overview:
Remove
type::TypeIdand replace withexecution::sql::SqlTypeId.Changes:
/typefolder.TypeId::VAR_ARRAY. Doesn't seem like it was used and not a real type.ColumnMapInfofromColumnMapin theSqlTable. This went in with TPL2 and stashed type info at the storage layer. That's a huge footgun for how we plan to do schema change in the future, and the catalog is the sole authority of transactional schema information.CatalogAccessorjust like their temp tables.TableVectorIteratorwas relying on theColumnMapInfoand now requests schema for all tables fromCatalogAccessor, so this is transparent to the execution engine.VarlenEntrymoved to its own header file.Future work:
SqlTypeId::Variadicwhich is used in pg_proc incorrectly. This will take a larger rewrite of pg_proc behavior to actually match Postgres (see #1359). I tried to do it in this PR, but the blast radius gets bigger than I'd like for an already-large PR.PostgresValueTypewithSqlTypeIdto reduce type translation. This would bump the latter to being anint32_tand has a large blast radius.