aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlvaro Herrera <alvherre@alvh.no-ip.org>2015-07-30 15:07:19 -0300
committerAlvaro Herrera <alvherre@alvh.no-ip.org>2015-07-30 15:07:19 -0300
commitc81276241b61d8759e024ed803e8f3f251d8e7c9 (patch)
tree3b909a6f7de7416644257c39433f41011b9390ce
parentd6314b20cd872a542d71738df54a906d2962abb8 (diff)
downloadpostgresql-c81276241b61d8759e024ed803e8f3f251d8e7c9.tar.gz
postgresql-c81276241b61d8759e024ed803e8f3f251d8e7c9.zip
Fix broken assertion in BRIN code
The code was assuming that any NULL value in scan keys was due to IS NULL or IS NOT NULL, but it turns out to be possible to get them with other operators too, if they are used in contrived-enough ways. Easiest way out of the problem seems to check explicitely for the IS NOT NULL flag, instead of assuming it must be set if the IS NULL flag is not set, when a null scan key is found; if neither flag is set, follow the lead of other index AMs and assume that all indexable operators must be strict, and thus the query is never satisfiable. Also, add a comment to try and lure some future hacker into improving analysis of scan keys in brin. Per report from Andreas Seltenreich; diagnosis by Tom Lane. Backpatch to 9.5. Discussion: http://www.postgresql.org/message-id/20646.1437919632@sss.pgh.pa.us
-rw-r--r--src/backend/access/brin/brin.c8
-rw-r--r--src/backend/access/brin/brin_inclusion.c10
-rw-r--r--src/backend/access/brin/brin_minmax.c10
3 files changed, 24 insertions, 4 deletions
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 268a55e71f9..360b26e6fc4 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -464,6 +464,14 @@ brinrescan(PG_FUNCTION_ARGS)
/* other arguments ignored */
+ /*
+ * Other index AMs preprocess the scan keys at this point, or sometime
+ * early during the scan; this lets them optimize by removing redundant
+ * keys, or doing early returns when they are impossible to satisfy; see
+ * _bt_preprocess_keys for an example. Something like that could be added
+ * here someday, too.
+ */
+
if (scankey && scan->numberOfKeys > 0)
memmove(scan->keyData, scankey,
scan->numberOfKeys * sizeof(ScanKeyData));
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 803b07f10a9..926487ec039 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -276,8 +276,14 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
* For IS NOT NULL, we can only skip ranges that are known to have
* only nulls.
*/
- Assert(key->sk_flags & SK_SEARCHNOTNULL);
- PG_RETURN_BOOL(!column->bv_allnulls);
+ if (key->sk_flags & SK_SEARCHNOTNULL)
+ PG_RETURN_BOOL(!column->bv_allnulls);
+
+ /*
+ * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+ * operators are strict and return false.
+ */
+ PG_RETURN_BOOL(false);
}
/* If it is all nulls, it cannot possibly be consistent. */
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 7cd98887c0f..2cc6e41e5f7 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -174,8 +174,14 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
* For IS NOT NULL, we can only skip ranges that are known to have
* only nulls.
*/
- Assert(key->sk_flags & SK_SEARCHNOTNULL);
- PG_RETURN_BOOL(!column->bv_allnulls);
+ if (key->sk_flags & SK_SEARCHNOTNULL)
+ PG_RETURN_BOOL(!column->bv_allnulls);
+
+ /*
+ * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+ * operators are strict and return false.
+ */
+ PG_RETURN_BOOL(false);
}
/* if the range is all empty, it cannot possibly be consistent */