From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Roger Curley <rocurley@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org, PG Bug reporting form <noreply@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: BUG #15623: Inconsistent use of default for updatable view
Date: Thu, 28 Feb 2019 16:46:57 +0900
Message-ID: <a1e31ca8-717a-094e-b701-0502f7cfb9b7@lab.ntt.co.jp> (raw)
In-Reply-To: <CAEZATCUDzfG_dLMjSkwd+2R=GUFtYoT9NWc8XebM0FUf1BDddQ@mail.gmail.com>
References: <15623-5d67a46788ec8b7f@postgresql.org>
<952caeff-2cc7-6db9-1f2b-418442b51045@lab.ntt.co.jp>
<CAEZATCUmSp3-8nLOpgGcPkpUEXK9TJGM=iA6q4E2Sn=+bwkKNA@mail.gmail.com>
<CA+HiwqGyuNo7O-G0LUz-=1f4ZSSST4-9sLhP+Twx-PFJSPRfug@mail.gmail.com>
<CAEZATCXTg_gxzf86+eTyLzJCY3OACQ71Wo_TPaeZyNAuo7frOA@mail.gmail.com>
<CAEZATCX1cN4Lcd80ZJXHf7qVRrMReOSOUF_0Z6YFhk89NNcGbw@mail.gmail.com>
<CAEZATCVGid6C7v6aMdA_7ZKPJPGYpwQ2bQhYrZ0=E5RDJQp7yw@mail.gmail.com>
<CAEZATCUDzfG_dLMjSkwd+2R=GUFtYoT9NWc8XebM0FUf1BDddQ@mail.gmail.com>
On 2019/02/27 18:37, Dean Rasheed wrote:
> On Tue, 12 Feb 2019 at 10:33, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> Here's an updated patch ...
>
> So I pushed that. However, ...
>
> Playing around with it some more, I realised that whilst this appeared
> to fix the reported problem, it exposes another issue which is down to
> the interaction between rewriteTargetListIU() and rewriteValuesRTE()
> --- for an INSERT with a VALUES RTE, rewriteTargetListIU() computes a
> list of target attribute numbers (attrno_list) from the targetlist in
> its original order, which rewriteValuesRTE() then relies on being the
> same length (and in the same order) as each of the lists in the VALUES
> RTE. That's OK for the initial invocation of those functions, but it
> breaks down when they're recursively invoked for auto-updatable views.
>> So actually, I think the right way to fix this is to give up trying to
> compute attrno_list in rewriteTargetListIU(), and have
> rewriteValuesRTE() work out the attribute assignments itself for each
> column of the VALUES RTE by examining the rewritten targetlist. That
> looks to be quite straightforward, and results in a cleaner separation
> of logic between the 2 functions, per the attached patch.
+1. Only rewriteValuesRTE needs to use that information, so it's better
to teach it to figure it by itself.
> I think that rewriteValuesRTE() should only ever see DEFAULT items for
> columns that are simple assignments to columns of the target relation,
> so it only needs to work out the target attribute numbers for TLEs
> that contain simple Vars referring to the VALUES RTE. Any DEFAULT seen
> in a column not matching that would be an error, but actually I think
> such a thing ought to be a "can't happen" error because of the prior
> checks during parse analysis, so I've used elog() to report if this
> does happen.
+ if (attrno == 0)
+ elog(ERROR, "Cannot set value in column %d to
DEFAULT", i);
Maybe: s/Cannot/cannot/g
+ Assert(list_length(sublist) == numattrs);
Isn't this Assert useless, because we're setting numattrs to
list_length(<first-sublist>) and transformInsertStmt ensures that all
sublists are same length?
Thanks,
Amit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: Langote_Amit_f8@lab.ntt.co.jp, dean.a.rasheed@gmail.com, amitlangote09@gmail.com, rocurley@gmail.com, noreply@postgresql.org, pgsql-hackers@postgresql.org
Subject: Re: BUG #15623: Inconsistent use of default for updatable view
In-Reply-To: <a1e31ca8-717a-094e-b701-0502f7cfb9b7@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox