From a0da596bb12875c6056b04a89fc141260c80bc48 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Fri, 16 Oct 2020 14:20:14 -0400 Subject: [PATCH] tpcc: remove unused and unpartitionable stock_item_fk_idx This was discussed in #50911. We ended up not making the change because it didn't appear to improve performance. Given the fact that the index is only used in a single place and can easily be replaced by the primary key of the stock table, it doesn't seem possible that this would actually hurt performance. However, it does seem possible for the index to hurt performance in multi-region clusters, where the fk validation was not guaranteed to be local because it was not using the partitioned primary key of the stock table. This allows us to remove a scary comment around partitioning code, because the index was not partitionable by warehouse id. --- pkg/ccl/importccl/read_import_avro_test.go | 5 ++--- pkg/sql/opt/bench/bench_test.go | 3 +-- pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema | 1 - pkg/sql/opt/xform/testdata/external/tpcc | 4 ++-- pkg/sql/opt/xform/testdata/external/tpcc-later-stats | 4 ++-- pkg/sql/opt/xform/testdata/external/tpcc-no-stats | 4 ++-- pkg/workload/tpcc/ddls.go | 2 +- pkg/workload/tpcc/partition.go | 3 --- pkg/workload/tpcc/tpcc.go | 4 ++-- 9 files changed, 12 insertions(+), 18 deletions(-) diff --git a/pkg/ccl/importccl/read_import_avro_test.go b/pkg/ccl/importccl/read_import_avro_test.go index 5d3fbba55dfa..c818b185d57a 100644 --- a/pkg/ccl/importccl/read_import_avro_test.go +++ b/pkg/ccl/importccl/read_import_avro_test.go @@ -570,9 +570,8 @@ func benchmarkAvroImport(b *testing.B, avroOpts roachpb.AvroOptions, testData st s_order_cnt integer, s_remote_cnt integer, s_data varchar(50), - primary key (s_w_id, s_i_id), - index stock_item_fk_idx (s_i_id)) - `) + primary key (s_w_id, s_i_id) +)`) require.NoError(b, err) diff --git a/pkg/sql/opt/bench/bench_test.go b/pkg/sql/opt/bench/bench_test.go index 94a6e0738d5b..78af7d553cb0 100644 --- a/pkg/sql/opt/bench/bench_test.go +++ b/pkg/sql/opt/bench/bench_test.go @@ -146,8 +146,7 @@ var schemas = [...]string{ s_order_cnt integer, s_remote_cnt integer, s_data varchar(50), - primary key (s_w_id, s_i_id), - index stock_item_fk_idx (s_i_id) + primary key (s_w_id, s_i_id) ) `, ` diff --git a/pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema b/pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema index dbee7b0c1fa2..66c38d98449e 100644 --- a/pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema +++ b/pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema @@ -141,7 +141,6 @@ CREATE TABLE stock s_remote_cnt integer, s_data varchar(50), primary key (s_w_id, s_i_id), - index stock_item_fk_idx (s_i_id), foreign key (s_w_id) references warehouse (w_id), foreign key (s_i_id) references item (i_id) ) interleave in parent warehouse (s_w_id) diff --git a/pkg/sql/opt/xform/testdata/external/tpcc b/pkg/sql/opt/xform/testdata/external/tpcc index b07f66394f66..0b3e5f9be6f1 100644 --- a/pkg/sql/opt/xform/testdata/external/tpcc +++ b/pkg/sql/opt/xform/testdata/external/tpcc @@ -404,9 +404,9 @@ insert order_line │ │ └── cardinality: [6 - 6] │ └── filters (true) └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id) - └── anti-join (lookup stock@stock_item_fk_idx) + └── anti-join (lookup stock) ├── columns: column6:35!null column5:36!null - ├── key columns: [36 35] = [37 38] + ├── key columns: [35 36] = [38 37] ├── lookup columns are key ├── cardinality: [0 - 6] ├── with-scan &1 diff --git a/pkg/sql/opt/xform/testdata/external/tpcc-later-stats b/pkg/sql/opt/xform/testdata/external/tpcc-later-stats index 24a63fc01906..358aa551aba7 100644 --- a/pkg/sql/opt/xform/testdata/external/tpcc-later-stats +++ b/pkg/sql/opt/xform/testdata/external/tpcc-later-stats @@ -407,9 +407,9 @@ insert order_line │ │ └── cardinality: [6 - 6] │ └── filters (true) └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id) - └── anti-join (lookup stock@stock_item_fk_idx) + └── anti-join (lookup stock) ├── columns: column6:35!null column5:36!null - ├── key columns: [36 35] = [37 38] + ├── key columns: [35 36] = [38 37] ├── lookup columns are key ├── cardinality: [0 - 6] ├── with-scan &1 diff --git a/pkg/sql/opt/xform/testdata/external/tpcc-no-stats b/pkg/sql/opt/xform/testdata/external/tpcc-no-stats index 777c7247a5f1..25e469a6aa4a 100644 --- a/pkg/sql/opt/xform/testdata/external/tpcc-no-stats +++ b/pkg/sql/opt/xform/testdata/external/tpcc-no-stats @@ -401,9 +401,9 @@ insert order_line │ │ └── cardinality: [6 - 6] │ └── filters (true) └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id) - └── anti-join (lookup stock@stock_item_fk_idx) + └── anti-join (lookup stock) ├── columns: column6:35!null column5:36!null - ├── key columns: [36 35] = [37 38] + ├── key columns: [35 36] = [38 37] ├── lookup columns are key ├── cardinality: [0 - 6] ├── with-scan &1 diff --git a/pkg/workload/tpcc/ddls.go b/pkg/workload/tpcc/ddls.go index 64d394c906ac..01874030fbdd 100644 --- a/pkg/workload/tpcc/ddls.go +++ b/pkg/workload/tpcc/ddls.go @@ -154,7 +154,7 @@ const ( s_remote_cnt integer, s_data varchar(50), primary key (s_w_id, s_i_id)` - tpccStockSchemaFkSuffix = ` + deprecatedTpccStockSchemaFkSuffix = ` index stock_item_fk_idx (s_i_id)` tpccStockSchemaInterleaveSuffix = ` interleave in parent warehouse (s_w_id)` diff --git a/pkg/workload/tpcc/partition.go b/pkg/workload/tpcc/partition.go index cd46cc7f6881..dcdd2bcc1e5e 100644 --- a/pkg/workload/tpcc/partition.go +++ b/pkg/workload/tpcc/partition.go @@ -347,9 +347,6 @@ func partitionOrderLine(db *gosql.DB, cfg zoneConfig, wPart *partitioner) error } func partitionStock(db *gosql.DB, cfg zoneConfig, wPart *partitioner) error { - // The stock_item_fk_idx can't be partitioned because it doesn't have a - // warehouse prefix. It's an all-around unfortunate index that we only - // need because of a restriction in SQL. See #36859 and #37255. return partitionTable(db, cfg, wPart, "stock", "s_w_id", 0) } diff --git a/pkg/workload/tpcc/tpcc.go b/pkg/workload/tpcc/tpcc.go index f5f1c2fec563..a50b97d369b8 100644 --- a/pkg/workload/tpcc/tpcc.go +++ b/pkg/workload/tpcc/tpcc.go @@ -540,9 +540,9 @@ func (w *tpcc) Tables() []workload.Table { Schema: maybeAddInterleaveSuffix( w.interleaved, maybeAddFkSuffix( - w.fks, + w.deprecatedFkIndexes, tpccStockSchemaBase, - tpccStockSchemaFkSuffix, + deprecatedTpccStockSchemaFkSuffix, ), tpccStockSchemaInterleaveSuffix, ),