Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nrUx3-0004Cb-Fn for pgsql-hackers@arkaria.postgresql.org; Thu, 19 May 2022 01:27:41 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nrUw3-0001gr-Tw for pgsql-hackers@arkaria.postgresql.org; Thu, 19 May 2022 01:26:39 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nrUw3-0001gi-HG for pgsql-hackers@lists.postgresql.org; Thu, 19 May 2022 01:26:39 +0000 Received: from mail-lf1-x12f.google.com ([2a00:1450:4864:20::12f]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nrUvw-0003mp-PD for pgsql-hackers@postgresql.org; Thu, 19 May 2022 01:26:38 +0000 Received: by mail-lf1-x12f.google.com with SMTP id w14so6479289lfl.13 for ; Wed, 18 May 2022 18:26:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UPjzlSjrCgQOUMMof7BP3SC27ImS+QzdT1NdEvSR8gc=; b=HrcoWchvmE9/WsH3I7BhEhP71f9fnOCAMh6c2pCz8UNuN+EseRkDMJR+XGOZQWPXGN YCMo7eIP9itopFPYdq4Z3UgZuFoTytnUmVJApRmYY0yaNEJs87O4OoU7zvAQ8MmHO5dL o+ch6SlH4J4GHcpvgbZD2xRJLlnf9VZ4T0aIk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UPjzlSjrCgQOUMMof7BP3SC27ImS+QzdT1NdEvSR8gc=; b=4sC02xiRmKf9IB16quSzOE8HQZqdpOa/50IG0PwvYyuFaGjtMllTfZ0O5tmsSvGsfW HkKbPcABuyoo7nHWrQ/Ph7xjepwaFLOBCXMqvnIhTYc85mSd2JQnVAGGLSSAa06qqgLy lPPcLTjIThnmwGFIaHkaXlVci0boAH6TY5MDLKlfamVJ2N4tkfAOwAKXa9pwwvwdre0C 5hcD1qUyACOCtuef9AtwjIcpqOjXWGHzBNU06fy8IWR7auzPvNd/h1zaEG12eDcfVhLP FxGwGfszDTRSVJwDL5pv7MHrHeJVCRgq8AaPLNEFsmODrBjIK7AhTIfwL2a/y8wYRM+1 /9SA== X-Gm-Message-State: AOAM532LZ6QdiNZ2gpt+xsF+Y/9mR84YNBYwODeiPagske9vDi0y6etS 5kR/7FosStLklXDYFYkBDvgYTwGpRcI5Ysj16MybJg== X-Google-Smtp-Source: ABdhPJymPADnhfheul5us6WTv2CNVVDBCtBWdOaQF34ncT1gVZWUhawJ6be0Vc4CySEca9vdxhwG2L5Se933D4JHZyU= X-Received: by 2002:ac2:4f0a:0:b0:477:c279:7839 with SMTP id k10-20020ac24f0a000000b00477c2797839mr1512117lfr.687.1652923591653; Wed, 18 May 2022 18:26:31 -0700 (PDT) MIME-Version: 1.0 References: <20220518231414.GN19626@telsasoft.com> In-Reply-To: From: Zhihong Yu Date: Wed, 18 May 2022 18:31:46 -0700 Message-ID: Subject: Re: ALTER TABLE SET ACCESS METHOD on partitioned tables To: Soumyadeep Chakraborty Cc: Justin Pryzby , pgsql-hackers , Michael Paquier , Ashwin Agrawal , vanjared@vmware.com Content-Type: multipart/alternative; boundary="00000000000085947705df534375" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000085947705df534375 Content-Type: text/plain; charset="UTF-8" On Wed, May 18, 2022 at 5:49 PM Soumyadeep Chakraborty < soumyadeep2007@gmail.com> wrote: > > On Wed, May 18, 2022 at 4:14 PM Justin Pryzby > wrote: > > > I didn't look closely yet, but this comment is wrong: > > > > + * Since these have no storage the tablespace can be updated with a > simple > > > > + * metadata only operation to update the tablespace. > > > > > Good catch. Fixed. > > > It'd be convenient if AMs worked the same way (and a bit odd that they > don't). > > Note that in v15, pg_dump/restore now allow --no-table-am, an exact > parallel to > > --no-tablespace. > > I agree that ATSET AM should behave in a similar fashion to ATSET > tablespaces. > However, the way that ATSET tablespace currently behaves is not consistent > with > the ONLY clause. > > On a given partition root: > ALTER TABLE ONLY am_partitioned SET TABLESPACE ts; > has the same effect as: > ALTER TABLE am_partitioned SET TABLESPACE ts; > > We are missing out on the feature to set the AM/tablespace throughout the > partition hierarchy, with one command. > > Regards, > Soumyadeep (VMware) > > Hi, + accessMethodId = ((Form_pg_class) GETSTRUCT(tup))->relam; - /* look up the access method, verify it is for a table */ - if (accessMethod != NULL) - accessMethodId = get_table_am_oid(accessMethod, false); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for relation %u", relid); The validity check of tup should be done before fetching the value of relam field. Cheers --00000000000085947705df534375 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, May 18, 2022 at 5:49 PM Soumy= adeep Chakraborty <soumyadee= p2007@gmail.com> wrote:

On Wed, May 18, 2022 at 4:= 14 PM Justin Pryzby <pryzby@telsasoft.com> wrote:

> I didn'= t look closely yet, but this comment is wrong:
>
> + * Since t= hese have no storage the tablespace can be updated with a simple =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <= br>> + * metadata only operation to update the tablespace. =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 <= br>
Good catch. Fixed.

> It'd be convenient if AMs worked = the same way (and a bit odd that they don't).
> Note that in v15,= pg_dump/restore now allow --no-table-am, an exact parallel to
> --no= -tablespace.

I agree that ATSET AM should behave in a similar fashio= n to ATSET tablespaces.
However, the way that ATSET tablespace currently= behaves is not consistent with
the ONLY clause.

On a given partition root:
ALTER TABLE ONLY am_p= artitioned SET TABLESPACE ts;
has the same effect as:
ALTER TABLE am_= partitioned SET TABLESPACE ts;

We are missing out on the feature to = set the AM/tablespace throughout the
partition hierarchy, with one comma= nd.

Regards,
Soumyade= ep (VMware)

=
Hi,

+ =C2=A0 =C2=A0 =C2=A0 acce= ssMethodId =3D ((Form_pg_class) GETSTRUCT(tup))->relam;

- =C2=A0 = /* look up the access method, verify it is for a table */
- =C2=A0 if (a= ccessMethod !=3D NULL)
- =C2=A0 =C2=A0 =C2=A0 accessMethodId =3D get_tab= le_am_oid(accessMethod, false);
+ =C2=A0 =C2=A0 =C2=A0 if (!HeapTupleIsV= alid(tup))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 elog(ERROR, "c= ache lookup failed for relation %u", relid);

= The validity check of tup should be done before fetching the value of relam= =C2=A0field.

Cheers
--00000000000085947705df534375--