From 07acc17df6baa69046e22a590f3c58dce0e562d0 Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Mon, 5 Jan 2015 12:06:39 +0200 Subject: [PATCH 1/4] Misleading error for incompletely created tables Before this fix, if you try to execute any query on tables which are distributed with master_create_distributed_table but no shards are created yet for the table (ie master_create_worker_shards not called), you get unclear error message. This fix catches that case implicitly and more meaningful message is shown. --- pg_shard.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pg_shard.c b/pg_shard.c index 3ecdc09..5d09738 100644 --- a/pg_shard.c +++ b/pg_shard.c @@ -226,8 +226,18 @@ PgShardPlanner(Query *query, int cursorOptions, ParamListInfo boundParams) ErrorIfQueryNotSupported(distributedQuery); - /* compute the list of shards this query needs to access */ + /* + * Compute the list of shards this query needs to access. + * Error out if there are no existing shards for the table. + */ queryShardList = DistributedQueryShardList(distributedQuery); + if (queryShardList == NIL) + { + ereport(ERROR, (errmsg("cannot plan SELECT query"), + errdetail("There are no existing shards for the distributed table."), + errhint("Run \"master_create_worker_shards\" to create shards " + "for the distributed table."))); + } /* * If a select query touches multiple shards, we don't push down the From 6e690cbdae08ca7994ffc5766b6208cc947c3aae Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Mon, 5 Jan 2015 13:39:15 +0200 Subject: [PATCH 2/4] Add unit test for distributed tables This commit aims to add a unit test for executing queries on distributed tables. Test aims to get the error message when there are no shards created for the distributed tables. --- expected/queries.out | 5 +++++ sql/queries.sql | 3 +++ 2 files changed, 8 insertions(+) diff --git a/expected/queries.out b/expected/queries.out index f96bc14..1792371 100644 --- a/expected/queries.out +++ b/expected/queries.out @@ -13,6 +13,11 @@ SELECT master_create_distributed_table('articles', 'author_id'); (1 row) +-- test when a table is distributed but no shards created yet +SELECT count(*) from articles; +ERROR: cannot plan SELECT query +DETAIL: There are no existing shards for the distributed table. +HINT: Run "master_create_worker_shards" to create shards for the distributed table. \set VERBOSITY terse SELECT master_create_worker_shards('articles', 2, 1); WARNING: Connection failed to adeadhost:5432 diff --git a/sql/queries.sql b/sql/queries.sql index bb1a8b6..4254b62 100644 --- a/sql/queries.sql +++ b/sql/queries.sql @@ -11,6 +11,9 @@ CREATE TABLE articles ( SELECT master_create_distributed_table('articles', 'author_id'); +-- test when a table is distributed but no shards created yet +SELECT count(*) from articles; + \set VERBOSITY terse SELECT master_create_worker_shards('articles', 2, 1); \set VERBOSITY default From 6678cee75860417f6ce418477268cd348a8ae54f Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Thu, 8 Jan 2015 13:57:53 +0200 Subject: [PATCH 3/4] Update misleading error messages for incompletely created tables This commit updates the previous solution for error messages for incompletely created tables. This commit also updates unit tests related to the tables that are marked as distributed but no shards created yet. --- expected/queries.out | 6 +++--- pg_shard.c | 36 ++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/expected/queries.out b/expected/queries.out index 1792371..09cd148 100644 --- a/expected/queries.out +++ b/expected/queries.out @@ -15,9 +15,9 @@ SELECT master_create_distributed_table('articles', 'author_id'); -- test when a table is distributed but no shards created yet SELECT count(*) from articles; -ERROR: cannot plan SELECT query -DETAIL: There are no existing shards for the distributed table. -HINT: Run "master_create_worker_shards" to create shards for the distributed table. +ERROR: could not find any shards for query +DETAIL: No shards exist for distributed table "articles". +HINT: Run master_create_worker_shards to create shards and try again. \set VERBOSITY terse SELECT master_create_worker_shards('articles', 2, 1); WARNING: Connection failed to adeadhost:5432 diff --git a/pg_shard.c b/pg_shard.c index 5d09738..a498973 100644 --- a/pg_shard.c +++ b/pg_shard.c @@ -231,13 +231,6 @@ PgShardPlanner(Query *query, int cursorOptions, ParamListInfo boundParams) * Error out if there are no existing shards for the table. */ queryShardList = DistributedQueryShardList(distributedQuery); - if (queryShardList == NIL) - { - ereport(ERROR, (errmsg("cannot plan SELECT query"), - errdetail("There are no existing shards for the distributed table."), - errhint("Run \"master_create_worker_shards\" to create shards " - "for the distributed table."))); - } /* * If a select query touches multiple shards, we don't push down the @@ -540,17 +533,36 @@ ExtractRangeTableEntryWalker(Node *node, List **rangeTableList) /* * DistributedQueryShardList prunes the shards for the table in the query based - * on the query's restriction qualifiers, and returns this list. + * on the query's restriction qualifiers, and returns this list. If the function + * cannot find any shards for the distributed table, it errors out. In other sense, + * the function errors out or returns a non-empty list. */ static List * DistributedQueryShardList(Query *query) { - Oid distributedTableId = ExtractFirstDistributedTableId(query); + List *restrictClauseList = NIL; + List *prunedShardList = NIL; - List *restrictClauseList = QueryRestrictList(query); + Oid distributedTableId = ExtractFirstDistributedTableId(query); List *shardIntervalList = LookupShardIntervalList(distributedTableId); - List *prunedShardList = PruneShardList(distributedTableId, restrictClauseList, - shardIntervalList); + + /* error out if no shards exists for the table */ + if (shardIntervalList == NIL) + { + char *relName = get_rel_name(distributedTableId); + + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("could not find any shards for query"), + errdetail("No shards exist for distributed table" + " \"%s\".", relName), + errhint("Run master_create_worker_shards to " + "create shards " + "and try again."))); + } + + restrictClauseList = QueryRestrictList(query); + prunedShardList = PruneShardList(distributedTableId, restrictClauseList, + shardIntervalList); return prunedShardList; } From acef92ffd6514de7da84afc8ed027c8bf821b53a Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Fri, 9 Jan 2015 11:02:20 +0200 Subject: [PATCH 4/4] Final style changes Minor variable naming/error style fixes. --- pg_shard.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pg_shard.c b/pg_shard.c index a498973..7883a7c 100644 --- a/pg_shard.c +++ b/pg_shard.c @@ -549,14 +549,13 @@ DistributedQueryShardList(Query *query) /* error out if no shards exists for the table */ if (shardIntervalList == NIL) { - char *relName = get_rel_name(distributedTableId); + char *relationName = get_rel_name(distributedTableId); ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("could not find any shards for query"), - errdetail("No shards exist for distributed table" - " \"%s\".", relName), - errhint("Run master_create_worker_shards to " - "create shards " + errdetail("No shards exist for distributed table \"%s\".", + relationName), + errhint("Run master_create_worker_shards to create shards " "and try again."))); }