On Mon, Feb 25, 2019 at 09:51:52PM +0100, Peter Eisentraut wrote:
> On 2019-01-15 08:13, Michael Paquier wrote:
>> + bool has_generated_stored;
>> + bool has_generated_virtual;
>> } TupleConstr;
>> Could have been more simple to use a char as representation here.
>
> Seems confusing if both apply at the same time.
Ouch, I see. The flags count for all attributes. I missed that in a
previous read of the patch. Yeah, two booleans make sense.
>> When testing a bulk INSERT into a table which has a stored generated
>> column, memory keeps growing in size linearly, which does not seem
>> normal to me.
>
> This was a silly coding error. It's fixed in v8.
Thanks, this one looks fine.
>> +/*
>> + * Thin wrapper around libpq to obtain server version.
>> + */
>> +static int
>> +libpqrcv_server_version(WalReceiverConn *conn)
>> This should be introduced in separate patch in my opinion (needed
>> afterwards for logirep).
>
> Yes, it could be committed separately.
I would split that one and I think that it could go in. If you wish
to keep things grouped that's fine by me as well.
>> What about the catalog representation of attgenerated? Would it merge
>> with attidentity & co? Or not?
>
> I think the way it is now seems best. The other options that were
> discussed are also plausible, but that the discussions did not reveal
> any overwhelming arguments for a a change.
Hm. Does the SQL standard mention more features which could be merged
with stored values, virtual values, default expressions and identity
columns? I don't know the last trends in this area but I am wondering
if there are any other column specific, expression-like features like
that associated to a column. That could give more strength with
having one column in pg_attribute to govern them all. Well, assuming
that something else is implemented of course. That's a lot of
assumptions, and it's not like the current implementation is wrong
either.
--
Michael