Re: reload-through-the-top-parent switch the partition table

From: Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: reload-through-the-top-parent switch the partition table
Date: 2017-08-11 09:36:38
Message-ID: CAGPqQf38k6X7gHQf2OsYG1nd4SRrSmmpnpWpZv3nj3_uF71u7g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Thu, Aug 10, 2017 at 8:26 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Aug 10, 2017 at 3:47 AM, Rushabh Lathia
> <rushabh(dot)lathia(at)gmail(dot)com> wrote:
> >> (1) seems like a pretty arbitrary restriction, so I don't like that
> >> option. (2) would hurt performance in some use cases. Do we have an
> >> option (3)?
> >
> > How about protecting option 2) with the load-via-partition-root
> protection.
> > Means
> > load the parents information even dump is not set only when
> > load-via-partition-root
> > & ispartition.
> >
> > This won't hurt performance for the normal cases.
>
> Yes, that seems like the right approach.
>
> + Dump data via the top-most partitioned table (rather than
> partition
> + table) when dumping data for the partition table.
>
> I think we should phrase this a bit more clearly, something like this:
> When dumping a COPY or INSERT statement for a partitioned table,
> target the root of the partitioning hierarchy which contains it rather
> than the partition itself. This may be useful when reloading data on
> a server where rows do not always fall into the same partitions as
> they did on the original server. This could happen, for example, if
> the partitioning column is of type text and the two system have
> different definitions of the collation used to partition the data.
>
>
Done.

> + printf(_(" --load-via-partition-root load partition table via
> the root relation\n"));
>
> "relation" seems odd to me here. root table?
>
>
Done.

> /* Don't bother computing anything for non-target tables, either
> */
> if (!tblinfo[i].dobj.dump)
> + {
> + /*
> + * Load the parents information for the partition table when
> + * the load-via-partition-root option is set. As we need the
> + * parents information to get the partition root.
> + */
> + if (dopt->load_via_partition_root &&
> + tblinfo[i].ispartition)
> + findParentsByOid(&tblinfo[i], inhinfo, numInherits);
> continue;
> + }
>
> Duplicating the call to findParentsByOid seems less then ideal. How
> about doing something like this:
>
> if (tblinfo[i].dobj.dump)
> {
> find_parents = true;
> mark_parents = true;
> }
> else if (dopt->load_via_partition_root && tblinfo[i].ispartition)
> find_parents = true;
>
> if (find_parents)
> findParentsByOid(&tblinfo[i], inhinfo, numInherits);
>
> etc.
>
>
Done changes to avoid duplicate call to findParentsByOid().

> The comments for this function also need some work - e.g. the function
> header comment deserves some kind of update for these changes.
>
>
Done.

> +static TableInfo *
> +getRootTableInfo(TableInfo *tbinfo)
> +{
> + Assert(tbinfo->ispartition);
> + Assert(tbinfo->numParents == 1);
> + if (tbinfo->parents[0]->ispartition)
> + return getRootTableInfo(tbinfo->parents[0]);
> +
> + return tbinfo->parents[0];
> +}
>
> This code should iterate, not recurse, to avoid any risk of blowing
> out the stack.
>
>
Done.

Please find attach patch with the changes.

Thanks,
Rushabh Lathia
www.EnterpriseDB.com

Attachment Content-Type Size
load_via_partition_root_v4.patch text/x-patch 11.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-08-11 11:18:05 Re: SCRAM protocol documentation
Previous Message Ashutosh Bapat 2017-08-11 08:52:59 Re: Foreign tables privileges not shown in information_schema.table_privileges