Re: inherit support for foreign tables

On Wed, Dec 3, 2014 at 4:05 PM, Ashutosh Bapat <
ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:

>
>
> On Tue, Dec 2, 2014 at 8:29 AM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp
> > wrote:
>
>> (2014/11/28 18:14), Ashutosh Bapat wrote:
>>
>>> On Thu, Nov 27, 2014 at 3:52 PM, Etsuro Fujita
>>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>>
>>> wrote:
>>> Apart from the above, I noticed that the patch doesn't consider to
>>> call ExplainForeignModify during EXPLAIN for an inherited
>>> UPDATE/DELETE, as shown below (note that there are no UPDATE remote
>>> queries displayed):
>>>
>>
>> So, I'd like to modify explain.c to show those queries like this:
>>>
>>
>> postgres=# explain verbose update parent set a = a * 2 where a = 5;
>>> QUERY PLAN
>>> ------------------------------__----------------------------
>>> --__-------------------------
>>> Update on public.parent (cost=0.00..280.77 rows=25 width=10)
>>> -> Seq Scan on public.parent (cost=0.00..0.00 rows=1 width=10)
>>> Output: (parent.a * 2), parent.ctid
>>> Filter: (parent.a = 5)
>>> Remote SQL: UPDATE public.mytable_1 SET a = $2 WHERE ctid = $1
>>> -> Foreign Scan on public.ft1 (cost=100.00..140.38 rows=12
>>> width=10)
>>> Output: (ft1.a * 2), ft1.ctid
>>> Remote SQL: SELECT a, ctid FROM public.mytable_1 WHERE ((a
>>> = 5)) FOR UPDATE
>>> Remote SQL: UPDATE public.mytable_2 SET a = $2 WHERE ctid = $1
>>> -> Foreign Scan on public.ft2 (cost=100.00..140.38 rows=12
>>> width=10)
>>> Output: (ft2.a * 2), ft2.ctid
>>> Remote SQL: SELECT a, ctid FROM public.mytable_2 WHERE ((a
>>> = 5)) FOR UPDATE
>>> (12 rows)
>>>
>>
>> Two "remote SQL" under a single node would be confusing. Also the node
>>> is labelled as "Foreign Scan". It would be confusing to show an "UPDATE"
>>> command under this "scan" node.
>>>
>>
>> I thought this as an extention of the existing (ie, non-inherited) case
>> (see the below example) to the inherited case.
>>
>> postgres=# explain verbose update ft1 set a = a * 2 where a = 5;
>> QUERY PLAN
>> ------------------------------------------------------------
>> -------------------------
>> Update on public.ft1 (cost=100.00..140.38 rows=12 width=10)
>> Remote SQL: UPDATE public.mytable_1 SET a = $2 WHERE ctid = $1
>> -> Foreign Scan on public.ft1 (cost=100.00..140.38 rows=12 width=10)
>> Output: (a * 2), ctid
>> Remote SQL: SELECT a, ctid FROM public.mytable_1 WHERE ((a = 5))
>> FOR UPDATE
>> (5 rows)
>>
>> I think we should show update commands somewhere for the inherited case
>> as for the non-inherited case. Alternatives to this are welcome.
>>
> This is not exactly extension of non-inheritance case. non-inheritance
> case doesn't show two remote SQLs under the same plan node. May be you can
> rename the label Remote SQL as Remote UPDATE/INSERT/DELETE (or something to
> that effect) for the DML command and the Foreign plan node should be
> renamed to Foreign access node or something to indicate that it does both
> the scan as well as DML. I am not keen about the actual terminology, but I
> think a reader of plan shouldn't get confused.
>
> We can leave this for committer's judgement.
>
>>
>> BTW, I was experimenting with DMLs being executed on multiple FDW server
>>> under same transaction and found that the transactions may not be atomic
>>> (and may be inconsistent), if one or more of the server fails to commit
>>> while rest of them commit the transaction. The reason for this is, we do
>>> not "rollback" the already "committed" changes to the foreign server, if
>>> one or more of them fail to "commit" a transaction. With foreign tables
>>> under inheritance hierarchy a single DML can span across multiple
>>> servers and the result may not be atomic (and may be inconsistent). So,
>>>
>>
>> IIUC, even the transactions over the local and the *single* remote server
>> are not guaranteed to be executed atomically in the current form. It is
>> possible that the remote transaction succeeds and the local one fails, for
>> example, resulting in data inconsistency between the local and the remote.
>>
>
> IIUC, while committing transactions involving a single remote server, the
> steps taken are as follows
> 1. the local changes are brought to PRE-COMMIT stage, which means that the
> transaction *will* succeed locally after successful completion of this
> phase,
> 2. COMMIT message is sent to the foreign server
> 3. If step two succeeds, local changes are committed and successful commit
> is conveyed to the client
> 4. if step two fails, local changes are rolled back and abort status is
> conveyed to the client
> 5. If step 1 itself fails, the remote changes are rolled back.
> This is as per one phase commit protocol which guarantees ACID for single
> foreign data source. So, the changes involving local and a single foreign
> server seem to be atomic and consistent.
>
>>
>> either we have to disable DMLs on an inheritance hierarchy which spans
>>> multiple servers. OR make sure that such transactions follow 2PC norms.
>>>
>>
>> -1 for disabling update queries on such an inheritance hierarchy because
>> I think we should leave that to the user's judgment. And I think 2PC is
>> definitely a separate patch.
>
>
> I agree that 2pc is much larger work and is subject for separate patch/es.
> But it may not be acceptable that changes made within a single command
> violate atomicity and consistency, which can not be controlled or altered
> by user intervention. Again, we can leave it for committer's judgement.
>
> Marking this as "ready for committer".
>

Sorry, I noticed in the commitfest app, that there are other reviewers as
well. Let's wait for them to comment.

>
>>
>> Thanks,
>>
>> Best regards,
>> Etsuro Fujita
>>
>
>
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company