On 05.09.24 10:27, jian he wrote:
On Wed, Sep 4, 2024 at 4:40 PM Peter Eisentraut <pe...@eisentraut.org> wrote:
Here is an implementation of this. It's much nicer! It also appears to
fix all the additional test cases that have been presented. (I haven't
integrated them into the patch set yet.)
I left the 0001 patch alone for now and put the new rewriting
implementation into 0002. (Unfortunately, the diff is kind of useless
for visual inspection.) Let me know if this matches what you had in
mind, please. Also, is this the right place in fireRIRrules()?
hi. some minor issues.
in get_dependent_generated_columns we can
/* skip if not generated column */
if (!TupleDescAttr(tupdesc, defval->adnum - 1)->attgenerated)
continue;
change to
/* skip if not generated stored column */
if (!(TupleDescAttr(tupdesc, defval->adnum -
1)->attgenerated == ATTRIBUTE_GENERATED_STORED))
continue;
I need to study more what to do with this function. I'm not completely
sure whether this should apply only to stored generated columns.
in ExecInitStoredGenerated
"if ((tupdesc->constr && tupdesc->constr->has_generated_stored)))"
is true.
then later we finish the loop
(for (int i = 0; i < natts; i++) loop)
we can "Assert(ri_NumGeneratedNeeded > 0)"
so we can ensure once has_generated_stored flag is true,
then we should have at least one stored generated attribute.
This is technically correct, but this code isn't touched by this patch,
so I don't think it belongs here.
similarly, in expand_generated_columns_internal
we can aslo add "Assert(list_length(tlist) > 0);"
above
node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
REPLACEVARS_CHANGE_VARNO, rt_index, NULL);
Ok, I'll add that.
@@ -2290,7 +2291,9 @@ ExecBuildSlotValueDescription(Oid reloid,
if (table_perm || column_perm)
{
- if (slot->tts_isnull[i])
+ if (att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ val = "virtual";
+ else if (slot->tts_isnull[i])
val = "null";
else
{
Oid foutoid;
bool typisvarlena;
getTypeOutputInfo(att->atttypid, &foutoid, &typisvarlena);
val = OidOutputFunctionCall(foutoid, slot->tts_values[i]);
}
we can add Assert here, if i understand it correctly, like
if (att->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
{
Assert(slot->tts_isnull[i]);
val = "virtual";
}
Also technically correct, but I don't see what benefit this would bring.
The code guarded by that assert would not make use of the thing being
asserted.
