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: DOCS: add helpful partitioning links @ 2024-03-28 17:52 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Alvaro Herrera @ 2024-03-28 17:52 UTC (permalink / raw) To: Ashutosh Bapat <[email protected]>; +Cc: Robert Treat <[email protected]>; PostgreSQL Hackers <[email protected]> On 2024-Mar-28, Ashutosh Bapat wrote: > LGTM. > > The commitfest entry is marked as RFC already. > > Thanks for taking care of the comments. Thanks for reviewing. I noticed a typo "seperate", fixed here. Also, I noticed that Robert added an empty line which looks in the source like he's breaking the paragraph -- but because he didn't add a closing </para> and an opening <para> to the next one, there's no actual new paragraph in the HTML output. My first instinct was to add those. However, upon reading the text, I noticed that the previous paragraph ends without offering an example, and then we attach the example to the paragraph that takes about CREATE TABLE LIKE showing both techniques, which seemed a bit odd. So instead I joined both paragraphs back together. I'm unsure which one looks better. Which one do you vote for? -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No hay ausente sin culpa ni presente sin disculpa" (Prov. francés) Attachments: [image/png] with-extra-para.png (101.7K, ../../[email protected]/2-with-extra-para.png) download | view image [image/png] no-extra-para.png (101.5K, ../../[email protected]/3-no-extra-para.png) download | view image [text/x-diff] 0001-doc-Improve-Partition-Maintenance-section.patch (7.3K, ../../[email protected]/4-0001-doc-Improve-Partition-Maintenance-section.patch) download | inline diff: From e6e736f44e22a7a7cf90c5d9e644ca930a921006 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <[email protected]> Date: Thu, 28 Mar 2024 18:43:24 +0100 Subject: [PATCH] doc: Improve "Partition Maintenance" section This adds some reference links and clarifies the wording a bit. Author: Robert Treat <[email protected]> Reviewed-by: Ashutosh Bapat <[email protected]> Discussion: https://postgr.es/m/CABV9wwNGn-pweak6_pvL5PJ1mivDNPKfg0Tck_1oTUETv5Y=dg@mail.gmail.com --- doc/src/sgml/ddl.sgml | 72 +++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 8616a8e9cc..c671122d86 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -4283,18 +4283,20 @@ CREATE TABLE measurement_y2008m02 PARTITION OF measurement TABLESPACE fasttablespace; </programlisting> - As an alternative, it is sometimes more convenient to create the - new table outside the partition structure, and attach it as a - partition later. This allows new data to be loaded, checked, and - transformed prior to it appearing in the partitioned table. + As an alternative to creating a new partition, it is sometimes more + convenient to create a new table separate from the partition structure + and attach it as a partition later. This allows new data to be loaded, + checked, and transformed prior to it appearing in the partitioned table. Moreover, the <literal>ATTACH PARTITION</literal> operation requires - only <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the - partitioned table, as opposed to the <literal>ACCESS - EXCLUSIVE</literal> lock that is required by <command>CREATE TABLE - ... PARTITION OF</command>, so it is more friendly to concurrent - operations on the partitioned table. - The <literal>CREATE TABLE ... LIKE</literal> option is helpful - to avoid tediously repeating the parent table's definition: + only a <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the + partitioned table rather than the <literal>ACCESS EXCLUSIVE</literal> + lock required by <command>CREATE TABLE ... PARTITION OF</command>, + so it is more friendly to concurrent operations on the partitioned table; + see <link linkend="sql-altertable-attach-partition"><literal>ALTER TABLE ... ATTACH PARTITION</literal></link> + for additional details. Furthermore, the + <link linkend="sql-createtable-parms-like"><literal>CREATE TABLE ... LIKE</literal></link> + option can be helpful to avoid tediously repeating the parent table's + definition; for example: <programlisting> CREATE TABLE measurement_y2008m02 @@ -4313,17 +4315,15 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 </para> <para> - Before running the <command>ATTACH PARTITION</command> command, it is - recommended to create a <literal>CHECK</literal> constraint on the table to - be attached that matches the expected partition constraint, as - illustrated above. That way, the system will be able to skip the scan - which is otherwise needed to validate the implicit - partition constraint. Without the <literal>CHECK</literal> constraint, + Note that when running the <command>ATTACH PARTITION</command> command, the table will be scanned to validate the partition constraint while holding an <literal>ACCESS EXCLUSIVE</literal> lock on that partition. - It is recommended to drop the now-redundant <literal>CHECK</literal> - constraint after the <command>ATTACH PARTITION</command> is complete. If - the table being attached is itself a partitioned table, then each of its + As shown above, it is recommended to avoid this scan by creating a + <literal>CHECK</literal> constraint matching the expected partition + constraint on the table prior to attaching it. Once the + <command>ATTACH PARTITION</command> is complete, it is recommended to drop + the now-redundant <literal>CHECK</literal> constraint. + If the table being attached is itself a partitioned table, then each of its sub-partitions will be recursively locked and scanned until either a suitable <literal>CHECK</literal> constraint is encountered or the leaf partitions are reached. @@ -4333,7 +4333,7 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 Similarly, if the partitioned table has a <literal>DEFAULT</literal> partition, it is recommended to create a <literal>CHECK</literal> constraint which excludes the to-be-attached partition's constraint. If - this is not done then the <literal>DEFAULT</literal> partition will be + this is not done, the <literal>DEFAULT</literal> partition will be scanned to verify that it contains no records which should be located in the partition being attached. This operation will be performed whilst holding an <literal>ACCESS EXCLUSIVE</literal> lock on the <literal> @@ -4344,21 +4344,21 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 </para> <para> - As explained above, it is possible to create indexes on partitioned tables - so that they are applied automatically to the entire hierarchy. - This is very - convenient, as not only will the existing partitions become indexed, but - also any partitions that are created in the future will. One limitation is - that it's not possible to use the <literal>CONCURRENTLY</literal> - qualifier when creating such a partitioned index. To avoid long lock - times, it is possible to use <command>CREATE INDEX ON ONLY</command> - the partitioned table; such an index is marked invalid, and the partitions - do not get the index applied automatically. The indexes on partitions can - be created individually using <literal>CONCURRENTLY</literal>, and then - <firstterm>attached</firstterm> to the index on the parent using - <command>ALTER INDEX .. ATTACH PARTITION</command>. Once indexes for all - partitions are attached to the parent index, the parent index is marked - valid automatically. Example: + As mentioned earlier, it is possible to create indexes on partitioned + tables so that they are applied automatically to the entire hierarchy. + This can be very convenient as not only will all existing partitions be + indexed, but any future partitions will be as well. However, one + limitation when creating new indexes on partitioned tables is that it + is not possible to use the <literal>CONCURRENTLY</literal> + qualifier, which could lead to long lock times. To avoid this, you can + use <command>CREATE INDEX ON ONLY</command> the partitioned table, which + creates the new index marked as invalid, preventing automatic application + to existing partitions. Instead, indexes can then be created individually + on each partition using <literal>CONCURRENTLY</literal> and + <firstterm>attached</firstterm> to the partitioned index on the parent + using <command>ALTER INDEX ... ATTACH PARTITION</command>. Once indexes for + all the partitions are attached to the parent index, the parent index will + be marked valid automatically. Example: <programlisting> CREATE INDEX measurement_usls_idx ON ONLY measurement (unitsales); -- 2.39.2 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-03-28 17:52 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]> 2024-03-28 17:52 Re: DOCS: add helpful partitioning links Alvaro Herrera <[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