Hi Andreas, On Tue, Feb 05, 2019 at 12:59:12PM +0900, Michael Paquier wrote: > Now... You have on this thread all the audience which already worked > on 874fe3a. And I am just looking at this patch, evaluating the > behavior change this is introducing. Still I would recommend a > separate thread as others may want to comment on that particular > point.
So I have read through your patch, and there are a couple of things which I think we could simplify more. Here are my notes: 1) We could remove the into clause from DR_intorel, which is used for two things: - Determine the relkind of the relation created. However the relation gets created before entering in the executor, and we already know its OID, so we also know its relkind. - skipData is visibly always false. We may want to keep skipData to have an assertion at the beginning of inforel_startup for sanity purposes though. 2) DefineIntoRelForDestReceiver is just a wrapper for create_ctas_nodata, so we had better just merge both of them and expose directly the routine creating the relation definition, so the new interface is a bit awkward. 3) The part about the regression diff is well... Expected... We may want a comment about that. We could consider as well adding a regression test inspired from REINDEX SCHEMA to show that the CTAS is created before the data is actually filled in. -- Michael
signature.asc
Description: PGP signature
