On Fri, Jun 24, 2022 at 8:29 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Jun 24, 2022 at 7:08 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > I have changed that. PFA, the updated patch.
>
> Apart from one minor nitpick (see below) I don't see a problem with
> this in isolation. It seems like a pretty clean renaming. So I think
> we need to move onto the question of how clean the rest of the patch
> series looks with this as a base.
>
PFA, the remaining set of patches. It might need to fix some
indentation but lets first see how is the overall idea then we can
work on it. I have fixed all the open review comment from the
previous thread except this comment from Robert.
>- It looks to me like you need to give significantly more thought to
> the proper way of adjusting the relfilenode-related test cases in
> alter_table.out.
It seems to me that this test case is just testing whether the
table/child table are rewritten or not after the alter table. And for
that it is comparing the oid with the relfilenode, now that is not
possible so I think it's quite reasonable to just compare the current
relfilenode with the old relfilenode and if they are same the table is
not rewritten. So I am not sure why the original test case had two
cases 'own' and 'orig'. With respect to this test case they both have
the same meaning, in fact comparing old relfilenode with current
relfilenode is better way of testing than comparing the oid with
relfilenode.
diff --git a/src/test/regress/expected/alter_table.out
b/src/test/regress/expected/alter_table.out
index 5ede56d..80af97e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2164,7 +2164,6 @@ select relname,
c.oid = oldoid as orig_oid,
case relfilenode
when 0 then 'none'
- when c.oid then 'own'
when oldfilenode then 'orig'
else 'OTHER'
end as storage,
@@ -2175,10 +2174,10 @@ select relname,
relname | orig_oid | storage | desc
------------------------------+----------+---------+---------------
at_partitioned | t | none |
- at_partitioned_0 | t | own |
- at_partitioned_0_id_name_key | t | own | child 0 index
- at_partitioned_1 | t | own |
- at_partitioned_1_id_name_key | t | own | child 1 index
+ at_partitioned_0 | t | orig |
+ at_partitioned_0_id_name_key | t | orig | child 0 index
+ at_partitioned_1 | t | orig |
+ at_partitioned_1_id_name_key | t | orig | child 1 index
at_partitioned_id_name_key | t | none | parent index
(6 rows)
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com