Horton

Re: [HACKERS] UPDATE of partition key

Amit Khandekar <amitdkhan.pg@gmail.com>

View thread View on postgr.es
From: Amit Khandekar <amitdkhan.pg@gmail.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@enterprisedb.com>
Date: 2017-12-20T06:22:38Z
Lists: pgsql-hackers

Attachments

On 14 December 2017 at 08:11, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>
> Forgot to remove the description of update_rri and num_update_rri in the
> header comment of ExecSetupPartitionTupleRouting().
>
> -
> +extern void pull_child_partition_columns(Relation rel,
> +                             Relation parent,
> +                             Bitmapset **partcols);
>
> It seems you forgot to remove this declaration in partition.h, because I
> don't find it defined or used anywhere.

Done both of the above. Attached v30 patch has the above changes.

>
> I think some of the changes that are currently part of the main patch are
> better taken out into their own patches, because having those diffs appear
> in the main patch is kind of distracting.  Just like you now have a patch
> that introduces a PartitionTupleRouting structure.  I know that leads to
> too many patches, but it helps to easily tell less substantial changes
> from the substantial ones.

Done. Created patches as shown below :

>
> 1. Patch to rename partition_tupconv_maps to parentchild_tupconv_maps.

As per Robert's suggestion, reverted back the renaming of this field.

>
> 2. Patch that introduces has_partition_attrs() in place of
>    is_partition_attr()

0002-Changed-is_partition_attr-to-has_partition_attrs.patch

>
> 3. Patch to change the names of map_partition_varattnos() arguments

0003-Renaming-parameters-of-map_partition_var_attnos.patch

>
> 4. Patch that does the refactoring involving ExecConstrains(),
>    ExecPartitionCheck(), and the introduction of
>    ExecPartitionCheckEmitError()

0004-Refactor-CheckConstraint-related-code.patch


The preparatory patches are to be applied in order of the patch
numbers, followed by the main patch update-partition-key_v30.patch

>
>
> Regarding ExecSetupChildParentMap(), it seems to me that it could simply
> be declared as
>
> static void ExecSetupChildParentMap(ModifyTableState *mtstate);
>
> Looking at the places from where it's called, it seems that you're just
> extracting information from mtstate and passing the same for the rest of
> its arguments.
>

Agreed. But the last parameter per_leaf might be necessary. I will
defer this until I address Robert's concern about the complexity of
the related code.

> mt_is_tupconv_perpart seems like it's unnecessary.  Its function could be
> fulfilled by inspecting the state of some other fields of
> ModifyTableState.  For example, in the case of an update (operation ==
> CMD_UPDATE), if mt_partition_tuple_routing is non-NULL, then we can always
> assume that mt_childparent_tupconv_maps has entries for all partitions.
> If it's NULL, then there would be only entries for partitions that have
> sub-plans.

I think we better have this field separately for code-clarity, and to
avoid repeated execution of multiple conditions, and in order to have
some signficant Asserts() that use this field.

>
> tupconv_map_for_subplan() looks like it could be done as a macro.

Or may be inline function. I will again defer this for similar reason
as the above deferred item about ExecSetupChildParentMap parameters.

-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company

Commits

  1. Avoid referencing off the end of subplan_partition_offsets.

  2. Allow UPDATE to move rows between partitions.

  3. Remove useless lookup of root partitioned rel in ExecInitModifyTable().

  4. Factor error generation out of ExecPartitionCheck.

  5. Minor preparatory refactoring for UPDATE row movement.

  6. Simplify and encapsulate tuple routing support code.

  7. Avoid coercing a whole-row variable that is already coerced.

  8. Use ResultRelInfo ** rather than ResultRelInfo * for tuple routing.

  9. Make RelationGetPartitionDispatchInfo expand depth-first.

  10. Expand partitioned tables in PartDesc order.

  11. Use a real RT index when setting up partition tuple routing.

  12. Fix transition tables for partition/inheritance.

  13. Fix confusion about number of subplans in partitioned INSERT setup.

  14. Prevent BEFORE triggers from violating partitioning constraints.

  15. Fire per-statement triggers on partitioned tables.

  16. Fix reporting of violations in ExecConstraints, again.

  17. Don't scan partitioned tables.

  18. Allow FDWs to push down quals without breaking EvalPlanQual rechecks.