From 6e10beb74aae7dc9946896b1e9993803b5f897d5 Mon Sep 17 00:00:00 2001 From: Jerry Sievert Date: Thu, 24 Aug 2023 13:58:17 -0700 Subject: [PATCH] [columnar] fix create table as parallelism bug --- .../backend/columnar/columnar_customscan.c | 89 +++++++++++++++++++ .../expected/columnar_alter_set_type.out | 4 +- .../test/regress/expected/columnar_create.out | 6 ++ .../src/test/regress/sql/columnar_create.sql | 6 ++ 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/columnar/src/backend/columnar/columnar_customscan.c b/columnar/src/backend/columnar/columnar_customscan.c index d8d8ae42..9471913f 100644 --- a/columnar/src/backend/columnar/columnar_customscan.c +++ b/columnar/src/backend/columnar/columnar_customscan.c @@ -37,6 +37,7 @@ #include "optimizer/pathnode.h" #include "optimizer/paths.h" #include "optimizer/plancat.h" +#include "optimizer/planner.h" #include "optimizer/restrictinfo.h" #include "storage/smgr.h" #include "utils/builtins.h" @@ -126,6 +127,9 @@ static void ColumnarSetRelPathlistHook(PlannerInfo *root, RelOptInfo *rel, Index RangeTblEntry *rte); static void ColumnarGetRelationInfoHook(PlannerInfo *root, Oid relationObjectId, bool inhparent, RelOptInfo *rel); +static PlannedStmt *ColumnarPlannerHook(Query *parse, const char *query_string, + int cursorOptions, ParamListInfo boundParams); + static Plan * ColumnarScanPath_PlanCustomPath(PlannerInfo *root, RelOptInfo *rel, struct CustomPath *best_path, @@ -172,10 +176,12 @@ static List * set_deparse_context_planstate(List *dpcontext, Node *node, /* other helpers */ static List * ColumnarVarNeeded(ColumnarScanState *columnarScanState); static Bitmapset * ColumnarAttrNeeded(ScanState *ss, List *customList); +static bool IsCreateTableAs(const char *query); /* saved hook value in case of unload */ static set_rel_pathlist_hook_type PreviousSetRelPathlistHook = NULL; static get_relation_info_hook_type PreviousGetRelationInfoHook = NULL; +static planner_hook_type PreviousPlannerHook = NULL; static bool EnableColumnarCustomScan = true; static bool EnableColumnarQualPushdown = true; @@ -239,6 +245,9 @@ columnar_customscan_init() PreviousGetRelationInfoHook = get_relation_info_hook; get_relation_info_hook = ColumnarGetRelationInfoHook; + PreviousPlannerHook = planner_hook; + planner_hook = ColumnarPlannerHook; + /* register customscan specific GUC's */ DefineCustomBoolVariable( "columnar.enable_custom_scan", @@ -302,6 +311,86 @@ columnar_customscan_init() RegisterCustomScanMethods(&ColumnarScanScanMethods); } +/* + * IsCreateTableAs + * + * Searches a lower case copy of the query string using strstr to check + * for the keywords CREATE, TABLE, and AS, in that order. There can be + * false positives, but we try to minimize them. + */ +static +bool IsCreateTableAs(const char *query) +{ + char *c, *t, *a; + char *haystack = (char *) palloc(strlen(query) + 1); + int16 i; + + /* Create a lower case copy of the string. */ + for (i = 0; i < strlen(query); i++) + { + haystack[i] = tolower(query[i]); + } + + haystack[i] = '\0'; + + c = strstr(haystack, "create"); + if (c == NULL) + { + pfree(haystack); + return false; + } + + t = strstr(c + 6, "table"); + if (t == NULL) + { + pfree(haystack); + return false; + } + + a = strstr(t + 5, "as"); + if (a == NULL) + { + pfree(haystack); + return false; + } + + pfree(haystack); + + return true; +} + +static +PlannedStmt *ColumnarPlannerHook(Query *parse, const char *query_string, + int cursorOptions, ParamListInfo boundParams) +{ + PlannedStmt *stmt; + + if (PreviousPlannerHook) + { + stmt = (*PreviousPlannerHook)(parse, query_string, cursorOptions, boundParams); + } + else + { + stmt = standard_planner(parse, query_string, cursorOptions, boundParams); + } + + /* + * In the case of a CREATE TABLE AS query, we are not able to successfully + * drop out of a parallel insert situation. This checks for a CMD_SELECT + * and in that case examines the query string to see if it matches the + * pattern of a CREATE TABLE AS. If so, set the parallelism to 0 (off). + */ + if (parse->commandType == CMD_SELECT) + { + if (IsCreateTableAs(query_string)) + { + stmt->parallelModeNeeded = 0; + } + } + + return stmt; +} + static void ColumnarSetRelPathlistHook(PlannerInfo *root, RelOptInfo *rel, Index rti, diff --git a/columnar/src/test/regress/expected/columnar_alter_set_type.out b/columnar/src/test/regress/expected/columnar_alter_set_type.out index 9a4e6713..4039f0fb 100644 --- a/columnar/src/test/regress/expected/columnar_alter_set_type.out +++ b/columnar/src/test/regress/expected/columnar_alter_set_type.out @@ -63,7 +63,7 @@ SELECT columnar.alter_columnar_table_set('test', compression => 'lz4'); INSERT INTO test VALUES(1); VACUUM VERBOSE test; INFO: statistics for "test": -storage id: 10000000137 +storage id: 10000000139 total file size: 24576, total data size: 6 compression rate: 0.83x total row count: 1, stripe count: 1, average rows per stripe: 1 @@ -72,7 +72,7 @@ chunk count: 1, containing data for dropped columns: 0, lz4 compressed: 1 ALTER TABLE test ALTER COLUMN i TYPE int8; VACUUM VERBOSE test; INFO: statistics for "test": -storage id: 10000000138 +storage id: 10000000140 total file size: 24576, total data size: 10 compression rate: 0.90x total row count: 1, stripe count: 1, average rows per stripe: 1 diff --git a/columnar/src/test/regress/expected/columnar_create.out b/columnar/src/test/regress/expected/columnar_create.out index dc1c8cde..b4f4fc5b 100644 --- a/columnar/src/test/regress/expected/columnar_create.out +++ b/columnar/src/test/regress/expected/columnar_create.out @@ -185,3 +185,9 @@ SELECT columnar_test_helpers.columnar_metadata_has_storage_id(:columnar_temp_sto f (1 row) +-- make sure we can create a table from a table +CREATE TABLE sampletable (x numeric) using columnar; +INSERT INTO sampletable SELECT generate_series(1, 1000000, 1); +CREATE TABLE sampletable_columnar USING columnar AS SELECT * FROM sampletable ORDER BY 1 ASC; +DROP TABLE sampletable; +DROP TABLE sampletable_columnar; diff --git a/columnar/src/test/regress/sql/columnar_create.sql b/columnar/src/test/regress/sql/columnar_create.sql index c14d540b..dab6d4e1 100644 --- a/columnar/src/test/regress/sql/columnar_create.sql +++ b/columnar/src/test/regress/sql/columnar_create.sql @@ -129,3 +129,9 @@ SELECT COUNT(*)=0 FROM columnar_temp; -- since we deleted all the rows, we shouldn't have any stripes for table SELECT columnar_test_helpers.columnar_metadata_has_storage_id(:columnar_temp_storage_id); +-- make sure we can create a table from a table +CREATE TABLE sampletable (x numeric) using columnar; +INSERT INTO sampletable SELECT generate_series(1, 1000000, 1); +CREATE TABLE sampletable_columnar USING columnar AS SELECT * FROM sampletable ORDER BY 1 ASC; +DROP TABLE sampletable; +DROP TABLE sampletable_columnar;