public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 2/8] Move IS NOT NULL checks to bringetbitmap 2+ messages / 2 participants [nested] [flat]
* [PATCH 2/8] Move IS NOT NULL checks to bringetbitmap @ 2019-06-09 20:05 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2019-06-09 20:05 UTC (permalink / raw) --- src/backend/access/brin/brin.c | 116 ++++++++++++++++++++--- src/backend/access/brin/brin_inclusion.c | 62 +----------- src/backend/access/brin/brin_minmax.c | 62 +----------- 3 files changed, 109 insertions(+), 131 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 6777d48faf..caf7b62688 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -389,8 +389,10 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) BrinMemTuple *dtup; BrinTuple *btup = NULL; Size btupsz = 0; - ScanKey **keys; - int *nkeys; + ScanKey **keys, + **nullkeys; + int *nkeys, + *nnullkeys; int keyno; opaque = (BrinOpaque *) scan->opaque; @@ -415,10 +417,13 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) /* * Make room for per-attribute lists of scan keys that we'll pass to the - * consistent support procedure. + * consistent support procedure. We keep null and regular keys separate, + * so that we can easily pass regular keys to the consistent function. */ keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); + nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); + nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); /* * Preprocess the scan keys - split them into per-attribute arrays. @@ -440,14 +445,12 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) keyattno - 1)->attcollation)); /* First time we see this index attribute, so init as needed. */ - if (!keys[keyattno-1]) + if (consistentFn[keyattno - 1].fn_oid == InvalidOid) { FmgrInfo *tmp; - keys[keyattno-1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); - - /* First time this column, so look up consistent function */ - Assert(consistentFn[keyattno - 1].fn_oid == InvalidOid); + Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0)); + Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0)); tmp = index_getprocinfo(idxRel, keyattno, BRIN_PROCNUM_CONSISTENT); @@ -455,9 +458,23 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) CurrentMemoryContext); } - /* Add key to the per-attribute array. */ - keys[keyattno - 1][nkeys[keyattno - 1]] = key; - nkeys[keyattno - 1]++; + /* Add key to the proper per-attribute array. */ + if (key->sk_flags & SK_ISNULL) + { + if (!nullkeys[keyattno - 1]) + nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); + + nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key; + nnullkeys[keyattno - 1]++; + } + else + { + if (!keys[keyattno - 1]) + keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); + + keys[keyattno - 1][nkeys[keyattno - 1]] = key; + nkeys[keyattno - 1]++; + } } /* allocate an initial in-memory tuple, out of the per-range memcxt */ @@ -544,6 +561,83 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) Assert((nkeys[attno - 1] > 0) && (nkeys[attno - 1] <= scan->numberOfKeys)); + /* + * First check if there are any IS [NOT] NULL scan keys, and + * if we're violating them. In that case we can terminate + * early, without invoking the support function. + * + * As there may be more keys, we can only detemine mismatch + * within this loop. + */ + for (keyno = 0; (keyno < nnullkeys[attno - 1]); keyno++) + { + ScanKey key = nullkeys[attno - 1][keyno]; + + Assert(key->sk_attno == bval->bv_attno); + + /* interrupt the loop as soon as we find a mismatch */ + if (!addrange) + break; + + /* handle IS NULL/IS NOT NULL tests */ + if (key->sk_flags & SK_ISNULL) + { + /* IS NULL scan key, but range has no NULLs */ + if (key->sk_flags & SK_SEARCHNULL) + { + if (!bval->bv_allnulls && !bval->bv_hasnulls) + addrange = false; + + continue; + } + + /* + * For IS NOT NULL, we can only skip ranges that are + * known to have only nulls. + */ + if (key->sk_flags & SK_SEARCHNOTNULL) + { + if (bval->bv_allnulls) + addrange = false; + + continue; + } + + /* + * Neither IS NULL nor IS NOT NULL was used; assume all + * indexable operators are strict and thus return false + * with NULL value in the scan key. + */ + addrange = false; + } + } + + /* + * If any of the IS [NOT] NULL keys failed, the page range as + * a whole can't pass. So terminate the loop. + */ + if (!addrange) + break; + + /* + * So either there are no IS [NOT] NULL keys, or all passed. If + * there are no regular scan keys, we're done - the page range + * matches. If there are regular keys, but the page range is + * marked as 'all nulls' it can't possibly pass (we're assuming + * the operators are strict). + */ + + /* No regular scan keys - page range as a whole passes. */ + if (!nkeys[attno - 1]) + continue; + + /* If it is all nulls, it cannot possibly be consistent. */ + if (bval->bv_allnulls) + { + addrange = false; + break; + } + /* * Check whether the scan key is consistent with the page * range values; if so, have the pages in the range added diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index 8968886ff5..22edc6b46f 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -265,63 +265,6 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) Oid colloid = PG_GET_COLLATION(); int keyno; bool matches; - bool regular_keys = false; - - /* - * First check if there are any IS NULL scan keys, and if we're - * violating them. In that case we can terminate early, without - * inspecting the ranges. - */ - for (keyno = 0; keyno < nkeys; keyno++) - { - ScanKey key = keys[keyno]; - - Assert(key->sk_attno == column->bv_attno); - - /* handle IS NULL/IS NOT NULL tests */ - if (key->sk_flags & SK_ISNULL) - { - if (key->sk_flags & SK_SEARCHNULL) - { - if (column->bv_allnulls || column->bv_hasnulls) - continue; /* this key is fine, continue */ - - PG_RETURN_BOOL(false); - } - - /* - * For IS NOT NULL, we can only skip ranges that are known to have - * only nulls. - */ - if (key->sk_flags & SK_SEARCHNOTNULL) - { - if (column->bv_allnulls) - PG_RETURN_BOOL(false); - - continue; - } - - /* - * Neither IS NULL nor IS NOT NULL was used; assume all indexable - * operators are strict and return false. - */ - PG_RETURN_BOOL(false); - } - else - /* note we have regular (non-NULL) scan keys */ - regular_keys = true; - } - - /* - * If the page range is all nulls, it cannot possibly be consistent if - * there are some regular scan keys. - */ - if (column->bv_allnulls && regular_keys) - PG_RETURN_BOOL(false); - - /* If there are no regular keys, the page range is considered consistent. */ - if (!regular_keys) - PG_RETURN_BOOL(true); /* It has to be checked, if it contains elements that are not mergeable. */ if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE])) @@ -333,9 +276,8 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS) { ScanKey key = keys[keyno]; - /* ignore IS NULL/IS NOT NULL tests handled above */ - if (key->sk_flags & SK_ISNULL) - continue; + /* NULL keys are handled and filtered-out in bringetbitmap */ + Assert(!(key->sk_flags & SK_ISNULL)); matches = inclusion_consistent_key(bdesc, column, key, colloid); diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index 1219a3a2ab..7a7bd21cec 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -153,63 +153,6 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) Oid colloid = PG_GET_COLLATION(); Datum matches; int keyno; - bool regular_keys = false; - - /* - * First check if there are any IS NULL scan keys, and if we're - * violating them. In that case we can terminate early, without - * inspecting the ranges. - */ - for (keyno = 0; keyno < nkeys; keyno++) - { - ScanKey key = keys[keyno]; - - Assert(key->sk_attno == column->bv_attno); - - /* handle IS NULL/IS NOT NULL tests */ - if (key->sk_flags & SK_ISNULL) - { - if (key->sk_flags & SK_SEARCHNULL) - { - if (column->bv_allnulls || column->bv_hasnulls) - continue; /* this key is fine, continue */ - - PG_RETURN_BOOL(false); - } - - /* - * For IS NOT NULL, we can only skip ranges that are known to have - * only nulls. - */ - if (key->sk_flags & SK_SEARCHNOTNULL) - { - if (column->bv_allnulls) - PG_RETURN_BOOL(false); - - continue; - } - - /* - * Neither IS NULL nor IS NOT NULL was used; assume all indexable - * operators are strict and return false. - */ - PG_RETURN_BOOL(false); - } - else - /* note we have regular (non-NULL) scan keys */ - regular_keys = true; - } - - /* - * If the page range is all nulls, it cannot possibly be consistent if - * there are some regular scan keys. - */ - if (column->bv_allnulls && regular_keys) - PG_RETURN_BOOL(false); - - /* If there are no regular keys, the page range is considered consistent. */ - if (!regular_keys) - PG_RETURN_BOOL(true); matches = true; @@ -217,9 +160,8 @@ brin_minmax_consistent(PG_FUNCTION_ARGS) { ScanKey key = keys[keyno]; - /* ignore IS NULL/IS NOT NULL tests handled above */ - if (key->sk_flags & SK_ISNULL) - continue; + /* NULL keys are handled and filtered-out in bringetbitmap */ + Assert(!(key->sk_flags & SK_ISNULL)); matches = minmax_consistent_key(bdesc, column, key, colloid); -- 2.25.4 --jsivyprvf2oxfjz3 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0003-Move-processing-of-NULLs-from-BRIN-support--20200807.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Allow tests to pass in OpenSSL FIPS mode @ 2023-10-05 20:55 Tom Lane <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tom Lane @ 2023-10-05 20:55 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers I found another bit of fun we'll need to deal with: on my F38 platform, pgcrypto/3des fails as attached. Some googling finds this relevant info: https://github.com/pyca/cryptography/issues/6875 That is, FIPS deprecation of 3DES is happening even as we speak. So apparently we'll have little choice but to deal with two different behaviors for that. As before, I'm not too pleased with the user-friendliness of the error: +ERROR: encrypt error: Cipher cannot be initialized That's even less useful to a user than "unsupported". FWIW, everything else seems to pass with this patchset. I ran check-world as well as the various "must run manually" test suites. regards, tom lane Attachments: [text/x-diff] pgcrypto-regression.diffs (2.3K, ../../[email protected]/2-pgcrypto-regression.diffs) download | inline diff: diff -U3 /home/tgl/pgsql/contrib/pgcrypto/expected/3des.out /home/tgl/pgsql/contrib/pgcrypto/results/3des.out --- /home/tgl/pgsql/contrib/pgcrypto/expected/3des.out 2023-10-05 15:25:46.922080156 -0400 +++ /home/tgl/pgsql/contrib/pgcrypto/results/3des.out 2023-10-05 16:29:32.416972002 -0400 @@ -5,61 +5,25 @@ SELECT encrypt('\x8000000000000000', '\x010101010101010101010101010101010101010101010101', '3des-ecb/pad:none'); - encrypt --------------------- - \x95f8a5e5dd31d900 -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized select encrypt('', 'foo', '3des'); - encrypt --------------------- - \x752111e37a2d7ac3 -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized -- 10 bytes key select encrypt('foo', '0123456789', '3des'); - encrypt --------------------- - \xd2fb8baa1717cb02 -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized -- 22 bytes key select encrypt('foo', '0123456789012345678901', '3des'); - encrypt --------------------- - \xa44360e699269817 -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized -- decrypt select encode(decrypt(encrypt('foo', '0123456', '3des'), '0123456', '3des'), 'escape'); - encode --------- - foo -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized -- iv select encrypt_iv('foo', '0123456', 'abcd', '3des'); - encrypt_iv --------------------- - \x50735067b073bb93 -(1 row) - +ERROR: encrypt_iv error: Cipher cannot be initialized select encode(decrypt_iv('\x50735067b073bb93', '0123456', 'abcd', '3des'), 'escape'); - encode --------- - foo -(1 row) - +ERROR: decrypt_iv error: Cipher cannot be initialized -- long message select encrypt('Lets try a longer message.', '0123456789012345678901', '3des'); - encrypt --------------------------------------------------------------------- - \xb71e3422269d0ded19468f33d65cd663c28e0871984792a7b3ba0ddcecec8d2c -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized select encode(decrypt(encrypt('Lets try a longer message.', '0123456789012345678901', '3des'), '0123456789012345678901', '3des'), 'escape'); - encode ----------------------------- - Lets try a longer message. -(1 row) - +ERROR: encrypt error: Cipher cannot be initialized ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-10-05 20:55 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-06-09 20:05 [PATCH 2/8] Move IS NOT NULL checks to bringetbitmap Tomas Vondra <[email protected]> 2023-10-05 20:55 Re: Allow tests to pass in OpenSSL FIPS mode Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox