From 096a1e3200dc28e1eae3aed5f6ca3478e2e8f8f4 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 3ee74975c..7083a2a0d 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -131,6 +131,9 @@ static bool HasUniformHashDistribution(ShardInterval **shardIntervalArray, int shardIntervalArrayLength); static bool HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArray, int shardCount); +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); @@ -700,6 +703,7 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) if (cacheEntry->partitionMethod == DISTRIBUTE_BY_NONE) { cacheEntry->hasUninitializedShardInterval = true; + cacheEntry->hasOverlappingShardInterval = true; /* * Note that during create_reference_table() call, @@ -728,6 +732,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"))); + } } @@ -903,6 +936,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, @@ -1832,6 +1911,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 65f69870a..6cc40ed4b 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -36,6 +36,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 be121413e..617cf9546 100644 --- a/src/test/regress/input/multi_outer_join_reference.source +++ b/src/test/regress/input/multi_outer_join_reference.source @@ -167,7 +167,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 2f9707b9b..49eddc8de 100644 --- a/src/test/regress/output/multi_outer_join_reference.source +++ b/src/test/regress/output/multi_outer_join_reference.source @@ -229,15 +229,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