From 4cb0003e486c1af13d6abccf6e457cea82de1194 Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Tue, 8 Feb 2022 16:09:03 -0500 Subject: [PATCH 1/4] server: remove DeprecateBaseEncryptionRegistry Remove the migration server DeprecateBaseEncryptionRegistry RPC. It was used for the migration of the encryption-at-rest registry format in 21.2 and is no longer relevant. Missed this in #74314. Release note: None --- pkg/server/migration.go | 31 ----------------------------- pkg/server/serverpb/migration.proto | 14 ------------- 2 files changed, 45 deletions(-) diff --git a/pkg/server/migration.go b/pkg/server/migration.go index 1861f3954352..03f70c0a2495 100644 --- a/pkg/server/migration.go +++ b/pkg/server/migration.go @@ -210,37 +210,6 @@ func (m *migrationServer) PurgeOutdatedReplicas( return resp, nil } -// TODO(ayang): remove this RPC and associated request/response in 22.1 -func (m *migrationServer) DeprecateBaseEncryptionRegistry( - ctx context.Context, req *serverpb.DeprecateBaseEncryptionRegistryRequest, -) (*serverpb.DeprecateBaseEncryptionRegistryResponse, error) { - const opName = "deprecate-base-encryption-registry" - ctx, span := m.server.AnnotateCtxWithSpan(ctx, opName) - defer span.Finish() - ctx = logtags.AddTag(ctx, opName, nil) - - if err := m.server.stopper.RunTaskWithErr(ctx, opName, func( - ctx context.Context, - ) error { - // Same as in SyncAllEngines, because stores can be added asynchronously, we - // need to ensure that the bootstrap process has happened. - m.server.node.waitForAdditionalStoreInit() - - for _, eng := range m.server.engines { - if err := eng.SetMinVersion(*req.Version); err != nil { - return err - } - } - - return nil - }); err != nil { - return nil, err - } - - resp := &serverpb.DeprecateBaseEncryptionRegistryResponse{} - return resp, nil -} - // WaitForSpanConfigSubscription implements the MigrationServer interface. func (m *migrationServer) WaitForSpanConfigSubscription( ctx context.Context, _ *serverpb.WaitForSpanConfigSubscriptionRequest, diff --git a/pkg/server/serverpb/migration.proto b/pkg/server/serverpb/migration.proto index 57b2e69e91a9..38ee1302fc17 100644 --- a/pkg/server/serverpb/migration.proto +++ b/pkg/server/serverpb/migration.proto @@ -52,16 +52,6 @@ message SyncAllEnginesRequest{} // SyncAllEnginesResponse is the response to a SyncAllEnginesRequest. message SyncAllEnginesResponse{} -// DeprecateBaseEncryptionRegistryRequest is used to instruct the target node -// to stop using the Base version monolithic encryption-at-rest registry. -message DeprecateBaseEncryptionRegistryRequest { - roachpb.Version version = 1; -} - -// DeprecateBaseEncryptionRegistryResponse is the response to a -// DeprecateBaseEncryptionRegistryRequest. -message DeprecateBaseEncryptionRegistryResponse{} - // WaitForSpanConfigSubscriptionRequest waits until the target node is wholly // subscribed to the global span configurations state. message WaitForSpanConfigSubscriptionRequest{} @@ -95,10 +85,6 @@ service Migration { // replicas with a version less than the one provided. rpc PurgeOutdatedReplicas (PurgeOutdatedReplicasRequest) returns (PurgeOutdatedReplicasResponse) { } - // DeprecateBaseRegistry is used to instruct the target node to stop - // using the Base version monolithic encryption-at-rest registry. - rpc DeprecateBaseEncryptionRegistry (DeprecateBaseEncryptionRegistryRequest) returns (DeprecateBaseEncryptionRegistryResponse) { } - // WaitForSpanConfigSubscription waits until the target node is wholly // subscribed to the global span configurations state. rpc WaitForSpanConfigSubscription (WaitForSpanConfigSubscriptionRequest) returns (WaitForSpanConfigSubscriptionResponse) { } From 1ea5e413a8030fac25f5b44b7db7bd2f6afa588a Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Sat, 5 Feb 2022 13:48:32 -0500 Subject: [PATCH 2/4] sql: use 16 as default bucket count for hash index for some reason I forgot to modify it in my previous pr. Release note (sql change): 16 is used as the default bucket count for hash sharded index. --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/sql/catalog/catconstants/constants.go | 2 +- .../testdata/logic_test/hash_sharded_index | 32 +++++++++---------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index 468c4d11aa5f..be1dfc7020d1 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -85,7 +85,7 @@ sql.cross_db_views.enabled boolean false if true, creating views that refer to o sql.defaults.cost_scans_with_default_col_size.enabled boolean false setting to true uses the same size for all columns to compute scan cost sql.defaults.datestyle enumeration iso, mdy default value for DateStyle session setting [iso, mdy = 0, iso, dmy = 1, iso, ymd = 2] sql.defaults.datestyle.enabled boolean false default value for datestyle_enabled session setting -sql.defaults.default_hash_sharded_index_bucket_count integer 8 used as bucket count if bucket count is not specified in hash sharded index definition +sql.defaults.default_hash_sharded_index_bucket_count integer 16 used as bucket count if bucket count is not specified in hash sharded index definition sql.defaults.default_int_size integer 8 the size, in bytes, of an INT type sql.defaults.disallow_full_table_scans.enabled boolean false setting to true rejects queries that have planned a full table scan sql.defaults.distsql enumeration auto default distributed SQL execution mode [off = 0, auto = 1, on = 2, always = 3] diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 783bdef619ae..9844dfa3ca21 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -96,7 +96,7 @@ sql.defaults.cost_scans_with_default_col_size.enabledbooleanfalsesetting to true uses the same size for all columns to compute scan cost sql.defaults.datestyleenumerationiso, mdydefault value for DateStyle session setting [iso, mdy = 0, iso, dmy = 1, iso, ymd = 2] sql.defaults.datestyle.enabledbooleanfalsedefault value for datestyle_enabled session setting -sql.defaults.default_hash_sharded_index_bucket_countinteger8used as bucket count if bucket count is not specified in hash sharded index definition +sql.defaults.default_hash_sharded_index_bucket_countinteger16used as bucket count if bucket count is not specified in hash sharded index definition sql.defaults.default_int_sizeinteger8the size, in bytes, of an INT type sql.defaults.disallow_full_table_scans.enabledbooleanfalsesetting to true rejects queries that have planned a full table scan sql.defaults.distsqlenumerationautodefault distributed SQL execution mode [off = 0, auto = 1, on = 2, always = 3] diff --git a/pkg/sql/catalog/catconstants/constants.go b/pkg/sql/catalog/catconstants/constants.go index 16520caa5d8f..0f76e7d66edb 100644 --- a/pkg/sql/catalog/catconstants/constants.go +++ b/pkg/sql/catalog/catconstants/constants.go @@ -385,6 +385,6 @@ var DefaultHashShardedIndexBucketCount = settings.RegisterIntSetting( settings.TenantWritable, "sql.defaults.default_hash_sharded_index_bucket_count", "used as bucket count if bucket count is not specified in hash sharded index definition", - 8, + 16, settings.NonNegativeInt, ).WithPublic() diff --git a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index index daab0d9a6477..8c5ce19dffe5 100644 --- a/pkg/sql/logictest/testdata/logic_test/hash_sharded_index +++ b/pkg/sql/logictest/testdata/logic_test/hash_sharded_index @@ -885,46 +885,46 @@ t_hash_pre_split 142 t_hash_pre_split_idx_b /Table/142/2/7 /Max subtest test_default_bucket_count statement ok -CREATE TABLE t_default_bucket_8 ( +CREATE TABLE t_default_bucket_16 ( a INT PRIMARY KEY USING HASH, b INT, c INT, - INDEX idx_t_default_bucket_8_b (b) USING HASH, - INDEX idx_t_default_bucket_8_c (c) USING HASH WITH (bucket_count=4), + INDEX idx_t_default_bucket_16_b (b) USING HASH, + INDEX idx_t_default_bucket_16_c (c) USING HASH WITH (bucket_count=4), FAMILY fam_0_a (a), FAMILY fam_1_c_b (c, b) ); query T -SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_8] +SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_16] ---- -CREATE TABLE public.t_default_bucket_8 ( - crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL, +CREATE TABLE public.t_default_bucket_16 ( + crdb_internal_a_shard_16 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 16:::INT8)) VIRTUAL, a INT8 NOT NULL, b INT8 NULL, c INT8 NULL, - crdb_internal_b_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(b)), 8:::INT8)) VIRTUAL, + crdb_internal_b_shard_16 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(b)), 16:::INT8)) VIRTUAL, crdb_internal_c_shard_4 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(c)), 4:::INT8)) VIRTUAL, - CONSTRAINT t_default_bucket_8_pkey PRIMARY KEY (a ASC) USING HASH WITH (bucket_count=8), - INDEX idx_t_default_bucket_8_b (b ASC) USING HASH WITH (bucket_count=8), - INDEX idx_t_default_bucket_8_c (c ASC) USING HASH WITH (bucket_count=4), + CONSTRAINT t_default_bucket_16_pkey PRIMARY KEY (a ASC) USING HASH WITH (bucket_count=16), + INDEX idx_t_default_bucket_16_b (b ASC) USING HASH WITH (bucket_count=16), + INDEX idx_t_default_bucket_16_c (c ASC) USING HASH WITH (bucket_count=4), FAMILY fam_0_a (a), FAMILY fam_1_c_b (c, b) ) statement ok -SET CLUSTER SETTING sql.defaults.default_hash_sharded_index_bucket_count = 16 +SET CLUSTER SETTING sql.defaults.default_hash_sharded_index_bucket_count = 8 statement ok -CREATE TABLE t_default_bucket_16 (a INT PRIMARY KEY USING HASH); +CREATE TABLE t_default_bucket_8 (a INT PRIMARY KEY USING HASH); query T -SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_16] +SELECT @2 FROM [SHOW CREATE TABLE t_default_bucket_8] ---- -CREATE TABLE public.t_default_bucket_16 ( - crdb_internal_a_shard_16 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 16:::INT8)) VIRTUAL, +CREATE TABLE public.t_default_bucket_8 ( + crdb_internal_a_shard_8 INT4 NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a)), 8:::INT8)) VIRTUAL, a INT8 NOT NULL, - CONSTRAINT t_default_bucket_16_pkey PRIMARY KEY (a ASC) USING HASH WITH (bucket_count=16), + CONSTRAINT t_default_bucket_8_pkey PRIMARY KEY (a ASC) USING HASH WITH (bucket_count=8), FAMILY "primary" (a) ) From a7f60f2572a0b92ad2e485b652f6373a74d4e017 Mon Sep 17 00:00:00 2001 From: Max Neverov Date: Fri, 4 Feb 2022 06:59:08 +0100 Subject: [PATCH 3/4] physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions. See #58347. Release note (performance improvement): regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions are now evaluated more efficiently in a distributed setting --- pkg/sql/distsql/columnar_operators_test.go | 5 + pkg/sql/execinfra/version.go | 10 +- pkg/sql/execinfrapb/aggregate_funcs.go | 5 + pkg/sql/execinfrapb/processors_sql.proto | 5 + .../logictest/testdata/logic_test/distsql_agg | 10 + .../opt/exec/execbuilder/testdata/distsql_agg | 23 +- pkg/sql/physicalplan/aggregator_funcs.go | 50 ++++ pkg/sql/sem/builtins/aggregate_builtins.go | 264 +++++++++++++++--- 8 files changed, 318 insertions(+), 54 deletions(-) diff --git a/pkg/sql/distsql/columnar_operators_test.go b/pkg/sql/distsql/columnar_operators_test.go index 082239f2799b..86c160bb2e5b 100644 --- a/pkg/sql/distsql/columnar_operators_test.go +++ b/pkg/sql/distsql/columnar_operators_test.go @@ -93,6 +93,11 @@ var aggregateFuncToNumArguments = map[execinfrapb.AggregatorSpec_Func]int{ execinfrapb.FinalRegrSxx: 1, execinfrapb.FinalRegrSxy: 1, execinfrapb.FinalRegrSyy: 1, + execinfrapb.FinalRegrAvgx: 1, + execinfrapb.FinalRegrAvgy: 1, + execinfrapb.FinalRegrIntercept: 1, + execinfrapb.FinalRegrR2: 1, + execinfrapb.FinalRegrSlope: 1, } // TestAggregateFuncToNumArguments ensures that all aggregate functions are diff --git a/pkg/sql/execinfra/version.go b/pkg/sql/execinfra/version.go index 8fff24396d63..5fd80a774804 100644 --- a/pkg/sql/execinfra/version.go +++ b/pkg/sql/execinfra/version.go @@ -39,7 +39,7 @@ import "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" // // ATTENTION: When updating these fields, add a brief description of what // changed to the version history below. -const Version execinfrapb.DistSQLVersion = 60 +const Version execinfrapb.DistSQLVersion = 61 // MinAcceptedVersion is the oldest version that the server is compatible with. // A server will not accept flows with older versions. @@ -51,6 +51,14 @@ const MinAcceptedVersion execinfrapb.DistSQLVersion = 60 Please add new entries at the top. +- Version: 61 (MinAcceptedVersion: 60) + - final_regr_avgx, final_regr_avgy, final_regr_intercept, final_regr_r2, and + final_regr_slope aggregate functions were introduced to support local and + final aggregation of the corresponding builtin functions. It would be + unrecognized by a server running older versions, hence the version bump. + However, a server running v61 can still process all plans from servers + running v60, thus the MinAcceptedVersion is kept at 60. + - Version: 60 (MinAcceptedVersion: 60): - Deprecated ExportWriterSpec and ParquetWriterSpec and merged them into ExportSpec diff --git a/pkg/sql/execinfrapb/aggregate_funcs.go b/pkg/sql/execinfrapb/aggregate_funcs.go index 2a5a5ce8e23e..028e47cee91e 100644 --- a/pkg/sql/execinfrapb/aggregate_funcs.go +++ b/pkg/sql/execinfrapb/aggregate_funcs.go @@ -65,4 +65,9 @@ const ( FinalRegrSxx = AggregatorSpec_FINAL_REGR_SXX FinalRegrSxy = AggregatorSpec_FINAL_REGR_SXY FinalRegrSyy = AggregatorSpec_FINAL_REGR_SYY + FinalRegrAvgx = AggregatorSpec_FINAL_REGR_AVGX + FinalRegrAvgy = AggregatorSpec_FINAL_REGR_AVGY + FinalRegrIntercept = AggregatorSpec_FINAL_REGR_INTERCEPT + FinalRegrR2 = AggregatorSpec_FINAL_REGR_R2 + FinalRegrSlope = AggregatorSpec_FINAL_REGR_SLOPE ) diff --git a/pkg/sql/execinfrapb/processors_sql.proto b/pkg/sql/execinfrapb/processors_sql.proto index dc165a3477a1..4b68f42979d3 100644 --- a/pkg/sql/execinfrapb/processors_sql.proto +++ b/pkg/sql/execinfrapb/processors_sql.proto @@ -832,6 +832,11 @@ message AggregatorSpec { FINAL_REGR_SXX = 50; FINAL_REGR_SXY = 51; FINAL_REGR_SYY = 52; + FINAL_REGR_AVGX = 53; + FINAL_REGR_AVGY = 54; + FINAL_REGR_INTERCEPT = 55; + FINAL_REGR_R2 = 56; + FINAL_REGR_SLOPE = 57; } enum Type { diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_agg b/pkg/sql/logictest/testdata/logic_test/distsql_agg index df7b5606ca1b..fd77754c2371 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_agg +++ b/pkg/sql/logictest/testdata/logic_test/distsql_agg @@ -622,11 +622,21 @@ SELECT covar_pop(y, x)::decimal FROM statistics_agg_test ---- 3.75 +query FFF +SELECT regr_intercept(y, x), regr_r2(y, x), regr_slope(y, x) FROM statistics_agg_test +---- +48.4545454545455 0.00204565911136568 0.454545454545455 + query FFF SELECT regr_sxx(y, x), regr_sxy(y, x), regr_syy(y, x) FROM statistics_agg_test ---- 825 375 83325 +query FF +SELECT regr_avgx(y, x), regr_avgy(y, x) FROM statistics_agg_test +---- +4.5 50.5 + # Regression test for #37211 (incorrect ordering between aggregator stages). statement ok CREATE TABLE uv (u INT PRIMARY KEY, v INT); diff --git a/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg b/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg index daafb6f630f0..726c87cf59a0 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg +++ b/pkg/sql/opt/exec/execbuilder/testdata/distsql_agg @@ -54,7 +54,12 @@ EXPLAIN (DISTSQL) SELECT covar_pop(a, b), regr_sxx(a, b), regr_sxy(a, b), - regr_syy(a, b) + regr_syy(a, b), + regr_avgx(a, b), + regr_avgy(a, b), + regr_intercept(a, b), + regr_r2(a, b), + regr_slope(a, b) FROM data GROUP BY b ---- distribution: full @@ -68,7 +73,7 @@ vectorized: true table: data@data_pkey spans: FULL SCAN · -Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzsmNFvujoUx9_vX9H0id3UaAHd5hP8NmdIHOwHbHfLzUKqNI5EqRdw2bLsf78pIoLbr9XhIw8b55R-T4_nWz8PfsD0vwUcwtHj3cS0bKBcW57v_Z6cAW80GV35YIoAeZ0r5AyBKWOLgMShEvBou8CSMp-xdZzlW5fkbfOM4vyZZmFIXythkJLlqpqvWJGul9tnEBXVXkkSkXhGt8lOzJOt8o0lAZnPlWD6ntG0bOdv3miUFY1HWZHmbefZjJVVEJieIZDQeRKkb297-Xs9fy9ycOM6tyAkGQFj17m_A7-ewBQiGLOQ2mRJUzj8F2KIoAoR1CCCOkSwD58RXCVsRtOUJXzLRy6wwjc47CEYxat1xpefEZyxhMLhB8yibEHhEPpkuqAuJSFNuj2IYEgzEi3yY3gXxiqJliR5hwhescV6GadDwBtFILeJP_h8-DPKIILeivAtnS4GJA4BBix7oQl8_kSQrbNdG2lG5hQO8Sc6vFVzPk_onGQs6fbrnRp8Gqb9FNiOH9j3k4liqGe8m_tbxcA8unLubb-IfznOJDDta8XQytRxiyzfGLjOP57C01vzsVDdWnYReb_da-vmZpvd3wZWWfvRcQNzPFYMPS9t-ZuD-tuMn5MnvmvanuVbjh24o7E78jwemuOxOxqb_kgxMOKfoT643Sym7-CFpC97Y8Dw-XM3XPWPw93VWccsCWlCw1qlvIpg_Lj35eD9-eNy_mptSlrdAb3mQL-2dVAacF4acMGjG8s2J4HnX1-PHhTjEhkq2tStvgjunLvay51Rvd3eB9O1TPtq9E2ZB9P9UqO0F-Oav1itGowrVa6csg7Wd8vc88B7fPx29em71ady1aVxSJMhMDAChto1NAQMHQGjj4AxQMA4R8C4QMC4LP5wj-_jARdgtfzHhVgvVJjLcR7xAphXwFyu9v749dVOe8Ns1mGrLu7v7fz-bL12Nj6ccvhklOviTlf9AeckzVa-aIOWc8dyDp-Sc7jlXMu5fc6ph7NGPR1r1E5X-wFrJM1WLvt5y5pjWaOekjVqy5qWNfus0Q5njXY61midrv4D1kiarVz2i5Y1x7JGOyVrtJY1LWv2WaMfzhr9dKzRO93-D1gjabZy2S9b1hzLGv2UrNFb1rSsEf1W9M39cWm6YnFKD_olqMdvIA3ndHNdU7ZOZvQuYbP8mE3q5Lp8IaRptnmLN4kVb17xBqtivC_GVbFaE-PjxIMm4ssmYtyob9wXq1XhvDWxWBObNRC7pQvVfbG438RqsVhitVgssVosllktUUusHjSx-lwovhCbddHELLFYYpZYLDFLLJaZJVFLzLpsYhaWUFSG0WYcbQbSZiRtiNJmLMWNYIolNNUlpn3B6VGmidUy08RqmWlitdQ0iVxm2heoCk17_vzr_wAAAP__GJu0AA== +Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzsmUFvqkoUx_fvU0xmZV_G6AxgW1dwLTUkFrxA-9q8NARl4iVRxgfY2Nz0u78MKoL1MlpcdMGiZc5h_mdOz7_zW-hvmPw3h32oP49HmmGC1p3huM7P0RVw9JE-cMEEAf9t1vKvEJgwNvf8KGh5fLVLsDiPp2wVpdnWhb_ePMMoeyZpENC3wtJL_MWyGC_ZNlwtdk8v3FZ78-PQj6Z0F-zFPNgp1yz2_Nms5U3eU5rk7fzNGw3TbeNhug2ztrNoyvIqCEyuEIjpLPaS9fogfi_H7-XYf5utDxPlHWGU0nhKl2kpG5Ny2Tlb0k0G3NvWAwj81AdD23ocgx8vYAIRjFhATX9BE9j_F2KIIIEIShBBGSKowFcElzGb0iRhMd_yOxMYwRr2uwiG0XKV8vQrglMWU9j_DdMwnVPYh64_mVOb-gGNO12IYEBTP5xnx_Au1GUcLvz4HSI4YPPVIkr6gDeKQOY_f_DB82eYQgSdpc-3tDsY-FEAMGDpLxrD1w8E2Srdt5Gk_ozCPv5Ap7eqzWYxnfkpiztKuVOVT0MzXzzTcj3zcTRqqeSKd_P40FIxXw2sR9Pdrn9Y1sjTzLuWKuWhZW-jbKNnW_84LR4-aM9b1YNhblfOT_vOuL_fRY8PnpHXfrZsTxsOW6qclTbczUHKLuLnZIFra6ZjuIZlerY-tHXH4UttOLT1oebqLRUj_jeUB7efxeQd_PKTXwdjwPD1Yz9c8sfh7uusIhYHNKZBqVJWpWL8uPvp4MP543z-pDQlqeyAXHJAKW3t5QZc5wbc8NW9YWojz3Hv7vSnlnqLVII2dYsvvLE1Lr3cG9Xd733SbEMzB_qRMk-a_alGbi_GJX8xKRqMC1UGVl4Hy_s099xznp-PZl-OZV-OZbWn4bES2tPw2G7DdHV7oI_dI-9scuzQkTXWd3mbRgGN-0DFCKiko0oIqDICqoKA2kNAvUZAvUFAvd3-4C7fxxdcgEn-iwuxvFVhLsfZihfAvALmcpLJs7O4inAV4Sqi_BEn0mX_403WZssOVg52Hj9bLp2NT6cuvhh1O7jdIV_grqDZwsXvNdw9l7v4ktzFDXcb7n537pLT2Ucuxz7S7khfYJ-g2cLlu27Ydy77yCXZRxr2Nez77uyTTmefdDn2Se2O_AX2CZotXL6bhn3nsk-6JPukhn0N-747--TT2Sdfjn1yu6N8gX2CZguX77Zh37nsky_JPrlhX8O-784-wVcXNk2WLEroSZ8kdvmNoMGMbq5PwlbxlI5jNs2O2YRWpssSAU3SzVu8CYxo84o3WBTjQzEuiklJjM8T9-qIb-uIca2-sVKtJpXzlqrFUrVZvWq35Eq1Ui1W6lhdLRZYXS0WWF0tFlktUAus7tWx-rpSfFNt1k0ds6rFArOqxQKzqsUiswRqgVm3dczCAoqKMFqPo_VAWo-kNVFaj6W4FkyxgKaywLRPOD3LtGq1yLRqtci0arXQNIFcZNonqFaa9vrx1_8BAAD__yCBgnk= statement ok @@ -266,7 +271,7 @@ Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJy0lNGK4jAUhu_3K # Regression aggregate functions have two local, and one final stage aggregations. # Calculation and rendering are happening at the end. query T -EXPLAIN (DISTSQL) SELECT covar_pop(a, c), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c) FROM data +EXPLAIN (DISTSQL) SELECT covar_pop(a, c), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c), regr_avgx(a, c), regr_avgy(a, c), regr_intercept(a, c), regr_r2(a, c), regr_slope(a, c) FROM data ---- distribution: full vectorized: true @@ -278,12 +283,13 @@ vectorized: true table: data@data_pkey spans: FULL SCAN · -Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJy8lNGrokAUxt_3rxjOU8GEjVq361PSekNoq1VZuiwhszq4QTnujC1F9L8v5oVbUaMY7VvnTJ_fN78znAPIP2uwwFnMJ7Y7Ra2vrh_43ydt5DsTZxSgiP-lIsx41qIYRW2MBEtEKHe7q3p_We8_avTmzb6hmOYUMKQ8ZlO6YRKsn0AAgw4YDMBgAoYeLDFkgkdMSi6KvxxOAjfegdXFsEqzbV60lxgiLhhYB8hX-ZqBBQH9tWYeozETWhcwxCynq_XJprAeZmK1oWIPGEZ8vd2k0kJFOsDgZ7SoOhpBNI0RQTz_zQQsjxj4Nv90lDlNGFjkiOunspNEsITmXGi9y1CBZ099N3Bn09Bzxp7j-8VPezz2nLEdOK0hwUO9fTeFfjfFp_k25SJmgsUXzsujOie5ovfmTu1JOJr9sL1wPpu3hqQN-KNbJA_9xeJW8_1G871s3ruUcXEpUn_gpMnANdLR9AYjr8h1hrL_xJHr9enojejoHc1oQKci1xmdlyfSMerTMRrRMTqa2YBORa4zOoMn0jHr0zEb0TE7Wq8BnYpcZ3Re_9MyvZHCYzLjqWRXS_X2l7vFsmVxwsrNLPlWRGwueHSyKcvZSXdqxEzm5SkpCzctj4qA52KiFOsXYnIt1tXOFdaGUm2qxeYjuXtKcV_t3H_E-UUpHqidB484v6pn1a14JupHdu29PH75FwAA__9Lzzph +Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJy8lV9ro0AUxd_3U8h9SmCCGTX941Mka4OQTbIqpWUJMqsXN2Add5yUlJLvvqgLja7VYOm-5R7neA4_L5NXyH8nYIL9sF1ZzloZfXU83_u-GiuevbIXvhLyZyaCjGcjRpRwTBSBsQjy47Exv9Tnl_rMnuNjU6if2KcSRYiZrKlCq7824RlWinLnbr4pEZMMCKQ8wjV7whzMH0CBgAYEdCBgAIEZ7AhkgoeY51wUR15LgxMdwZwS2KfZQRbyjkDIBYL5CnIvEwQTfPYzQRdZhEKdAoEIJdsnZUwRPc_E_omJFyCw4MnhKc1NpWgHBLyMFdNEpQpLI4UqXP5CAbsTAX6Qb4m5ZDGCSU_k8lZWHAuMmeRCndVL-a619hzf2awD1166tucVP63l0rWXlm-P5pTMtfG7LbR3W7yFH1IuIhQY1ZJ3p-6etEHvzllbq2CxubfcYLvZjuZ0DOSvWjQPvIeHNvGxRXxsEa37ZYvful-2nHXWvu0u7K3_7yNXa8lbbbZ2Kb-HUa9hpJevGB2yYiqdqNqAJevpdfbxrj5xybTL6WiD6GgTVR9Ap6fXGZ3rT6SjX05HH0RHn6jGADo9vc7o3HwiHeNyOsYgOsZEnQ2g09PrjM7tf7q-W1q4mGc8zbFxjbe_eVpc7xjFWP0X5PwgQtwKHpYx1bgpfaUQYS6rp7QanLR6VBQ8N9NOs1Yz06ZZ607uidY73Ua32fhI71mn-ao7-eojyded5pvu5JuPJN92f6tpz5p0L1kze3f68icAAP__g2h04w== + # Test various combinations of aggregation functions and verify that the # aggregation processors are set up correctly. query T -EXPLAIN (DISTSQL) SELECT sum(a), avg(b), sum(c), avg(d), stddev(a), stddev_samp(a), stddev_pop(a), variance(b), var_samp(b), var_pop(b), sum(a+b+c::INT+d), covar_pop(a, c), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c) FROM data +EXPLAIN (DISTSQL) SELECT sum(a), avg(b), sum(c), avg(d), stddev(a), stddev_samp(a), stddev_pop(a), variance(b), var_samp(b), var_pop(b), sum(a+b+c::INT+d), covar_pop(a, c), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c), regr_avgx(a, c), regr_avgy(a, c), regr_intercept(a, c), regr_r2(a, c), regr_slope(a, c) FROM data ---- distribution: full vectorized: true @@ -297,11 +303,11 @@ vectorized: true table: data@data_pkey spans: FULL SCAN · -Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzcVl-LskoYvz-fYniujCZs1NrWq5HWXYRe61Xfl305REw6dILSzmixy9J3P4xaWbQWJ-iiC-X595vnz-8ZmC9I_12ACfb7aGA5LlJeHD_wfw4ayLcHdj9A6XqpsAZGbDNTpg2c62GpR1LPoohv8pBCnKRsuarqq6RQN0zMWRzy_JgNE0XgTpFRu_MVhaEmmjZQE4Wm6bhBT4oyW5jsYhlGsgzBZ2KSfnyc6J_H-mepo1dv-ANFLGOAIU4i7rIlT8H8Gwhg0ACDDhgMwNCBMYaVSEKepomQIV85wIk-wGxjmMerdSbNYwxhIjiYX5DNswUHEwI2XXCPs4gLtQ0YIp6x-SJPI1PTlZgvmfgEDP1ksV7GqYkYRlOMQowiwOCvmLS1VIJYHCGCkuwfLgCDx-OICxNRo6kolDSp1mhSvZwQRpRgRDWMqI4RNWC8xZCss0OdacZmHEyyxdf3Ys1mgs9Ylgi1c9yK_-uHQrWGrFdKupT6w19uUMq51dhLnYo_l_2f3ovz-lqeUXq0I8_hHCKlwLNc3wmcoTvx7DfP9n0pWm9vnv1mBbZCNSwTfte39m3fh3bXcSIiLnh01Ot4Wz8Z0j4zGrIvfjekiVM7nH1EV2qvjmsNJn7w8mL_VugTpgTT3qljMhqOzjh_W55juX1boc9YDkU_chWgiqcot30I6g_3YYQczHLoE__9_az1zznrn711v7r5jqrFimJEOyrtYkSfyq-HEX0uP9KWPxlP5FITCSESQzrfcqwfcUyuv6fk_99TlbRU7Q439UI3lX3sPtRN1a5nUbuBRa2l6ndg8UI3FRafHopF_XoW9RtY1FuqcQcWL3RTYbH3UCwa17No3MCi0VI7d2DxQjcVFp8fisULbz6Pp6skTvnJG-j8yW35NuLRjBcPqTRZi5CPRBLmaQp1mONyQ8TTrPCSQnHiwiULrIJJLVg7ApNTsFaf-UJqvRZt1IONW-ru1IK79Zm7t2R-qgX36jP3bsn8XM9V-8Ka1C_Zae7x9q__AgAA___FoV71 +Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzcVt1vsjoYvz9_RfNcYazBAjrHVYljhsQXfYF32XJiTAeNx0SBU9BsWfa_nxT8wjA0x2QXu4A8X78-H7-26Qdk_67ABPt5OrYcFykPjh_4v8ct5NtjexigbLNWWAsjtl0ory1c6OFOj6SeRxHfFiGlOM_YOj3V06RUt0wsWRzyYpktE2XgXpFR-_UVhaE2em2hNgpN03GDgRRltjDZxzKMZBmCL8Q8e3s709-r-ntVZ9vF27mhGrGMcy5CnuYVq9Cqy66SlJcW9OhNfqGI5QwwxEnEXbbmGZh_AwEMGmDQAYMBGHoww5CKJORZlggZ8lEAnOgNzC6GZZxucmmeYQgTwcH8gHyZrziYELDXFfc4i7hQu4Ah4jlbroo0MjVNxXLNxDtgGCarzTrOTMQwesUoxCgCDH7KpK2jEsTiCBGU5P9wARg8HkdcmIgabUWhpE21Vpvqu9FjRAlGVMOI6hhRA2afGJJNfqwzy9mCg0k-8fW9WIuF4AuWJ0LtVVvx__xSqNaS9UpJl9Jw8scNdnJhNQ5S78RfyP5v78F5fNytsfNoFc9xHSKlwLNc3wmciTv37JFn-74UrdHIs0dWYCtUwzLhV31rX_Z9bHcTJyLigkeVXmefzZMh3ZrRkEPx-yHNncbhHCL6Unt0XGs894OHB_tJoXeYEkwH5475dDKtcT5ZnmO5Q1uh91gORa-4StCJpyy3ewwaTg5hhBzNcuhz__m51vpSZ32ps1pPo7olrKdRXbTjBrY3tKdBjc_T6pKOJ1N7bz-cmOJoqOXJwIj2VNrHiN7tvgFG9H73ka78yXgizxKRECIxpCd_EkYkhkgQkQit--We0yt7jlx_b5D_f2-opKNq33BzXOjm5Hz0f9TNoV3PonYDi1pH1b-BxQvdnLB496NY1K9nUb-BRb2jGt_A4oVuTlgc_CgWjetZNG5g0eiovW9g8UI3Jyze_ygWL7xBPZ6lSZzxszdZ_cpd-Vbj0YKXD7ss2YiQT0USFmlKdVLgCkPEs7z0klJx4tIlCzwFk0awVgGTc7DWnPlCar0RbTSDjVvq7jWC-82Z-7dkvmsED5ozD27JfN_MVffCNmneZOe5Z59__RcAAP__8oSfLA== query T -EXPLAIN (DISTSQL) SELECT sum(a), min(b), max(c), count(d), avg(a+b+c::INT+d), stddev(a+b), variance(c::INT+d), covar_pop(b, d), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c) FROM data +EXPLAIN (DISTSQL) SELECT sum(a), min(b), max(c), count(d), avg(a+b+c::INT+d), stddev(a+b), variance(c::INT+d), covar_pop(b, d), regr_sxx(a, c), regr_sxy(a, c), regr_syy(a, c), regr_avgx(a, c), regr_avgy(b, c), regr_intercept(a, b), regr_r2(b, c), regr_slope(a, c) FROM data ---- distribution: full vectorized: true @@ -315,7 +321,8 @@ vectorized: true table: data@data_pkey spans: FULL SCAN · -Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzsVl1v6jgQfd9fYc2TEYPASfhonhzRtIrUht4kt9urFUImsVgkSFgnVK2q_veVAwXSbQN7eehLX8Bz5kzGc3ws-QXyfxZgg_twd-N4PqGXXhiFP24aJHRv3GFE8vWSigaS5TylU_0vnmjcQBJn67SgSQOJeJxRSgVpkmmDNEls254fDfRSZ_MiSeTjNo3kUai5SGNJt7QtK84ehZqsshWdYgkoOVOT_OmJCiTxPn6uxs_bmFwFo1uSiEIAQpol0hdLmYP9FzBAMADBBAQLELowRlipLJZ5nilNeSkLvOQJ7A7CPF2tCw2PEeJMSbBfoJgXCwk2RGK6kIEUiVTtDiAkshDzRdlGt-YrNV8K9QwIw2yxXqa5TQSSKZIYSQII4UporNVmRKQJYSQr_pYKEAKZJlLZhFtNSjlrcqPR5OZWRyQlgjr7BmoMSQmaOgHjV4RsXez3nhdiJsFmr3j6fM5spuRMFJlqd6vjhT9vKbcagHDr-ZR3y5XzQHlPr4ajn340CUZ_hlSHJZntEtt1-CO49K6uKDd2HOOAY1Q45o5jHnDKdRQ4fuhF3sifBO514IahXjrX14F77UQu5V3k_ZOIFurtf6ac8alye8HWaaYSqWRSUWv8Wq8t63wgLtuJa-zEfVNh4unprZ0m3QpeHsGV5zs3kzC6vHTvKe8jHyC_2CfuncBz_KFLOesgZww5M_bZ4ejeCSZ3ozvKmbmHtWqT8OGBcmb9B_31Efprh-4cXbEpEt5t8x4S3kfCtYsvtJM7-kcTmfHpcZiV42CnX1T2-xe1zVpt44uu6pEJD-zU-76qFeWM071hnOENo9U2v8gbRyY88Eb_2xsV5czTvWGe4Q2z1ba-yBtHJjzwxuDbGxXlrNO9YZ3hDavV7n6RN45MeOCNi29v_J-HcyDzVZbm8t0z8OMvd_TzUCYzuXlL5tlaxfJOZXHZZhOOyroSSGRebLJsE3jpJqU3eFjMaouNSjF7X2zUdz7S2qyttuqLrXP23a0t7tV37p3TuV9bPKjvPDin80X9WXWO2KTeZO97j1__-DcAAP__rUWnTA== +Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzsV11v6jgQfd9fYc2TUV2B8wFtnhzRFEWigZvksr1aIWQSi0WChHUCoqr631cOFAhLA1rU25e-BPvMGY9n5tgyr5D9MwMLnOd-13Y9hB_cIAx-dGsocLpOO0TZco55jaD5NMFj9cvXOKoRFKXLJMdxjSC-mmCMObpB4xq6QZFluV54p4bKmuVxLFZbM0ErLqc8iQTe0rasKF1xOVqkCzwmBSDFRI6y9RpzgqL9_KU8fynP-WqyPgZe1Io7YJrkQkZikSva-B2VWomUzdKF2KyDHv3eE4p5zoFAksbC43ORgfUXUCCgAQEdCBhAwIQhgYVMI5FlqVSU18LBjddgNQhMk8UyV_CQQJRKAdYr5NN8JsCCkI9nwhc8FrLeAAKxyPl0VoRRodlCTudcvgCBdjpbzpPMQmr7BEUExUAgWHCF3dYp4kmMKErzv4UEAr5IYiEtxIwbjBm9YVrthunbBhFUIERZ30GFEVSAujLA8I1Ausz3e89yPhFg0TdyeX72ZCLFhOeprJvl9IKfT5gZNSDw5HqYmcXIfsasqUbt3k8vHPm9PwOspgWZ7gzbcfDDf3AfHzHTdhztgKOVOPqOox9winHo217ghm7PG_lOx3eCQA3tTsd3OnboYGYS1rqIaJDN9i9Z8TKiQVRpPuqF9mEv9i1YJqmMhRRxqf7Dt-pu0caJdtFdu7Rdu97rOnJVPY1dlc0SXiT76Hp2dxSEDw_OALMWYXeE3e8NA9t3ba_tYEYbhFFKGNX21nZvYPujfq-PGdX3sKraKHh-xowa_0F_nUJ_nULtQefUEvago9jmEex6oeO3nb4SYvPI5msnHIJur--8r787mqXzRhAz66xJEGsRxNRxvFdHsqE-ikgVkyoqVVxqqo-i09aH8tBL8qCXX0X0_19FdXpb177oMjqT4YG8m9-X0SdfRtrlatOuUJt2W9e_SG1nMjxQW-tbbZ-sNv1ytelXqE2_rRtfpLYzGR6o7e5bbZ-sNuNytRlXqM24rZtfpLYzGR6o7f5bbb_xWX-iF77IFmmSiaPn_emVG-rZL-KJ2PxHyNKljERfplERZjPtFX4FEIss31jpZuImG5Pa4KEzrXTWSs702FmrjnwmtF7pbVQ7G9fs26x0blZHbl4TuVXpfFcd-e6ayPfVvWqckUm1yI5jD9_--DcAAP__ERJXMA== + # Verify that local and final aggregation is correctly shared and de-duplicated. query T diff --git a/pkg/sql/physicalplan/aggregator_funcs.go b/pkg/sql/physicalplan/aggregator_funcs.go index 8a720e3831f8..18a67917ded0 100644 --- a/pkg/sql/physicalplan/aggregator_funcs.go +++ b/pkg/sql/physicalplan/aggregator_funcs.go @@ -368,4 +368,54 @@ var DistAggregationTable = map[execinfrapb.AggregatorSpec_Func]DistAggregationIn }, }, }, + + execinfrapb.RegrAvgx: { + LocalStage: []execinfrapb.AggregatorSpec_Func{execinfrapb.TransitionRegrAggregate}, + FinalStage: []FinalStageInfo{ + { + Fn: execinfrapb.FinalRegrAvgx, + LocalIdxs: passThroughLocalIdxs, + }, + }, + }, + + execinfrapb.RegrAvgy: { + LocalStage: []execinfrapb.AggregatorSpec_Func{execinfrapb.TransitionRegrAggregate}, + FinalStage: []FinalStageInfo{ + { + Fn: execinfrapb.FinalRegrAvgy, + LocalIdxs: passThroughLocalIdxs, + }, + }, + }, + + execinfrapb.RegrIntercept: { + LocalStage: []execinfrapb.AggregatorSpec_Func{execinfrapb.TransitionRegrAggregate}, + FinalStage: []FinalStageInfo{ + { + Fn: execinfrapb.FinalRegrIntercept, + LocalIdxs: passThroughLocalIdxs, + }, + }, + }, + + execinfrapb.RegrR2: { + LocalStage: []execinfrapb.AggregatorSpec_Func{execinfrapb.TransitionRegrAggregate}, + FinalStage: []FinalStageInfo{ + { + Fn: execinfrapb.FinalRegrR2, + LocalIdxs: passThroughLocalIdxs, + }, + }, + }, + + execinfrapb.RegrSlope: { + LocalStage: []execinfrapb.AggregatorSpec_Func{execinfrapb.TransitionRegrAggregate}, + FinalStage: []FinalStageInfo{ + { + Fn: execinfrapb.FinalRegrSlope, + LocalIdxs: passThroughLocalIdxs, + }, + }, + }, } diff --git a/pkg/sql/sem/builtins/aggregate_builtins.go b/pkg/sql/sem/builtins/aggregate_builtins.go index 0ed982f71745..8a10b951165f 100644 --- a/pkg/sql/sem/builtins/aggregate_builtins.go +++ b/pkg/sql/sem/builtins/aggregate_builtins.go @@ -204,6 +204,41 @@ var aggregates = map[string]builtinDefinition{ ), )), + "final_regr_avgx": makePrivate(makeBuiltin(aggProps(), + makeAggOverload([]*types.T{types.DecimalArray}, types.Float, newFinalRegressionAvgXAggregate, + "Calculates the average of the independent variable (sum(X)/N) in final stage.", + tree.VolatilityImmutable, + ), + )), + + "final_regr_avgy": makePrivate(makeBuiltin(aggProps(), + makeAggOverload([]*types.T{types.DecimalArray}, types.Float, newFinalRegressionAvgYAggregate, + "Calculates the average of the dependent variable (sum(Y)/N) in final stage.", + tree.VolatilityImmutable, + ), + )), + + "final_regr_intercept": makePrivate(makeBuiltin(aggProps(), + makeAggOverload([]*types.T{types.DecimalArray}, types.Float, newFinalRegressionInterceptAggregate, + "Calculates y-intercept of the least-squares-fit linear equation determined by the (X, Y) pairs in final stage.", + tree.VolatilityImmutable, + )), + ), + + "final_regr_r2": makePrivate(makeBuiltin(aggProps(), + makeAggOverload([]*types.T{types.DecimalArray}, types.Float, newFinalRegressionR2Aggregate, + "Calculates square of the correlation coefficient in final stage.", + tree.VolatilityImmutable, + )), + ), + + "final_regr_slope": makePrivate(makeBuiltin(aggProps(), + makeAggOverload([]*types.T{types.DecimalArray}, types.Float, newFinalRegressionSlopeAggregate, + "Calculates slope of the least-squares-fit linear equation determined by the (X, Y) pairs in final stage.", + tree.VolatilityImmutable, + )), + ), + "transition_regression_aggregate": makePrivate(makeTransitionRegressionAggregateBuiltin()), "covar_samp": makeRegressionAggregateBuiltin( @@ -1171,6 +1206,11 @@ var _ tree.AggregateFunc = &finalCovarPopAggregate{} var _ tree.AggregateFunc = &finalRegrSXXAggregate{} var _ tree.AggregateFunc = &finalRegrSXYAggregate{} var _ tree.AggregateFunc = &finalRegrSYYAggregate{} +var _ tree.AggregateFunc = &finalRegressionAvgXAggregate{} +var _ tree.AggregateFunc = &finalRegressionAvgYAggregate{} +var _ tree.AggregateFunc = &finalRegressionInterceptAggregate{} +var _ tree.AggregateFunc = &finalRegressionR2Aggregate{} +var _ tree.AggregateFunc = &finalRegressionSlopeAggregate{} var _ tree.AggregateFunc = &covarSampAggregate{} var _ tree.AggregateFunc = ®ressionInterceptAggregate{} var _ tree.AggregateFunc = ®ressionR2Aggregate{} @@ -2153,6 +2193,87 @@ func (a *regressionAccumulatorDecimalBase) regrSYYLastStage() (tree.Datum, error return mapToDFloat(&a.syy, a.ed.Err()) } +// regressionAvgXLastStage computes SQL:2003 average of the independent variable +// (sum(X)/N) from the precalculated transition values. +func (a *regressionAccumulatorDecimalBase) regressionAvgXLastStage() (tree.Datum, error) { + if a.n.Cmp(decimalOne) < 0 { + return tree.DNull, nil + } + + // a.sx / a.n + a.ed.Quo(&a.tmp, &a.sx, &a.n) + return mapToDFloat(&a.tmp, a.ed.Err()) +} + +// regressionAvgYLastStage computes SQL:2003 average of the dependent variable +// (sum(Y)/N) from the precalculated transition values. +func (a *regressionAccumulatorDecimalBase) regressionAvgYLastStage() (tree.Datum, error) { + if a.n.Cmp(decimalOne) < 0 { + return tree.DNull, nil + } + + // a.sy / a.n + a.ed.Quo(&a.tmp, &a.sy, &a.n) + return mapToDFloat(&a.tmp, a.ed.Err()) +} + +// regressionInterceptLastStage computes y-intercept from the precalculated +// transition values. +func (a *regressionAccumulatorDecimalBase) regressionInterceptLastStage() (tree.Datum, error) { + if a.n.Cmp(decimalOne) < 0 { + return tree.DNull, nil + } + if a.sxx.Cmp(decimalZero) == 0 { + return tree.DNull, nil + } + + // (a.sy - a.sx*a.sxy/a.sxx) / a.n + a.ed.Quo( + &a.tmp, + a.ed.Sub(&a.tmp, &a.sy, a.ed.Mul(&a.tmp, &a.sx, a.ed.Quo(&a.tmp, &a.sxy, &a.sxx))), + &a.n, + ) + return mapToDFloat(&a.tmp, a.ed.Err()) +} + +// regressionR2LastStage computes square of the correlation coefficient from the +// precalculated transition values. +func (a *regressionAccumulatorDecimalBase) regressionR2LastStage() (tree.Datum, error) { + if a.n.Cmp(decimalOne) < 0 { + return tree.DNull, nil + } + if a.sxx.Cmp(decimalZero) == 0 { + return tree.DNull, nil + } + if a.syy.Cmp(decimalZero) == 0 { + return tree.NewDFloat(tree.DFloat(1.0)), nil + } + + // (a.sxy * a.sxy) / (a.sxx * a.syy) + a.ed.Quo( + &a.tmp, + a.ed.Mul(&a.tmp, &a.sxy, &a.sxy), + a.ed.Mul(&a.tmpN, &a.sxx, &a.syy), + ) + return mapToDFloat(&a.tmp, a.ed.Err()) +} + +// regressionSlopeLastStage computes slope of the least-squares-fit linear +// equation determined by the (X, Y) pairs from the precalculated transition +// values. +func (a *regressionAccumulatorDecimalBase) regressionSlopeLastStage() (tree.Datum, error) { + if a.n.Cmp(decimalOne) < 0 { + return tree.DNull, nil + } + if a.sxx.Cmp(decimalZero) == 0 { + return tree.DNull, nil + } + + // a.sxy / a.sxx + a.ed.Quo(&a.tmp, &a.sxy, &a.sxx) + return mapToDFloat(&a.tmp, a.ed.Err()) +} + type finalRegressionAccumulatorDecimalBase struct { regressionAccumulatorDecimalBase otherTransitionValues [regrFieldsTotal]*apd.Decimal @@ -2503,13 +2624,28 @@ func newRegressionAvgXAggregate( // Result implements tree.AggregateFunc interface. func (a *regressionAvgXAggregate) Result() (tree.Datum, error) { - if a.n.Cmp(decimalOne) < 0 { - return tree.DNull, nil + return a.regressionAvgXLastStage() +} + +// finalRegressionAvgXAggregate represents SQL:2003 average of the independent +// variable (sum(X)/N). +type finalRegressionAvgXAggregate struct { + finalRegressionAccumulatorDecimalBase +} + +func newFinalRegressionAvgXAggregate( + _ []*types.T, ctx *tree.EvalContext, _ tree.Datums, +) tree.AggregateFunc { + return &finalRegressionAvgXAggregate{ + finalRegressionAccumulatorDecimalBase{ + regressionAccumulatorDecimalBase: makeRegressionAccumulatorDecimalBase(ctx), + }, } +} - // a.sx / a.n - a.ed.Quo(&a.tmp, &a.sx, &a.n) - return mapToDFloat(&a.tmp, a.ed.Err()) +// Result implements tree.AggregateFunc interface. +func (a *finalRegressionAvgXAggregate) Result() (tree.Datum, error) { + return a.regressionAvgXLastStage() } // regressionAvgYAggregate represents SQL:2003 average of the dependent @@ -2528,13 +2664,28 @@ func newRegressionAvgYAggregate( // Result implements tree.AggregateFunc interface. func (a *regressionAvgYAggregate) Result() (tree.Datum, error) { - if a.n.Cmp(decimalOne) < 0 { - return tree.DNull, nil + return a.regressionAvgYLastStage() +} + +// finalRegressionAvgYAggregate represents SQL:2003 average of the independent +// variable (sum(Y)/N). +type finalRegressionAvgYAggregate struct { + finalRegressionAccumulatorDecimalBase +} + +func newFinalRegressionAvgYAggregate( + _ []*types.T, ctx *tree.EvalContext, _ tree.Datums, +) tree.AggregateFunc { + return &finalRegressionAvgYAggregate{ + finalRegressionAccumulatorDecimalBase{ + regressionAccumulatorDecimalBase: makeRegressionAccumulatorDecimalBase(ctx), + }, } +} - // a.sy / a.n - a.ed.Quo(&a.tmp, &a.sy, &a.n) - return mapToDFloat(&a.tmp, a.ed.Err()) +// Result implements tree.AggregateFunc interface. +func (a *finalRegressionAvgYAggregate) Result() (tree.Datum, error) { + return a.regressionAvgYLastStage() } // regressionInterceptAggregate represents y-intercept. @@ -2552,20 +2703,27 @@ func newRegressionInterceptAggregate( // Result implements tree.AggregateFunc interface. func (a *regressionInterceptAggregate) Result() (tree.Datum, error) { - if a.n.Cmp(decimalOne) < 0 { - return tree.DNull, nil - } - if a.sxx.Cmp(decimalZero) == 0 { - return tree.DNull, nil + return a.regressionInterceptLastStage() +} + +// finalRegressionInterceptAggregate represents y-intercept. +type finalRegressionInterceptAggregate struct { + finalRegressionAccumulatorDecimalBase +} + +func newFinalRegressionInterceptAggregate( + _ []*types.T, ctx *tree.EvalContext, _ tree.Datums, +) tree.AggregateFunc { + return &finalRegressionInterceptAggregate{ + finalRegressionAccumulatorDecimalBase{ + regressionAccumulatorDecimalBase: makeRegressionAccumulatorDecimalBase(ctx), + }, } +} - // (a.sy - a.sx*a.sxy/a.sxx) / a.n - a.ed.Quo( - &a.tmp, - a.ed.Sub(&a.tmp, &a.sy, a.ed.Mul(&a.tmp, &a.sx, a.ed.Quo(&a.tmp, &a.sxy, &a.sxx))), - &a.n, - ) - return mapToDFloat(&a.tmp, a.ed.Err()) +// Result implements tree.AggregateFunc interface. +func (a *finalRegressionInterceptAggregate) Result() (tree.Datum, error) { + return a.regressionInterceptLastStage() } // regressionR2Aggregate represents square of the correlation coefficient. @@ -2583,23 +2741,27 @@ func newRegressionR2Aggregate( // Result implements tree.AggregateFunc interface. func (a *regressionR2Aggregate) Result() (tree.Datum, error) { - if a.n.Cmp(decimalOne) < 0 { - return tree.DNull, nil - } - if a.sxx.Cmp(decimalZero) == 0 { - return tree.DNull, nil - } - if a.syy.Cmp(decimalZero) == 0 { - return tree.NewDFloat(tree.DFloat(1.0)), nil + return a.regressionR2LastStage() +} + +// finalRegressionR2Aggregate represents square of the correlation coefficient. +type finalRegressionR2Aggregate struct { + finalRegressionAccumulatorDecimalBase +} + +func newFinalRegressionR2Aggregate( + _ []*types.T, ctx *tree.EvalContext, _ tree.Datums, +) tree.AggregateFunc { + return &finalRegressionR2Aggregate{ + finalRegressionAccumulatorDecimalBase{ + regressionAccumulatorDecimalBase: makeRegressionAccumulatorDecimalBase(ctx), + }, } +} - // (a.sxy * a.sxy) / (a.sxx * a.syy) - a.ed.Quo( - &a.tmp, - a.ed.Mul(&a.tmp, &a.sxy, &a.sxy), - a.ed.Mul(&a.tmpN, &a.sxx, &a.syy), - ) - return mapToDFloat(&a.tmp, a.ed.Err()) +// Result implements tree.AggregateFunc interface. +func (a *finalRegressionR2Aggregate) Result() (tree.Datum, error) { + return a.regressionR2LastStage() } // regressionSlopeAggregate represents slope of the least-squares-fit linear @@ -2618,16 +2780,28 @@ func newRegressionSlopeAggregate( // Result implements tree.AggregateFunc interface. func (a *regressionSlopeAggregate) Result() (tree.Datum, error) { - if a.n.Cmp(decimalOne) < 0 { - return tree.DNull, nil - } - if a.sxx.Cmp(decimalZero) == 0 { - return tree.DNull, nil + return a.regressionSlopeLastStage() +} + +// finalRegressionSlopeAggregate represents slope of the least-squares-fit +// linear equation determined by the (X, Y) pairs. +type finalRegressionSlopeAggregate struct { + finalRegressionAccumulatorDecimalBase +} + +func newFinalRegressionSlopeAggregate( + _ []*types.T, ctx *tree.EvalContext, _ tree.Datums, +) tree.AggregateFunc { + return &finalRegressionSlopeAggregate{ + finalRegressionAccumulatorDecimalBase{ + regressionAccumulatorDecimalBase: makeRegressionAccumulatorDecimalBase(ctx), + }, } +} - // a.sxy / a.sxx - a.ed.Quo(&a.tmp, &a.sxy, &a.sxx) - return mapToDFloat(&a.tmp, a.ed.Err()) +// Result implements tree.AggregateFunc interface. +func (a *finalRegressionSlopeAggregate) Result() (tree.Datum, error) { + return a.regressionSlopeLastStage() } // regressionSXXAggregate represents sum of squares of the independent variable. From bfb9b8b40ac533ea807a6159a2a640a00f34ba03 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 2 Feb 2022 11:57:41 -0500 Subject: [PATCH 4/4] rfc: pgwire-compatible query cancellation Release note: None --- .../RFCS/20220202_pgwire_compatible_cancel.md | 316 ++++++++++++++++++ 1 file changed, 316 insertions(+) create mode 100644 docs/RFCS/20220202_pgwire_compatible_cancel.md diff --git a/docs/RFCS/20220202_pgwire_compatible_cancel.md b/docs/RFCS/20220202_pgwire_compatible_cancel.md new file mode 100644 index 000000000000..c0a16750c079 --- /dev/null +++ b/docs/RFCS/20220202_pgwire_compatible_cancel.md @@ -0,0 +1,316 @@ +- Feature Name: Postgres-Compatible Cancel Protocol +- Status: draft +- Start Date: 2022-02-02 +- Authors: Rafi Shamim +- RFC PR: https://github.com/cockroachdb/cockroach/pull/75870 +- Cockroach Issue: https://github.com/cockroachdb/cockroach/issues/41335 + +## Dedication + +The proposal here is entirely dependent on many thoughtful discussions and prior +work with Jordan Lewis, knz, Andrew Werner, Andy Kimball, Peter Mattis, Ben +Darnell, and several others going back to 2019 all the way until now. + +## Summary + +The Postgres (pgwire) query cancel protocol provides a way to cancel a query +running in a SQL session. Many database drivers use this protocol, but +currently, CockroachDB just ignores any pgwire cancel request. The protocol is +hard to implement since it only uses 64 bits of data as an identifier, and is +sent over a separate (unauthenticated) connection, different from the SQL +connection. For dedicated clusters, these 64 bits of data need to identify a +node and session to cancel. We need at most 32 bits to identify which node is +running the query, so that when any node receives a cancellation request, it can +forward the request to the correct node in a cluster. We use the other bits to +identify a session running on that node. Finally, we add a semaphore to guard +the cancel logic so that an attacker cannot spam guesses for session IDs. + +In CockroachDB Serverless, a SQL proxy instance also needs to identify which +tenant to send the cancel request to. To solve this, each SQL proxy will save +the 64-bit cancel keys returned by the SQL node, then send a different 64-bit +key back to the client. This proxy-client key will contain the IP address of the +proxy that is able to handle this key. (Or, if the proxies are all on the same +subnet, fewer bits can be used to identify the proxy.) The remaining bits are +random. When any proxy receives a cancel request, it can look at the key to +figure out which other proxy to forward the request to. Then when the correct +proxy receives the request, it checks that the random bits of the key are in its +in-memory map of cancel keys, and forwards the original cancel key to the tenant +where it came from. + +Serverless support can be implemented entirely separately from +dedicated/self-hosted support. + + +## Motivation + +Nearly all Postgres drivers support the Postgres query cancellation protocol. +For example, the PGJDBC +[setQueryTimeout](https://jdbc.postgresql.org/documentation/publicapi/org/postgresql/jdbc/PgStatement.html#setQueryTimeout-int-) +setting +[uses](https://github.com/pgjdbc/pgjdbc/blob/3a54d28e0b416a84353d85e73a23180a6719435e/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java#L171) +it. Currently, when a client sends a cancellation request using this protocol, +CockroachDB simply ignores it. Implementing it would mean that applications +using drivers like this would immediately benefit. Specifically, it would allow +CockroachDB to stop executing queries that the client is no longer waiting for, +thereby reducing load on the cluster. + +This protocol is the top unimplemented feature in our telemetry data. According +to our [Looker dashboard](https://cockroachlabs.looker.com/looks/47), 3,430 +long-running clusters have attempted to use it (compared to 456 clusters for the +next unimplemented feature). + + +## Background + +The [Postgres +documentation](https://www.postgresql.org/docs/14/protocol-flow.html#id-1.10.5.7.9) +describes how the protocol works. During connection startup, the server returns +a BackendKeyData message to the client. This is a 64-bit value; in Postgres 32 +bits are used for a process ID, and 32 bits are used for a random secret that is +generated when the connection starts. + +To issue a cancel request, the client opens a new unencrypted connection to the +server and sends a CancelRequest message. For security reasons, the server never +replies to this message. If the data in the request matches the BackendKeyData +that was generated earlier, then the query is cancelled. + +The Postgres documentation also mentions that this protocol is best-effort, and +specifically says, "Issuing a cancel simply improves the odds that the current +query will finish soon, and improves the odds that it will fail with an error +message instead of succeeding." + +There have been internal discussions about this in [April +2020](https://github.com/cockroachdb/cockroach/pull/34520#discussion_r407414290), +[July 2021](https://github.com/cockroachdb/cockroach/pull/67501), and [January +2022](https://cockroachlabs.slack.com/archives/CGA9F858R/p1643382222564939). + + +## Technical Design + + +#### SQL Node Changes + +The connExecutor will be updated to generate a random 64-bit integer +(BackendKeyData) when it is initialized, and register it with the server’s +[SessionRegistry](https://github.com/cockroachdb/cockroach/blob/a434c8418c36dbeb64e73588bcd4dd5b248c3238/pkg/sql/conn_executor.go#L1692). +If the SQL node's 32-bit SQLInstanceID fits in 11 bits, then the leading bit of +the BackendKeyData is set to 0, and the following 11 bits are set to the +SQLInstanceID. Otherwise, the leading bit is set to 1, and the next 31 bits are +set to the SQLInstanceID. The diagram below shows these two formats. Note that +SQLInstanceIDs are always positive, so it's safe to use the leading bit in this +way. This BackendKeyData is sent back to the client. + +Contents of BackendKeyData with a small SQLInstanceID: +``` + 0 1 2 ... 6 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 ... 3 ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +|0| SQLInstanceID | Random data | ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +``` +Contents of BackendKeyData with a large SQLInstanceID: +``` + 0 1 2 3 ... 6 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 ... 3 ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +|1| SQLInstanceID | Random data | ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +``` + +The status server will have a new endpoint named CancelQueryByKey, +analogous to the existing CancelQuery endpoint. The main difference is that it +has a 64-bit BackendKeyData in the request body. The endpoint will extract +the SQLInstanceID from the BackendKeyData and will forward the request to the +correct node. This endpoint will only be called by the pgwire/server code that +handles a CancelRequest – the endpoint is not meant to be called directly by a +client, so therefore it will not be exposed on HTTP. The endpoint will call the +SessionRegistry's cancellation function using the BackendKeyData. + +Since this endpoint is unauthenticated, before performing any business logic, it +will use a semaphore to guard the number of concurrent cancellation requests. If +a cancel request fails, which is likely to only happen if an attacker is +spamming requests, it will be penalized by holding onto the semaphore for an +extra second. This semaphore prevents an attacker from being able to cause +excess inter-node network traffic, and from being able to brute force a +successful cancel request. In the code, we will use `TryAcquire` on the +semaphore, which means that if we've already reached the max number of +concurrent requests, any excess requests will simply be dropped. This is allowed +because the protocol is defined to be best-effort. + +If we set the concurrency of the semaphore to 256 (2^8), then that means it +would take an attacker 2^24 seconds to guess all possible 32-bit secrets and be +guaranteed to cancel something. If we suppose there are 256 concurrent queries +running on the node, then on average it would take 2^16 seconds (18 hours) to +cancel any one of the queries. In the more common case, where we use 52-bits of +randomness in the BackendKeyData, the time-to-expected-cancel increases to 2^36 +seconds (2117 years). + +An attacker could still spam cancel requests and use up the entire quota of the +semaphore, and therefore starve legitimate cancel requests from being handled. +We consider this risk acceptable in the short-term, since the protocol is +best-effort, and this behavior is no worse than the status quo. + + +#### SQL Proxy Changes + +The proxy code will be updated to intercept BackendKeyData messages that are +sent to the client as well as CancelRequest messages that are sent by the client +to the server. + +When a proxy sees a BackendKeyData, it will generate a new random 32-bit secret +named proxySecretID. (NB: This proxySecretID could be larger depending on how +many bits are needed to identify a proxy instance.) The proxySecretID will be +used to key a map whose values are structs containing (1) the original +BackendKeyData, (2) the address of the SQL node, and (3) the remote client +address that initiated this connection. The proxy will then return a new +BackendKeyData consisting of 32 bits for the proxy’s IP address, and 32 bits for +the proxySecretID. If the proxies are all on a subnet, then fewer bits are +needed for the IP address. + +Contents of the proxy-client BackendKeyData: +``` + 0 1 2 3 ... 6 + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 ... 3 ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +| SQL Proxy IP | Random data | ++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+...+-+ +``` + +When a proxy sees a CancelRequest, it first extracts the IP address component of +the message. If needed, it will forward the request to the proxy with that +address using an RPC that is only for proxy-proxy communication. This RPC +request will include the remote address of the client that sent the +CancelRequest. If the proxy is the intended recipient, then it will extract the +proxySecretID and check if it exists in the BackendKeyData map. If so, it will +check that the remote client address in the map matches the address that sent +the CancelRequest. If that matches, then the proxy will send a CancelRequest +with the original BackendKeyData to the SQL node using the address that is +stored in the map. + +If the proxy migrates a session from one SQL node to another, that will make the +BackendKeyData it had previously saved obsolete. When the migration occurs, the +proxy will need to update the contents of its BackendKeyData map and replace the +old data with the new BackendKeyData provided by the session on the new SQL +node. + +If the proxy crashes unexpectedly, then all the BackendKeyData entries will be +lost. From the clients’ perspective, the connection will be broken when the +proxy crashes, and they will not be able to interact with any in-flight queries, +so it’s not a huge problem that the queries can no longer be cancelled. + +#### Proxy to Proxy communication + +The previous section mentioned that proxies will start making RPCs to other +proxies. Proxy to proxy communication does not currently exist anywhere else in +the architecture. When it's added, we need to ensure that the RPC is only +exposed to other proxy instances. Similar to SQL nodes, we will start deploying +TLS certs for proxy instances, and add a corresponding +`cockroach mt cert create-proxy` command. The proxy will need to load these +certs to create a TLSConfig to initialize a server and client. + +#### Example flow for handling a cancel request + +Suppose we are in the following scenario: +- "SQL Instance B" is running a query that needs to be cancelled. +- "SQL Instance A" and "SQL Instance B" are both part of the same tenant. +- The client initially connected to the tenant through "SQL Proxy B". +- The CancelRequest is routed by the cloud load balancer to "SQL Proxy A". + +``` + ┌──────┐ ┌───────────┐ ┌───────────┐ ┌──────────────┐ ┌──────────────┐ + │Client│ │SQL Proxy A│ │SQL Proxy B│ │SQL Instance A│ │SQL Instance B│ + └──┬───┘ └─────┬─────┘ └─────┬─────┘ └──────┬───────┘ └──────┬───────┘ + │ CancelRequest │ │ │ │ + │ with proxy-client │ │ │ │ + │ BackendKeyData │ │ │ │ + │ ──────────────────>│ │ │ │ + │ │ │ │ │ + │ │ ╔════════════════╗ │ │ │ + │ │ ║Extract proxy ░║ │ │ │ + │ │ ║IP address and ║ │ │ │ + │ │ ║ProxySecretID. ║ │ │ │ + │ │ ╚════════════════╝ │ │ │ + │ │RPC with remote client │ │ │ + │ │address and ProxySecretID │ │ │ + │ │─────────────────────────>│ │ │ + │ │ │ │ │ + │ │ │ ╔═════════════════════╧════════════════════╗ │ + │ │ │ ║Check ProxySecretID map. ░║ │ + │ │ │ ║Ensure remote client address matches. ║ │ + │ │ │ ║Get the tenant ID and DB BackendKeyData. ║ │ + │ │ │ ╚═════════════════════╤════════════════════╝ │ + │ │ │CancelRequest │ │ + │ │ │with DB BackendKeyData │ │ + │ │ │──────────────────────>│ │ + │ │ │ │ │ + │ │ │ │ ╔══════════════════════╧╗ + │ │ │ │ ║Extract SQLInstanceID ░║ + │ │ │ │ ║from BackendKeyData. ║ + │ │ │ │ ╚══════════════════════╤╝ + │ │ │ │ RPC with DB │ + │ │ │ │ BackendKeyData │ + │ │ │ │ ────────────────────────> + │ │ │ │ │ + │ │ │ │ │ ╔═══════════════════════╗ + │ │ │ │ │ ║Use BackendKeyData to ░║ + │ │ │ │ │ ║cancel a query. ║ + ┌──┴───┐ ┌─────┴─────┐ ┌─────┴─────┐ ┌──────┴───────┐ ┌──────┴──╚═══════════════════════╝ + │Client│ │SQL Proxy A│ │SQL Proxy B│ │SQL Instance A│ │SQL Instance B│ + └──────┘ └───────────┘ └───────────┘ └──────────────┘ └──────────────┘ +``` + +#### New observability metrics + +In addition to adding warning logs for failed cancellation attempts, we will add +the following metrics. + +- Total count of cancellation requests. +- Count of cancellation requests that were ignored due to rate limiting. +- Count of cancellation requests that were successful. + +### Alternatives + + +#### Make SQL nodes verify the remote address + +Instead of using a semaphore, the SQL nodes could also make sure the remote +client address that received the BackendKeyData matches the remote address that +sent the CancelRequest. This would work well in dedicated clusters. But in +serverless clusters, this would mean that the SQL proxy needs to propagate the +remote address of the client while sending the CancelRequest. For normal SQL +sessions, this is done using the **crdb:remote_addr** StartupMessage, but the +cancel protocol does not have a similar way of sending data like this. + + +#### Obfuscate the SQL proxy identifiers + +Using 32 bits of the proxy-client BackendKeyData for the proxy IP address means +that an attacker could more easily guess an address and spam it. Instead, we +could obfuscate the address by hashing it along with a salt that is shared by +all the proxy instances. This would require additional secret management at the +proxy layer. To avoid this complexity, the proposal instead is to validate the +address sending the CancelRequest. This still allows an attacker to cause +additional network traffic, but prevents them from doing any functional damage. + +### Possible Future Extensions + +#### Custom protocol for SQL node to SQL proxy communication + +The SQL node to SQL proxy part of the protocol described above could be changed +to be entirely custom. This would allow us to use more bits to identify the +session to cancel, and would eliminate the need for a semaphore. However, this +custom protocol could only be used in deployments where there is a SQL proxy in +front of the SQL nodes. + +#### IP-based rate limiting + +Another way of preventing an attacker from spamming cancel requests is to rate +limit these requests by IP. This also would eliminate the need for a semaphore, +and additionally, would prevent an attacker from causing extra network traffic +and starving legitimate cancel requests. However, we cannot add an IP-based rate +limiter at the SQL node layer, unless we also develop a custom protocol for +propagating the remote client address from the proxy to the SQL node. + +It is possible that in the future, _all_ clusters might live behind a SQL proxy +process (possibly an embedded process). If we wait until that is the case, then +we can add IP-based rate limiting at the proxy layer exclusively.