Re: behaviour change - default_tablesapce + partition table - Mailing list pgsql-hackers

From Rushabh Lathia
Subject Re: behaviour change - default_tablesapce + partition table
Date
Msg-id CAGPqQf0miU8h=JNUB_7ccu0THDj_eZZgymVTQ=1Om-3D6O9kjA@mail.gmail.com
Whole thread Raw
In response to Re: behaviour change - default_tablesapce + partition table  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Responses Re: behaviour change - default_tablesapce + partition table  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
List pgsql-hackers


On Fri, May 17, 2019 at 10:30 AM Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
Agree that this behavior change seems unintentional.

On 2019/05/17 12:40, Rushabh Lathia wrote:
> Looking at the commit changes, it seems like at condition when no other
> tablespace is specified, we default the tablespace to the parent partitioned
> table's.
>
>     else if (stmt->partbound)
>     {
>         /*
>          * For partitions, when no other tablespace is specified, we default
>          * the tablespace to the parent partitioned table's.
>          */
>         Assert(list_length(inheritOids) == 1);
>         tablespaceId = get_rel_tablespace(linitial_oid(inheritOids));
>     }
>
> But here it doesn't consider the default_tablespace if the parent
> partitioned
> tablespace is an InvalidOid (which was the care before this commit).
>
> PFA patch to fix the same.

+
+               if (!OidIsValid(tablespaceId))
+                       tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence,
partitioned);
        }
        else
                tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence,

Why not change it like this instead:

@@ -681,7 +681,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid
ownerId,
         Assert(list_length(inheritOids) == 1);
         tablespaceId = get_rel_tablespace(linitial_oid(inheritOids));
     }
-    else
+
+    if (!OidIsValid(tablespaceId))
         tablespaceId = GetDefaultTablespace(stmt->relation->relpersistence,
                                             partitioned);


Yes, sure we can do that. Here is the patch for the same.


--
Rushabh Lathia
Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Do we expect tests to work with default_transaction_isolation=serializable
Next
From: Amit Langote
Date:
Subject: Re: behaviour change - default_tablesapce + partition table