Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. - Mailing list pgsql-hackers
From | Masahiko Sawada |
---|---|
Subject | Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. |
Date | |
Msg-id | CAD21AoCEY3S-N54-8TaBYGiuS=rf4LX9E=cPM1gYwHM7t67jZA@mail.gmail.com Whole thread Raw |
In response to | Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table.
Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. |
List | pgsql-hackers |
On Wed, Jul 5, 2023 at 2:46 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, Jul 5, 2023 at 9:01 AM Peter Smith <smithpb2250@gmail.com> wrote: > > > > Hi. Here are some review comments for this patch. > > > > +1 for the patch idea. > > > > ------ > > > > I wasn't sure about the code comment adjustments suggested by Amit [1]: > > "Accordingly, the comments atop build_replindex_scan_key(), > > FindUsableIndexForReplicaIdentityFull(), IsIndexOnlyOnExpression() > > should also be adjusted." As for IsIndexOnlyOnExpression(), what part do you think we need to adjust? It says: /* * Returns true if the given index consists only of expressions such as: * CREATE INDEX idx ON table(foo(col)); * * Returns false even if there is one column reference: * CREATE INDEX idx ON table(foo(col), col_2); */ and it seems to me that the function doesn't check if the leftmost index column is a non-expression. > > doc/src/sgml/logical-replication.sgml > > > > 1. > > the key. When replica identity <literal>FULL</literal> is specified, > > indexes can be used on the subscriber side for searching the rows. > > Candidate > > indexes must be btree, non-partial, and have at least one column reference > > - (i.e. cannot consist of only expressions). These restrictions > > - on the non-unique index properties adhere to some of the restrictions that > > - are enforced for primary keys. If there are no such suitable indexes, > > + at the leftmost column indexes (i.e. cannot consist of only > > expressions). These > > + restrictions on the non-unique index properties adhere to some of > > the restrictions > > + that are enforced for primary keys. If there are no such suitable indexes, > > the search on the subscriber side can be very inefficient, therefore > > replica identity <literal>FULL</literal> should only be used as a > > fallback if no other solution is possible. If a replica identity other > > > > Isn't this using the word "indexes" with different meanings in the > > same sentence? e.g. IIUC "leftmost column indexes" is referring to the > > ordinal number of the index fields. That was my mistake, it should be " at the leftmost column". > > I don't know if this suggestion is what the code is actually doing. In > function RemoteRelContainsLeftMostColumnOnIdx(), we have the following > checks: > ========== > keycol = indexInfo->ii_IndexAttrNumbers[0]; > if (!AttributeNumberIsValid(keycol)) > return false; > > if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol)) > return false; > > return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0; > ========== > > The first of these checks indicates that the leftmost column of the > index should be non-expression, second and third indicates what you > suggest in your wording. We can also think that what you wrote in a > way is a superset of "leftmost index column is a non-expression" and > "leftmost index column should be present in remote rel" but I feel it > would be better to explicit about the first part as it is easy to > understand for users at least in docs. +1 Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
pgsql-hackers by date: