From 6bd2e3ed302e83f7dacab0b6d76695691c1d31d1 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 27 Apr 2017 18:17:14 -0700 Subject: [PATCH] Add DistTableCacheEntry->hasOverlappingShardInterval. This determines whether it's possible to perform binary search on sortedShardIntervalArray or not. If e.g. two shards have overlapping ranges, that'd be prohibitive. That'll be useful in later commit introducing faster shard pruning. --- .../distributed/utils/metadata_cache.c | 80 +++++++++++++++++++ src/include/distributed/metadata_cache.h | 1 + .../input/multi_outer_join_reference.source | 2 +- .../output/multi_outer_join_reference.source | 5 +- 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index a36dd5c46..15062f78e 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -145,6 +145,9 @@ static bool HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArr static void ErrorIfInstalledVersionMismatch(void); static char * AvailableExtensionVersion(void); static char * InstalledExtensionVersion(void); +static bool HasOverlappingShardInterval(ShardInterval **shardIntervalArray, + int shardIntervalArrayLength, + FmgrInfo *shardIntervalSortCompareFunction); static void InitializeDistTableCache(void); static void InitializeWorkerNodeCache(void); static uint32 WorkerNodeHashCode(const void *key, Size keySize); @@ -714,6 +717,7 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) if (cacheEntry->partitionMethod == DISTRIBUTE_BY_NONE) { cacheEntry->hasUninitializedShardInterval = true; + cacheEntry->hasOverlappingShardInterval = true; /* * Note that during create_reference_table() call, @@ -742,6 +746,35 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) cacheEntry->hasUninitializedShardInterval = HasUninitializedShardInterval(sortedShardIntervalArray, shardIntervalArrayLength); + + if (!cacheEntry->hasUninitializedShardInterval) + { + cacheEntry->hasOverlappingShardInterval = + HasOverlappingShardInterval(sortedShardIntervalArray, + shardIntervalArrayLength, + shardIntervalCompareFunction); + } + else + { + cacheEntry->hasOverlappingShardInterval = true; + } + + /* + * If table is hash-partitioned and has shards, there never should be + * any uninitalized shards. Historically we've not prevented that for + * range partitioned tables, but it might be a good idea to start + * doing so. + */ + if (cacheEntry->partitionMethod == DISTRIBUTE_BY_HASH && + cacheEntry->hasUninitializedShardInterval) + { + ereport(ERROR, (errmsg("hash partitioned table has uninitialized shards"))); + } + if (cacheEntry->partitionMethod == DISTRIBUTE_BY_HASH && + cacheEntry->hasOverlappingShardInterval) + { + ereport(ERROR, (errmsg("hash partitioned table has overlapping shards"))); + } } @@ -917,6 +950,52 @@ HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArray, int shar } +/* + * HasOverlappingShardInterval determines whether the given list of sorted + * shards has overlapping ranges. + */ +static bool +HasOverlappingShardInterval(ShardInterval **shardIntervalArray, + int shardIntervalArrayLength, + FmgrInfo *shardIntervalSortCompareFunction) +{ + int shardIndex = 0; + ShardInterval *lastShardInterval = NULL; + Datum comparisonDatum = 0; + int comparisonResult = 0; + + /* zero/a single shard can't overlap */ + if (shardIntervalArrayLength < 2) + { + return false; + } + + lastShardInterval = shardIntervalArray[0]; + for (shardIndex = 1; shardIndex < shardIntervalArrayLength; shardIndex++) + { + ShardInterval *curShardInterval = shardIntervalArray[shardIndex]; + + /* only called if !hasUninitializedShardInterval */ + Assert(lastShardInterval->minValueExists && lastShardInterval->maxValueExists); + Assert(curShardInterval->minValueExists && curShardInterval->maxValueExists); + + comparisonDatum = CompareCall2(shardIntervalSortCompareFunction, + lastShardInterval->maxValue, + curShardInterval->minValue); + comparisonResult = DatumGetInt32(comparisonDatum); + + if (comparisonResult >= 0) + { + return true; + } + + lastShardInterval = curShardInterval; + } + + return false; +} + + /* * CitusHasBeenLoaded returns true if the citus extension has been created * in the current database and the extension script has been executed. Otherwise, @@ -2138,6 +2217,7 @@ ResetDistTableCacheEntry(DistTableCacheEntry *cacheEntry) cacheEntry->shardIntervalArrayLength = 0; cacheEntry->hasUninitializedShardInterval = false; cacheEntry->hasUniformHashDistribution = false; + cacheEntry->hasOverlappingShardInterval = false; } diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index 5cc2bbf26..ae83940d4 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -38,6 +38,7 @@ typedef struct bool isDistributedTable; bool hasUninitializedShardInterval; bool hasUniformHashDistribution; /* valid for hash partitioned tables */ + bool hasOverlappingShardInterval; /* pg_dist_partition metadata for this table */ char *partitionKeyString; diff --git a/src/test/regress/input/multi_outer_join_reference.source b/src/test/regress/input/multi_outer_join_reference.source index f1e5946c4..9a106eb61 100644 --- a/src/test/regress/input/multi_outer_join_reference.source +++ b/src/test/regress/input/multi_outer_join_reference.source @@ -166,7 +166,7 @@ FROM -- load some more data \copy multi_outer_join_right_reference FROM '@abs_srcdir@/data/customer-21-30.data' with delimiter '|' --- Update shards so that they do not have 1-1 matching. We should error here. +-- Update shards so that they do not have 1-1 matching, triggering an error. UPDATE pg_dist_shard SET shardminvalue = '2147483646' WHERE shardid = 1260006; UPDATE pg_dist_shard SET shardmaxvalue = '2147483647' WHERE shardid = 1260006; SELECT diff --git a/src/test/regress/output/multi_outer_join_reference.source b/src/test/regress/output/multi_outer_join_reference.source index 82d1b88ed..a2b17a0fa 100644 --- a/src/test/regress/output/multi_outer_join_reference.source +++ b/src/test/regress/output/multi_outer_join_reference.source @@ -228,15 +228,14 @@ LOG: join order: [ "multi_outer_join_left_hash" ][ broadcast join "multi_outer_ -- load some more data \copy multi_outer_join_right_reference FROM '@abs_srcdir@/data/customer-21-30.data' with delimiter '|' --- Update shards so that they do not have 1-1 matching. We should error here. +-- Update shards so that they do not have 1-1 matching, triggering an error. UPDATE pg_dist_shard SET shardminvalue = '2147483646' WHERE shardid = 1260006; UPDATE pg_dist_shard SET shardmaxvalue = '2147483647' WHERE shardid = 1260006; SELECT min(l_custkey), max(l_custkey) FROM multi_outer_join_left_hash a LEFT JOIN multi_outer_join_right_hash b ON (l_custkey = r_custkey); -ERROR: cannot perform distributed planning on this query -DETAIL: Shards of relations in outer join queries must have 1-to-1 shard partitioning +ERROR: hash partitioned table has overlapping shards UPDATE pg_dist_shard SET shardminvalue = '-2147483648' WHERE shardid = 1260006; UPDATE pg_dist_shard SET shardmaxvalue = '-1073741825' WHERE shardid = 1260006; -- empty tables