Feedback-ID: i131946ab:Fastmail
Message-ID: <2fa8b595-62e6-49db-02aa-a01e2455ba79@enterprisedb.com>
Date: Mon, 13 Jun 2022 09:35:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.9.1
Subject: Re: Add header support to text format and matching feature
Content-Language: en-US
To: Julien Rouhaud <rjuju123@gmail.com>, Andrew Dunstan <andrew@dunslane.net>
Cc: Greg Stark <stark@mit.edu>, =?UTF-8?Q?R=c3=a9mi_Lapeyre?=
 <remi.lapeyre@lenstra.fr>, Zhihong Yu <zyu@yugabyte.com>,
 David Steele <david@pgmasters.net>, Michael Paquier <michael@paquier.xyz>,
 vignesh C <vignesh21@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Daniel Verite <daniel@manitou-mail.org>
References: <94FDDB44-A608-4C75-A733-9F8FC5180D6B@lenstra.fr>
 <4816b2df-646f-dc19-e5cc-4bfbd04c0e43@enterprisedb.com>
 <07891B9D-2140-4C06-AF96-C14F1E1B0E8E@lenstra.fr>
 <f62f0262-2a3c-8839-5ced-25ca2421d738@enterprisedb.com>
 <8eba1968-9503-5be7-b473-37383f17bb72@enterprisedb.com>
 <CAM-w4HPkjYQGPm0-zx3XZHnQ_+B8AyWuCn_LyTeX7sNJCv4x8A@mail.gmail.com>
 <ae4dc590-090f-ae9b-6fdb-01a901dd4841@enterprisedb.com>
 <206ac69a-4e47-c49a-66ae-e748c339ba35@enterprisedb.com>
 <20220607154744.vvmitnqhyxrne5ms@jrouhaud>
 <d35e05ac-fde1-0dba-39bf-b30e75082b48@dunslane.net>
 <20220613023213.bj7qjnplvgfmx522@jrouhaud>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <20220613023213.bj7qjnplvgfmx522@jrouhaud>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/2fa8b595-62e6-49db-02aa-a01e2455ba79%40enterprisedb.com>
Precedence: bulk

On 13.06.22 04:32, Julien Rouhaud wrote:
>> I think it makes more sense to have a sanity check to prevent HEADER
>> MATCH with COPY TO.
> 
> I'm fine with it.  I added such a check and mentioned it in the documentation.

> I think it would still be problematic if the target table has dropped columns.
> Fortunately, as I initially thought the problem is only due to a thinko in the
> original commit which used a wrong variable for the raw_fields offset.  Once
> fixed (attached v1) I didn't see any other problem in the rest of the logic and
> all the added regression tests work as expected.

Thanks for this patch.  I'll check it in detail in a bit.  It looks good 
to me at first glance.