Thread: [HACKERS] [PATCH] WIP Add ALWAYS DEFERRED option for constraints
Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
immediate any constraint/trigger that is declared as ALWAYS DEFERRED.
I.e., the opposite of NOT DEFERRED. Perhaps I should make this NOT
IMMEDIATE? Making it NOT IMMEDIATE has the benefit of not having to
change the precedence of ALWAYS to avoid a shift/reduce conflict... It
may also be more in keeping with NOT DEFERRED.
Motivation:
- I have trigger procedures that must run at the end of the transaction
(after the last statement prior to COMMIT sent by the client/user),
which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs out
of, but SET CONSTRAINTS can be used to foil my triggers. I have
written SQL code to detect that constraint triggers have fired too
soon, but I'd rather not need it.
- Symmetry. If we can have NOT DEFERRABLE constraints, why not also
NOT IMMEDIABLE? :) Naturally "immediable" is not a word, but you
get the point.
- To learn my way around PostgreSQL source code in preparation for
other contributions.
Anyways, this patch is NOT passing tests at the moment, and I'm not sure
why. I'm sure I can figure it out, but first I need to understand the
failures. E.g., I see this sort of difference:
\d testschema.test_index1
Index "testschema.test_index1"
Column | Type | Definition
--------+--------+------------
id | bigint | id
-btree, for table "testschema.test_default_tab"
+f, for table "testschema.btree", predicate (test_default_tab)
which means, I think, that I've screwed up in src/bin/psql/describe.c,
don't it's not obvious to me yet how.
Some questions for experienced PostgreSQL developers:
Q0: Is this sort of patch welcomed?
Q1: Should new columns for pg_catalog.pg_constraint go at the end, or may
they be added in the middle?
Q2: Can I add new columns to information_schema tables, or are there
standards-compliance issues with that?
Q3: Is the C-style for PG documented somewhere? (sorry if I missed this)
Q4: Any ideas what I'm doing wrong in this patch series?
Nico
--
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
På tirsdag 03. oktober 2017 kl. 21:51:30, skrev Nico Williams <nico@cryptonector.com>:
Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
immediate any constraint/trigger that is declared as ALWAYS DEFERRED.
I.e., the opposite of NOT DEFERRED. Perhaps I should make this NOT
IMMEDIATE? Making it NOT IMMEDIATE has the benefit of not having to
change the precedence of ALWAYS to avoid a shift/reduce conflict... It
may also be more in keeping with NOT DEFERRED.
Motivation:
- I have trigger procedures that must run at the end of the transaction
(after the last statement prior to COMMIT sent by the client/user),
which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs out
of, but SET CONSTRAINTS can be used to foil my triggers. I have
written SQL code to detect that constraint triggers have fired too
soon, but I'd rather not need it.
- Symmetry. If we can have NOT DEFERRABLE constraints, why not also
NOT IMMEDIABLE? :) Naturally "immediable" is not a word, but you
get the point.
- To learn my way around PostgreSQL source code in preparation for
other contributions.
Anyways, this patch is NOT passing tests at the moment, and I'm not sure
why. I'm sure I can figure it out, but first I need to understand the
failures. E.g., I see this sort of difference:
\d testschema.test_index1
Index "testschema.test_index1"
Column | Type | Definition
--------+--------+------------
id | bigint | id
-btree, for table "testschema.test_default_tab"
+f, for table "testschema.btree", predicate (test_default_tab)
which means, I think, that I've screwed up in src/bin/psql/describe.c,
don't it's not obvious to me yet how.
Some questions for experienced PostgreSQL developers:
Q0: Is this sort of patch welcomed?
Q1: Should new columns for pg_catalog.pg_constraint go at the end, or may
they be added in the middle?
Q2: Can I add new columns to information_schema tables, or are there
standards-compliance issues with that?
Q3: Is the C-style for PG documented somewhere? (sorry if I missed this)
Q4: Any ideas what I'm doing wrong in this patch series?
Nico
+1.
While we're in deferrable constraints land...;
I even more often need deferrable conditional unique-indexes.
In PG you now may have:
ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, folder_type, name) DEFERRABLE INITIALLY DEFERRED;
But this isn't supported:
CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED;
Are there any plans to support this?
Thanks.
--
Andreas Joseph Krogh
Andreas Joseph Krogh
On Tue, Oct 03, 2017 at 10:10:59PM +0200, Andreas Joseph Krogh wrote: > +1. > > While we're in deferrable constraints land...; I even more often need > deferrable conditional unique-indexes. > In PG you now may have: > ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, folder_type, > name) DEFERRABLE INITIALLY DEFERRED; > > But this isn't supported: > CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) WHERE > parent_id IS NULL DEFERRABLE INITIALLY DEFERRED; > > Are there any plans to support this? Not by me, but I can take a look and, if it is trivial, do it. At a quick glance it does look like it should be easy enough to do it, at least to get started on a patch. If I can get some help with my current patch, I'll take a look :) But yes, I'd like to have full consistency between CREATE and ALTER. Everything that one can do with CREATE should be possible to do with ALTER, including IF NOT EXISTS. Nico -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Oct 03, 2017 at 02:51:30PM -0500, Nico Williams wrote: > Anyways, this patch is NOT passing tests at the moment, and I'm not sure > why. I'm sure I can figure it out, but first I need to understand the > failures. E.g., I see this sort of difference: > > \d testschema.test_index1 > Index "testschema.test_index1" > Column | Type | Definition > --------+--------+------------ > id | bigint | id > -btree, for table "testschema.test_default_tab" > +f, for table "testschema.btree", predicate (test_default_tab) > > which means, I think, that I've screwed up in src/bin/psql/describe.c, > don't it's not obvious to me yet how. Ah, I needed to adjust references to results columns. I suspect that something similar relates to other remaining failures. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 10/03/2017 10:10 PM, Andreas Joseph Krogh wrote: > While we're in deferrable constraints land...; > I even more often need deferrable /conditional /unique-indexes. > In PG you now may have: > > ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, folder_type, name) DEFERRABLE INITIALLY DEFERRED; > > > But this isn't supported: > > CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED; > > Are there any plans to support this? I don't want to hijack the thread, but you can do that with exclusion constraints. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
På onsdag 04. oktober 2017 kl. 00:24:19, skrev Vik Fearing <vik.fearing@2ndquadrant.com>:
On 10/03/2017 10:10 PM, Andreas Joseph Krogh wrote:
> While we're in deferrable constraints land...;
> I even more often need deferrable /conditional /unique-indexes.
> In PG you now may have:
>
> ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, folder_type, name) DEFERRABLE INITIALLY DEFERRED;
>
>
> But this isn't supported:
>
> CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED;
>
> Are there any plans to support this?
I don't want to hijack the thread, but you can do that with exclusion
constraints.
True.
--
Andreas Joseph Krogh
Andreas Joseph Krogh
[make check-world passes. Tests and docs included. Should be ready for
code review.]
Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
immediate any constraint/trigger that is declared as ALWAYS DEFERRED.
I.e., the opposite of NOT DEFERRED.
Motivation:
- Security.
One may have triggers they need to always be deferred and they
cannot give direct PG access because of SET CONSTRAINTS .. IMMEDIATE.
I have such triggers that must run at the end of the transaction
(after the last statement prior to COMMIT sent by the client/user),
which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs.
I have written SQL code to detect that constraint triggers have fired
too soon, but I'd rather not need it as it does slow things down (it
uses DDL event triggers and per-table triggers).
Making it easier to write secure code DEFERRED CONSTRAINT TRIGGERs
seems like a good idea to me.
- Symmetry.
Not using NOT DEFERRABLE is not the inverse of NOT DEFERRABLE. There
is no inverse at this time.
If we can have NOT DEFERRABLE constraints, why not also the inverse,
a constraint that cannot be made IMMEDIATE with SET CONSTRAINTs?
I've *not* cleaned up C style issues in surrounding -- I'm not sure
if that's desired. Not cleaning up makes it easier to see what I
changed.
Some questions for experienced PostgreSQL developers:
Q0: Is this sort of patch welcomed?
Q1: Should new columns for pg_catalog tables go at the end, or may they
be added in the middle?
FYI, I'm adding them in the middle, so they are next to related
columns.
Q2: Can I add new columns to information_schema tables, or are there
standards-compliance issues with that?
This is done in the second patch, and it can be dropped safely.
Q3: Perhaps I should make this NOT IMMEDIATE rather than ALWAYS DEFERRED?
Making it NOT IMMEDIATE has the benefit of not having to change the
precedence of ALWAYS to avoid a shift/reduce conflict... It may
also be more in keeping with NOT DEFERRED. Thoughts?
Nico
--
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Ay, NOT WIP -- I left that in the Subject: by accident. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Ah, David Fetter points out that I should also update tabe completion for psql. I'll do that at some point. I notice there's no table completion for column constraint attributes... If it's obvious enough I'll try to fix that too. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
I accidentally typoed when saving a file. Here's the new patch with that typo corrected, changes to information_schema dropped, and with the addition of tab completion of ALWAYS DEFERRED in psql. Nico -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
FYI, I've added my patch to the commitfest. https://commitfest.postgresql.org/15/1319/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Rebased (there were conflicts in the SGML files). Nico -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
I haven't really thought about this feature too hard; I just want to give you a couple of code comments. I think the catalog structure, and relatedly also the parser structures, could be made more compact. We currently have condeferrable and condeferred to represent three valid states (NOT DEFERRABLE, DEFERRABLE INITIALLY IMMEDIATE, DEFERRABLE INITIALLY DEFERRED). You are adding conalwaysdeferred, but you are adding only additional state (ALWAYS DEFERRED). So we end up with three bool fields to represent four states. I think this should all be rolled into one char field with four states. In psql and pg_dump, if you are query new catalog fields, you need to have a version check to have a different query for >=PG11. (This would likely apply whether you adopt my suggestion above or not.) Maybe a test case in pg_dump would be useful. Other than that, this looks like a pretty complete patch. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 02, 2017 at 04:20:19PM -0400, Peter Eisentraut wrote: > I haven't really thought about this feature too hard; I just want to > give you a couple of code comments. Thanks! > I think the catalog structure, and relatedly also the parser structures, > could be made more compact. We currently have condeferrable and > condeferred to represent three valid states (NOT DEFERRABLE, DEFERRABLE > INITIALLY IMMEDIATE, DEFERRABLE INITIALLY DEFERRED). You are adding > conalwaysdeferred, but you are adding only additional state (ALWAYS > DEFERRED). So we end up with three bool fields to represent four > states. I think this should all be rolled into one char field with four > states. I thought about this. I couldn't see a way to make the two existing boolean columns have a combination meaning "ALWAYS DEFERRED" that might not break something else. Since (condeferred AND NOT condeferrable) is an impossible combination today, I could use it to mean ALWAYS DEFERRED. I'm not sure how safe that would be. And it does seem like a weird way to express ALWAYS DEFERRED, though it would work. Replacing condeferred and condeferrable with a char columns also occurred to me, and though I assume backwards-incompatible changes to pg_catalog tables are fair game, I assumed everyone would prefer avoiding such changes where possible. Also, a backwards-incompatible change to the table would significantly enlarge the patch, as more version checks would be needed, particularly regarding upgrades (which are otherwise trivial). I felt adding a new column was probably safest. I'll make a backwards- incompatible change if requested, naturally, but I guess I'd want to get wider consensus on that, as I fear others may not agree. That fear may just be due to my ignorance of the community's preference as to pg_catalog backwards-compatibility vs. cleanliness. Hmmm, must I do anything special about _downgrades_? Does PostgreSQL support downgrades? > In psql and pg_dump, if you are query new catalog fields, you need to > have a version check to have a different query for >=PG11. (This would > likely apply whether you adopt my suggestion above or not.) Ah, yes, of course. I will add such a check. > Maybe a test case in pg_dump would be useful. Will do. > Other than that, this looks like a pretty complete patch. Thanks for the review! It's a small-ish patch, and my very first for PG. It was fun writing it. I greatly appreciate that PG source is easy to read. Nico -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 11/2/17 16:54, Nico Williams wrote: > Replacing condeferred and condeferrable with a char columns also > occurred to me, and though I assume backwards-incompatible changes to > pg_catalog tables are fair game, I assumed everyone would prefer > avoiding such changes where possible. I don't think there is an overriding mandate to avoid such catalog changes. Consider old clients that don't know about your new column. They might look at the catalog entries and derive information about a constraint, not being aware that there is additional information in another column that overrides that. So in such cases it's arguably better to make a break. (In any case, it might be worth waiting for a review of the rest of the patch before taking on a significant rewrite of the catalog structures.) > Hmmm, must I do anything special about _downgrades_? Does PostgreSQL > support downgrades? no -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 03, 2017 at 01:41:45PM -0400, Peter Eisentraut wrote: > On 11/2/17 16:54, Nico Williams wrote: > > Replacing condeferred and condeferrable with a char columns also > > occurred to me, and though I assume backwards-incompatible changes to > > pg_catalog tables are fair game, I assumed everyone would prefer > > avoiding such changes where possible. > > I don't think there is an overriding mandate to avoid such catalog > changes. Consider old clients that don't know about your new column. > They might look at the catalog entries and derive information about a > constraint, not being aware that there is additional information in > another column that overrides that. So in such cases it's arguably > better to make a break. Makes sense. > (In any case, it might be worth waiting for a review of the rest of the > patch before taking on a significant rewrite of the catalog structures.) I'll wait then :) When you're done with that I'll make this change (replacing those three bool columns with a single char column). > > Hmmm, must I do anything special about _downgrades_? Does PostgreSQL > > support downgrades? > > no Oh good. Thanks for clarifying that. Nico -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Oct 20, 2017 at 9:05 AM, Nico Williams <nico@cryptonector.com> wrote: > Rebased (there were conflicts in the SGML files). Hi Nico FYI that version has some stray absolute paths in constraints.source: -COPY COPY_TBL FROM '@abs_srcdir@/data/constro.data'; +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constro.data'; -COPY COPY_TBL FROM '@abs_srcdir@/data/constrf.data'; +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constrf.data'; -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Nov 06, 2017 at 05:50:21PM +1300, Thomas Munro wrote: > On Fri, Oct 20, 2017 at 9:05 AM, Nico Williams <nico@cryptonector.com> wrote: > > Rebased (there were conflicts in the SGML files). > > Hi Nico > > FYI that version has some stray absolute paths in constraints.source: > > -COPY COPY_TBL FROM '@abs_srcdir@/data/constro.data'; > +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constro.data'; > > -COPY COPY_TBL FROM '@abs_srcdir@/data/constrf.data'; > +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constrf.data'; Oops! Thanks for catching that! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 8, 2017 at 8:30 AM, Nico Williams <nico@cryptonector.com> wrote: > On Mon, Nov 06, 2017 at 05:50:21PM +1300, Thomas Munro wrote: >> On Fri, Oct 20, 2017 at 9:05 AM, Nico Williams <nico@cryptonector.com> wrote: >> > Rebased (there were conflicts in the SGML files). >> >> Hi Nico >> >> FYI that version has some stray absolute paths in constraints.source: >> >> -COPY COPY_TBL FROM '@abs_srcdir@/data/constro.data'; >> +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constro.data'; >> >> -COPY COPY_TBL FROM '@abs_srcdir@/data/constrf.data'; >> +COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constrf.data'; > > Oops! Thanks for catching that! Those are not fixed yet, and it has been three weeks since this report, so I am marking this patch as returned with feedback. Of course feel free to send an updated version if you can get into it. -- Michael
[Re-send; first attempt appears to have hit /dev/null somewhere. My apologies if you get two copies.] I've finally gotten around to rebasing this patch and making the change that was requested, which was: merge the now-would-be-three deferral- related bool columns in various pg_catalog tables into one char column. Instead of (deferrable, initdeferred, alwaysdeferred), now there is just (deferral). All tests (make check) pass. Sorry for the delay in doing this! Incidentally, I had to do commit-by-commit rebasing to make the rebase easier. I have a shell function I use for this, if anyone wants a copy of it -- sometimes it's much easier to do this than to do one huge jump. Nico --
Attachment
Nico Williams <nico@cryptonector.com> writes:
> [Re-send; first attempt appears to have hit /dev/null somewhere. My
> apologies if you get two copies.]
>
> I've finally gotten around to rebasing this patch and making the change
> that was requested, which was: merge the now-would-be-three deferral-
> related bool columns in various pg_catalog tables into one char column.
>
> Instead of (deferrable, initdeferred, alwaysdeferred), now there is just
> (deferral).
This design seems correct to me. I have a couple questions inline, and
some nits to go with them.
> All tests (make check) pass.
Thank you for adding tests!
> diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
> index 3ed9021..e82e39b 100644
> --- a/doc/src/sgml/catalogs.sgml
> +++ b/doc/src/sgml/catalogs.sgml
> @@ -2239,17 +2239,15 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
> </row>
>
> <row>
> - <entry><structfield>condeferrable</structfield></entry>
> - <entry><type>bool</type></entry>
> - <entry></entry>
> - <entry>Is the constraint deferrable?</entry>
> - </row>
> -
> - <row>
> - <entry><structfield>condeferred</structfield></entry>
> - <entry><type>bool</type></entry>
> + <entry><structfield>condeferral</structfield></entry>
> + <entry><type>char</type></entry>
> <entry></entry>
> - <entry>Is the constraint deferred by default?</entry>
> + <entry>Constraint deferral option:
> + <literal>a</literal> = always deferred,
> + <literal>d</literal> = deferrable,
> + <literal>d</literal> = deferrable initially deferred,
From the rest of the code, I think this is supposed to be 'i', not 'd'.
> + <literal>n</literal> = not deferrable
> + </entry>
> </row>
>
> <row>
> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
> index 8b276bc..795a7a9 100644
> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -1070,6 +1070,7 @@ index_create(Relation heapRelation,
>
> recordDependencyOn(&myself, &referenced, deptype);
> }
> + Assert((flags & INDEX_CREATE_ADD_CONSTRAINT) == 0);
What does this ensure, and why is it in this part of the code? We're in
the `else` branch here - isn't this always the case (i.e., Assert can
never fire), since `flags` isn't manipulated in this block?
> }
>
> /* Store dependency on parent index, if any */
> diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
> index f4e69f4..bde6199 100644
> --- a/src/backend/catalog/information_schema.sql
> +++ b/src/backend/catalog/information_schema.sql
> @@ -891,10 +891,14 @@ CREATE VIEW domain_constraints AS
> CAST(current_database() AS sql_identifier) AS domain_catalog,
> CAST(n.nspname AS sql_identifier) AS domain_schema,
> CAST(t.typname AS sql_identifier) AS domain_name,
> - CAST(CASE WHEN condeferrable THEN 'YES' ELSE 'NO' END
> + CAST(CASE WHEN condeferral = 'n' THEN 'NO' ELSE 'YES' END
> AS yes_or_no) AS is_deferrable,
> - CAST(CASE WHEN condeferred THEN 'YES' ELSE 'NO' END
> + CAST(CASE WHEN condeferral = 'i' OR condeferral = 'a' THEN 'YES' ELSE 'NO' END
> AS yes_or_no) AS initially_deferred
> + /*
> + * XXX Can we add is_always_deferred here? Are there
> + * standards considerations?
> + */
I'm not familiar enough to speak to this. Hopefully someone else can.
Absent other input, I think we should add is_always_deferred. (And if
we were building this today, probably just expose a single character
instead of three booleans.)
> FROM pg_namespace rs, pg_namespace n, pg_constraint con, pg_type t
> WHERE rs.oid = con.connamespace
> AND n.oid = t.typnamespace
> diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
> index 57519fe..41dc6a4 100644
> --- a/src/backend/commands/trigger.c
> +++ b/src/backend/commands/trigger.c
> @@ -3632,6 +3627,7 @@ typedef struct AfterTriggerSharedData
> TriggerEvent ats_event; /* event type indicator, see trigger.h */
> Oid ats_tgoid; /* the trigger's ID */
> Oid ats_relid; /* the relation it's on */
> + bool ats_alwaysdeferred; /* whether this can be deferred */
"Whether this must be deferred"? Also, I'm not sure what this is for -
it doesn't seem to be used anywhere.
> CommandId ats_firing_id; /* ID for firing cycle */
> struct AfterTriggersTableData *ats_table; /* transition table access */
> } AfterTriggerSharedData;
> diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
> index 90dfac2..dab721a 100644
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
> @@ -184,8 +185,8 @@ static void SplitColQualList(List *qualList,
> List **constraintList, CollateClause **collClause,
> core_yyscan_t yyscanner);
> static void processCASbits(int cas_bits, int location, const char *constrType,
> - bool *deferrable, bool *initdeferred, bool *not_valid,
> - bool *no_inherit, core_yyscan_t yyscanner);
> + char *deferral,
> + bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner);
Can you fix the wrapping on this?
> static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
>
> %}
> @@ -5538,17 +5568,24 @@ ConstraintAttributeSpec:
> int newspec = $1 | $2;
>
> /* special message for this case */
> - if ((newspec & (CAS_NOT_DEFERRABLE | CAS_INITIALLY_DEFERRED)) == (CAS_NOT_DEFERRABLE |
CAS_INITIALLY_DEFERRED))
> + if ((newspec & CAS_NOT_DEFERRABLE) &&
> + (newspec & (CAS_INITIALLY_DEFERRED | CAS_ALWAYS_DEFERRED)))
> ereport(ERROR,
> (errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("constraint declared INITIALLY DEFERRED must be DEFERRABLE"),
> parser_errposition(@2)));
> /* generic message for other conflicts */
> + if ((newspec & CAS_ALWAYS_DEFERRED) &&
> + (newspec & (CAS_INITIALLY_IMMEDIATE)))
> + ereport(ERROR,
> + (errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("conflicting constraint properties 1"),
> + parser_errposition(@2)));
> if ((newspec & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE)) == (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE)
||
> (newspec & (CAS_INITIALLY_IMMEDIATE | CAS_INITIALLY_DEFERRED)) == (CAS_INITIALLY_IMMEDIATE |
CAS_INITIALLY_DEFERRED))
> ereport(ERROR,
> (errcode(ERRCODE_SYNTAX_ERROR),
> - errmsg("conflicting constraint properties"),
> + errmsg("conflicting constraint properties 2"),
I'd prefer you just repeat the message (or make them more situationally
descriptive), rather than appending a number. (Repeating error messages
is in keeping with the style here.)
> parser_errposition(@2)));
> $$ = newspec;
> }
> @@ -16197,34 +16234,41 @@ SplitColQualList(List *qualList,
> */
> static void
> processCASbits(int cas_bits, int location, const char *constrType,
> - bool *deferrable, bool *initdeferred, bool *not_valid,
> - bool *no_inherit, core_yyscan_t yyscanner)
> + char *deferral,
> + bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner)
Line wrapping?
> {
> /* defaults */
> - if (deferrable)
> - *deferrable = false;
> - if (initdeferred)
> - *initdeferred = false;
> + if (deferral)
> + *deferral = 'n';
> if (not_valid)
> *not_valid = false;
>
> - if (cas_bits & (CAS_DEFERRABLE | CAS_INITIALLY_DEFERRED))
> + if (cas_bits & CAS_ALWAYS_DEFERRED)
> {
> - if (deferrable)
> - *deferrable = true;
> + if (deferral)
> + *deferral = 'a';
> else
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> /* translator: %s is CHECK, UNIQUE, or similar */
> - errmsg("%s constraints cannot be marked DEFERRABLE",
> + errmsg("%s constraints cannot be marked ALWAYS DEFERRED",
> constrType),
> parser_errposition(location)));
> - }
> -
> - if (cas_bits & CAS_INITIALLY_DEFERRED)
> + } else if (cas_bits & CAS_INITIALLY_DEFERRED)
Style on this file doesn't cuddle `else`. (i.e., `else if (cond)` gets its own
line without any braces on it.)
> + {
> + if (deferral)
> + *deferral = 'i';
> + else
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + /* translator: %s is CHECK, UNIQUE, or similar */
> + errmsg("%s constraints cannot be marked INITIALLY DEFERRED",
> + constrType),
> + parser_errposition(location)));
> + } else if (cas_bits & CAS_DEFERRABLE)
> {
> - if (initdeferred)
> - *initdeferred = true;
> + if (deferral)
> + *deferral = 'd';
> else
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
Thanks,
--Robbie
Attachment
On Tue, Jun 26, 2018 at 04:54:13PM -0400, Robbie Harwood wrote: > Nico Williams <nico@cryptonector.com> writes: > > > [Re-send; first attempt appears to have hit /dev/null somewhere. My > > apologies if you get two copies.] > > > > I've finally gotten around to rebasing this patch and making the change > > that was requested, which was: merge the now-would-be-three deferral- > > related bool columns in various pg_catalog tables into one char column. > > > > Instead of (deferrable, initdeferred, alwaysdeferred), now there is just > > (deferral). > > This design seems correct to me. I have a couple questions inline, and > some nits to go with them. Thanks for the review. I'm traveling (on vacation). I'll try to get to your comments within a week. Thanks!
On Tue, Jun 26, 2018 at 04:54:13PM -0400, Robbie Harwood wrote:
> Nico Williams <nico@cryptonector.com> writes:
>
> > [Re-send; first attempt appears to have hit /dev/null somewhere. My
> > apologies if you get two copies.]
> >
> > I've finally gotten around to rebasing this patch and making the change
> > that was requested, which was: merge the now-would-be-three deferral-
> > related bool columns in various pg_catalog tables into one char column.
> >
> > Instead of (deferrable, initdeferred, alwaysdeferred), now there is just
> > (deferral).
>
> This design seems correct to me. I have a couple questions inline, and
> some nits to go with them.
Thanks. Replies below.
> > All tests (make check) pass.
>
> Thank you for adding tests!
Well, yeah :)
> > diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
> > index 3ed9021..e82e39b 100644
> > --- a/doc/src/sgml/catalogs.sgml
> > +++ b/doc/src/sgml/catalogs.sgml
> > @@ -2239,17 +2239,15 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
> > </row>
> >
> > <row>
> > - <entry><structfield>condeferrable</structfield></entry>
> > - <entry><type>bool</type></entry>
> > - <entry></entry>
> > - <entry>Is the constraint deferrable?</entry>
> > - </row>
> > -
> > - <row>
> > - <entry><structfield>condeferred</structfield></entry>
> > - <entry><type>bool</type></entry>
> > + <entry><structfield>condeferral</structfield></entry>
> > + <entry><type>char</type></entry>
> > <entry></entry>
> > - <entry>Is the constraint deferred by default?</entry>
> > + <entry>Constraint deferral option:
> > + <literal>a</literal> = always deferred,
> > + <literal>d</literal> = deferrable,
> > + <literal>d</literal> = deferrable initially deferred,
>
> From the rest of the code, I think this is supposed to be 'i', not 'd'.
Oh yes, good catch.
> > + <literal>n</literal> = not deferrable
> > + </entry>
> > </row>
> >
> > <row>
>
> > diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
> > index 8b276bc..795a7a9 100644
> > --- a/src/backend/catalog/index.c
> > +++ b/src/backend/catalog/index.c
> > @@ -1070,6 +1070,7 @@ index_create(Relation heapRelation,
> >
> > recordDependencyOn(&myself, &referenced, deptype);
> > }
> > + Assert((flags & INDEX_CREATE_ADD_CONSTRAINT) == 0);
>
> What does this ensure, and why is it in this part of the code? We're in
> the `else` branch here - isn't this always the case (i.e., Assert can
> never fire), since `flags` isn't manipulated in this block?
Oy, nothing. I think that's a cut-n-paste error.
I'll remove it.
> > }
> >
> > /* Store dependency on parent index, if any */
>
> > diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
> > index f4e69f4..bde6199 100644
> > --- a/src/backend/catalog/information_schema.sql
> > +++ b/src/backend/catalog/information_schema.sql
> > @@ -891,10 +891,14 @@ CREATE VIEW domain_constraints AS
> > CAST(current_database() AS sql_identifier) AS domain_catalog,
> > CAST(n.nspname AS sql_identifier) AS domain_schema,
> > CAST(t.typname AS sql_identifier) AS domain_name,
> > - CAST(CASE WHEN condeferrable THEN 'YES' ELSE 'NO' END
> > + CAST(CASE WHEN condeferral = 'n' THEN 'NO' ELSE 'YES' END
> > AS yes_or_no) AS is_deferrable,
> > - CAST(CASE WHEN condeferred THEN 'YES' ELSE 'NO' END
> > + CAST(CASE WHEN condeferral = 'i' OR condeferral = 'a' THEN 'YES' ELSE 'NO' END
> > AS yes_or_no) AS initially_deferred
> > + /*
> > + * XXX Can we add is_always_deferred here? Are there
> > + * standards considerations?
> > + */
>
> I'm not familiar enough to speak to this. Hopefully someone else can.
> Absent other input, I think we should add is_always_deferred. (And if
> we were building this today, probably just expose a single character
> instead of three booleans.)
I had found the answer ("yes") in the history of this file but forgot to
remove the comment while rebasing. I'll remove the comment.
> > FROM pg_namespace rs, pg_namespace n, pg_constraint con, pg_type t
> > WHERE rs.oid = con.connamespace
> > AND n.oid = t.typnamespace
>
> > diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
> > index 57519fe..41dc6a4 100644
> > --- a/src/backend/commands/trigger.c
> > +++ b/src/backend/commands/trigger.c
> > @@ -3632,6 +3627,7 @@ typedef struct AfterTriggerSharedData
> > TriggerEvent ats_event; /* event type indicator, see trigger.h */
> > Oid ats_tgoid; /* the trigger's ID */
> > Oid ats_relid; /* the relation it's on */
> > + bool ats_alwaysdeferred; /* whether this can be deferred */
>
> "Whether this must be deferred"? Also, I'm not sure what this is for -
> it doesn't seem to be used anywhere.
Ah, this became unused during the rebase. I'll remove it.
> > CommandId ats_firing_id; /* ID for firing cycle */
> > struct AfterTriggersTableData *ats_table; /* transition table access */
> > } AfterTriggerSharedData;
>
> > diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
> > index 90dfac2..dab721a 100644
> > --- a/src/backend/parser/gram.y
> > +++ b/src/backend/parser/gram.y
> > @@ -184,8 +185,8 @@ static void SplitColQualList(List *qualList,
> > List **constraintList, CollateClause **collClause,
> > core_yyscan_t yyscanner);
> > static void processCASbits(int cas_bits, int location, const char *constrType,
> > - bool *deferrable, bool *initdeferred, bool *not_valid,
> > - bool *no_inherit, core_yyscan_t yyscanner);
> > + char *deferral,
> > + bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner);
>
> Can you fix the wrapping on this?
Maybe! :) I couldn't quite figure out what it should be. There
doesn't seem to be a consistent style of wrapping.
> > static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
> >
> > %}
> > @@ -5538,17 +5568,24 @@ ConstraintAttributeSpec:
> > int newspec = $1 | $2;
> >
> > /* special message for this case */
> > - if ((newspec & (CAS_NOT_DEFERRABLE | CAS_INITIALLY_DEFERRED)) == (CAS_NOT_DEFERRABLE |
CAS_INITIALLY_DEFERRED))
> > + if ((newspec & CAS_NOT_DEFERRABLE) &&
> > + (newspec & (CAS_INITIALLY_DEFERRED | CAS_ALWAYS_DEFERRED)))
> > ereport(ERROR,
> > (errcode(ERRCODE_SYNTAX_ERROR),
> > errmsg("constraint declared INITIALLY DEFERRED must be DEFERRABLE"),
> > parser_errposition(@2)));
> > /* generic message for other conflicts */
> > + if ((newspec & CAS_ALWAYS_DEFERRED) &&
> > + (newspec & (CAS_INITIALLY_IMMEDIATE)))
> > + ereport(ERROR,
> > + (errcode(ERRCODE_SYNTAX_ERROR),
> > + errmsg("conflicting constraint properties 1"),
> > + parser_errposition(@2)));
> > if ((newspec & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE)) == (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE)
||
> > (newspec & (CAS_INITIALLY_IMMEDIATE | CAS_INITIALLY_DEFERRED)) == (CAS_INITIALLY_IMMEDIATE
|CAS_INITIALLY_DEFERRED))
> > ereport(ERROR,
> > (errcode(ERRCODE_SYNTAX_ERROR),
> > - errmsg("conflicting constraint properties"),
> > + errmsg("conflicting constraint properties 2"),
>
> I'd prefer you just repeat the message (or make them more situationally
> descriptive), rather than appending a number. (Repeating error messages
> is in keeping with the style here.)
Oy, I forgot about these. The number was a bread crumb I forgot to
cleanup :( So sorry about that.
> > parser_errposition(@2)));
> > $$ = newspec;
> > }
> > @@ -16197,34 +16234,41 @@ SplitColQualList(List *qualList,
> > */
> > static void
> > processCASbits(int cas_bits, int location, const char *constrType,
> > - bool *deferrable, bool *initdeferred, bool *not_valid,
> > - bool *no_inherit, core_yyscan_t yyscanner)
> > + char *deferral,
> > + bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner)
>
> Line wrapping?
>
> > {
> > /* defaults */
> > - if (deferrable)
> > - *deferrable = false;
> > - if (initdeferred)
> > - *initdeferred = false;
> > + if (deferral)
> > + *deferral = 'n';
> > if (not_valid)
> > *not_valid = false;
> >
> > - if (cas_bits & (CAS_DEFERRABLE | CAS_INITIALLY_DEFERRED))
> > + if (cas_bits & CAS_ALWAYS_DEFERRED)
> > {
> > - if (deferrable)
> > - *deferrable = true;
> > + if (deferral)
> > + *deferral = 'a';
> > else
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > /* translator: %s is CHECK, UNIQUE, or similar */
> > - errmsg("%s constraints cannot be marked DEFERRABLE",
> > + errmsg("%s constraints cannot be marked ALWAYS DEFERRED",
> > constrType),
> > parser_errposition(location)));
> > - }
> > -
> > - if (cas_bits & CAS_INITIALLY_DEFERRED)
> > + } else if (cas_bits & CAS_INITIALLY_DEFERRED)
>
> Style on this file doesn't cuddle `else`. (i.e., `else if (cond)` gets its own
> line without any braces on it.)
OK.
> > + {
> > + if (deferral)
> > + *deferral = 'i';
> > + else
> > + ereport(ERROR,
> > + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > + /* translator: %s is CHECK, UNIQUE, or similar */
> > + errmsg("%s constraints cannot be marked INITIALLY DEFERRED",
> > + constrType),
> > + parser_errposition(location)));
> > + } else if (cas_bits & CAS_DEFERRABLE)
> > {
> > - if (initdeferred)
> > - *initdeferred = true;
> > + if (deferral)
> > + *deferral = 'd';
> > else
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>
> Thanks,
> --Robbie
Thank you. I'll rebase, update, and post a new patch soon.
Nico
--
On 2018-Jun-06, Nico Williams wrote: > I've finally gotten around to rebasing this patch and making the change > that was requested, which was: merge the now-would-be-three deferral- > related bool columns in various pg_catalog tables into one char column. > > Instead of (deferrable, initdeferred, alwaysdeferred), now there is just > (deferral). Nice stuff. Please add #defines for the chars in pg_constraint.h instead of using the values directly in the source. Also, catalogs.sgml has a typo in one of the values. What happens if you do SET CONSTRAINTS ALL IMMEDIATE and you have one of these constraints? I expect that it silently does not alter that constraint, but you didn't change that manpage. I suggest not to include psql/tab-complete changes with your main patch. If you want to update it, split it out to its own patch. Committer can choose to include it in one commit or not (I'm mildly inclined not to, but I'm probably inconsistent about it), but for review IMO it's better not to mix things -- It's way too easy to get entangled in silly details while editing that code, and it's not critical anyway. > Incidentally, I had to do commit-by-commit rebasing to make the rebase > easier. I have a shell function I use for this, if anyone wants a copy > of it -- sometimes it's much easier to do this than to do one huge jump. I've done this manually a few times. Please share, I'm curious. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Jul 11, 2018 at 03:13:30PM -0400, Alvaro Herrera wrote: > On 2018-Jun-06, Nico Williams wrote: > > I've finally gotten around to rebasing this patch and making the change > > that was requested, which was: merge the now-would-be-three deferral- > > related bool columns in various pg_catalog tables into one char column. > > > > Instead of (deferrable, initdeferred, alwaysdeferred), now there is just > > (deferral). > > Nice stuff. > > Please add #defines for the chars in pg_constraint.h instead of using > the values directly in the source. Also, catalogs.sgml has a typo in > one of the values. > > What happens if you do SET CONSTRAINTS ALL IMMEDIATE and you have one of > these constraints? I expect that it silently does not alter that > constraint, but you didn't change that manpage. Correct, that's the idea, that it should be possible to write deferred constraints/triggers which users cannot make immediate. For example, an audit extension that logs changes via FOR EACH ROW ALWAYS DEFERRED, or my PoC COMMIT TRIGGER implementation (which depends on deferred triggers). I missed that there is a page for SET CONSTRAINTS! I'll update it. > I suggest not to include psql/tab-complete changes with your main patch. > If you want to update it, split it out to its own patch. Committer can > choose to include it in one commit or not (I'm mildly inclined not to, > but I'm probably inconsistent about it), but for review IMO it's better > not to mix things -- It's way too easy to get entangled in silly details > while editing that code, and it's not critical anyway. OK, sure, though, of course, the committer could always leave that out themselves, no? To me though it seems that the change should be complete. > > Incidentally, I had to do commit-by-commit rebasing to make the rebase > > easier. I have a shell function I use for this, if anyone wants a copy > > of it -- sometimes it's much easier to do this than to do one huge jump. > > I've done this manually a few times. Please share, I'm curious. OK, attached is my latest version of that script, though this one is a bit changed from the one I used. This version tries to be faster / more efficient by first doing 1 commit, then 2, then 3, and so on, and on conflict aborts and halves N to try again. The idea is to only have to merge conflicts at each commit where conflicts arise, never resolving conflicts across more than one commit -- this makes is much easier to reason about conflicts! Note that the script is actually a shell function, and that it keeps state in shel variables. A better implementation would do the sort of thing that git(1) itself does to keep rebase state. Nico --
Attachment
On Wed, Jul 11, 2018 at 01:41:12PM -0500, Nico Williams wrote:
> > > @@ -5538,17 +5568,24 @@ ConstraintAttributeSpec:
> > > int newspec = $1 | $2;
> > >
> > > /* special message for this case */
> > > - if ((newspec & (CAS_NOT_DEFERRABLE | CAS_INITIALLY_DEFERRED)) == (CAS_NOT_DEFERRABLE |
CAS_INITIALLY_DEFERRED))
> > > + if ((newspec & CAS_NOT_DEFERRABLE) &&
> > > + (newspec & (CAS_INITIALLY_DEFERRED | CAS_ALWAYS_DEFERRED)))
> > > ereport(ERROR,
> > > (errcode(ERRCODE_SYNTAX_ERROR),
> > > errmsg("constraint declared INITIALLY DEFERRED must be DEFERRABLE"),
> > > parser_errposition(@2)));
> > > /* generic message for other conflicts */
> > > + if ((newspec & CAS_ALWAYS_DEFERRED) &&
> > > + (newspec & (CAS_INITIALLY_IMMEDIATE)))
> > > + ereport(ERROR,
> > > + (errcode(ERRCODE_SYNTAX_ERROR),
> > > + errmsg("conflicting constraint properties 1"),
> > > + parser_errposition(@2)));
> > > if ((newspec & (CAS_NOT_DEFERRABLE | CAS_DEFERRABLE)) == (CAS_NOT_DEFERRABLE |
CAS_DEFERRABLE)||
> > > (newspec & (CAS_INITIALLY_IMMEDIATE | CAS_INITIALLY_DEFERRED)) ==
(CAS_INITIALLY_IMMEDIATE| CAS_INITIALLY_DEFERRED))
> > > ereport(ERROR,
> > > (errcode(ERRCODE_SYNTAX_ERROR),
> > > - errmsg("conflicting constraint properties"),
> > > + errmsg("conflicting constraint properties 2"),
> >
> > I'd prefer you just repeat the message (or make them more situationally
> > descriptive), rather than appending a number. (Repeating error messages
> > is in keeping with the style here.)
>
> Oy, I forgot about these. The number was a bread crumb I forgot to
> cleanup :( So sorry about that.
So, I'm tempted not to add new messages that will require translation,
leaving these as "conflicting constraint properties".
But I'm perfectly happy to make these more informative too if that's
desired. I just don't know what the expectations are regarding message
catalog translation. Any advice?
Nico
--
Attached is an additional patch, as well as a new, rebased patch. This includes changes responsive to Álvaro Herrera's commentary about the SET CONSTRAINTS manual page. Nico --
Attachment
Nico Williams <nico@cryptonector.com> writes: > Attached is an additional patch, as well as a new, rebased patch. > > This includes changes responsive to Álvaro Herrera's commentary about > the SET CONSTRAINTS manual page. This patch looks good to me. +1; Álvaro, please update the CF entry when you're also satisfied. Thanks, --Robbie
Attachment
On Thu, Jul 12, 2018 at 03:07:33PM -0400, Robbie Harwood wrote: > This patch looks good to me. +1; Álvaro, please update the CF entry > when you're also satisfied. The patch set does not apply anymore, so I have moved it to next CF, waiting on author. -- Michael
Attachment
> On Tue, Oct 2, 2018 at 7:43 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Thu, Jul 12, 2018 at 03:07:33PM -0400, Robbie Harwood wrote: > > This patch looks good to me. +1; Álvaro, please update the CF entry > > when you're also satisfied. > > The patch set does not apply anymore, so I have moved it to next CF, > waiting on author. Nothing was changed since then. Since the patch got some interest I'm moving it to the next CF, but please post the rebased version.
Hi, On 2018-11-29 19:04:24 +0100, Dmitry Dolgov wrote: > > On Tue, Oct 2, 2018 at 7:43 AM Michael Paquier <michael@paquier.xyz> wrote: > > > > On Thu, Jul 12, 2018 at 03:07:33PM -0400, Robbie Harwood wrote: > > > This patch looks good to me. +1; Álvaro, please update the CF entry > > > when you're also satisfied. > > > > The patch set does not apply anymore, so I have moved it to next CF, > > waiting on author. > > Nothing was changed since then. Since the patch got some interest I'm moving it > to the next CF, but please post the rebased version. Since this hasn't happened yet, I'm going to mark it as returned with feedback. Greetings, Andres Freund