Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge conflict in ec0925c22a3da7199650c9903a03a0017705ed5c Fix ENABLE/DISABLE TRIGGER to handle recursion correctly #68

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

Sairakan
Copy link

Merge conflict due to both trying to add an extra field to Struct AlterTableCmd, resolve by just taking both changes.

Extension PR for validation: amazon-aurora/babelfish_extensions#54

Merge conflict in src/include/nodes/parsenodes.h
typedef struct AlterTableCmd	/* one subcommand of an ALTER TABLE */
{
	NodeTag		type;
	AlterTableType subtype;		/* Type of table alteration to apply */
	char	   *name;			/* column, constraint, or trigger to act on,
								 * or tablespace */
	int16		num;			/* attribute number for columns referenced by
								 * number */
	RoleSpec   *newowner;
	Node	   *def;			/* definition of new column, index,
								 * constraint, or parent table */
	DropBehavior behavior;		/* RESTRICT or CASCADE for DROP cases */
	bool		missing_ok;		/* skip error if missing? */
<<<<<<< ours
	char	   *schemaname;
=======
	bool		recurse;		/* exec-time recursion */
>>>>>>> theirs
} AlterTableCmd;

BABELFISH-CONFLICT: see Postgres community repo for original commit

Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b did is
not correct, because ATPrepCmd() can't distinguish between triggers that
may be cloned and those that may not, so would wrongly try to recurse
for the latter category of triggers.

So this commit restores the code in EnableDisableTrigger() that
86f5759 had added to do the recursion, which would do it only for
triggers that may be cloned, that is, row-level triggers.  This also
changes tablecmds.c such that ATExecCmd() is able to pass the value of
ONLY flag down to EnableDisableTrigger() using its new 'recurse'
parameter.

This also fixes what seems like an oversight of 86f5759 that the
recursion to partition triggers would only occur if EnableDisableTrigger()
had actually changed the trigger.  It is more apt to recurse to inspect
partition triggers even if the parent's trigger didn't need to be
changed: only then can we be certain that all descendants share the same
state afterwards.

Backpatch all the way back to 11, like bbb927b.  Care is taken not
to break ABI compatibility (and that no catversion bump is needed.)

Co-authored-by: Amit Langote <[email protected]>
Reviewed-by: Dmitry Koval <[email protected]>
Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
(cherry picked from commit ec0925c22a3da7199650c9903a03a0017705ed5c)
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 30, 2023
@2jungkook 2jungkook merged commit 1646725 into BABEL_main Dec 1, 2023
4 checks passed
@2jungkook 2jungkook deleted the pg16-cherry-pick-bff-1 branch December 1, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants