On Mon, Feb 25, 2019 at 09:46:35PM +0100, Peter Eisentraut wrote: > The virtual generated column part is still a bit iffy. I'm still > finding places here and there where virtual columns are not being > expanded correctly. Maybe it needs more refactoring. One big unsolved > issue is how the storage of such columns should work. Right now, they > are stored as nulls. That works fine, but what I suppose we'd really > want is to not store them at all. That, however, creates all kinds of > complications in the planner if target lists have non-matching lengths > or the resnos don't match up. I haven't figured out how to do this > cleanly.
Hmm. Not storing virtual columns looks like the correct concept to me instead of storing them as NULL. > So I'm thinking if we can get agreement on the stored columns, I can cut > out the virtual column stuff for PG12. That should be fairly easy. The shape of what is used for stored columns looks fine to me. + if (attgenerated) + { + /* + * Generated column: Dropping anything that the generation expression + * refers to automatically drops the generated column. + */ + recordDependencyOnSingleRelExpr(&colobject, expr, RelationGetRelid(rel), + DEPENDENCY_AUTO, + DEPENDENCY_AUTO, false); + } A CCI is not necessary I think here, still the recent thread about automatic dependencies with identity columns had a much similar pattern... + else if (generated[0] == ATTRIBUTE_GENERATED_VIRTUAL) + default_str = psprintf("generated always as (%s)", PQgetvalue(res, i, attrdef_col)); Nit: I would add VIRTUAL instead of relying on the default option. Another thing I was thinking about: could it be possible to add a sanity check in sanity_check.sql so as a column more that one field in attidentity, attgenerated and atthasdef set at the same time? -- Michael
signature.asc
Description: PGP signature