I find an easier way as follows: diff --git a/contrib/pg_plan_advice/pgpa_trove.c b/contrib/pg_plan_advice/pgpa_trove.c index ca69f3bd3df..64af4b1435b 100644 --- a/contrib/pg_plan_advice/pgpa_trove.c +++ b/contrib/pg_plan_advice/pgpa_trove.c @@ -179,7 +179,6 @@ pgpa_build_trove(List *advice_items) * but in the future this might not be true, e.g. a custom * scan could replace a join. */ - Assert(target->ttype == PGPA_TARGET_IDENTIFIER); pgpa_trove_add_to_slice(&trove->scan,
item->tag, target); }
Thanks for checking this.
I agree that removing the assertion looks like the better approach.
Keeping the original target tree seems preferable. As you noted, pgpa_identifier_matches_target() already handles non-identifier targets by checking their descendants. pgpa_trove_add_to_hash() does the same when building the lookup table, so a grouped target such as ((a)) should still be indexed and matched through its child identifier while preserving the original shape for output.
So I think the assertion in pgpa_build_trove() is too strict, and the nearby comment should be adjusted to avoid saying/implying that scan advice always has a direct identifier target.
