From da0fe2de2aa98c518686f337bb06ff0cb4400ed3 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 13 Nov 2022 15:24:31 +0100 Subject: [PATCH v21 2/6] review comments --- src/backend/optimizer/util/relnode.c | 11 +++++++++++ src/include/nodes/pathnodes.h | 3 +++ 2 files changed, 14 insertions(+) diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 94720865f47..d4367ba14a5 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -382,6 +382,12 @@ find_base_rel(PlannerInfo *root, int relid) /* * build_rel_hash * Construct the auxiliary hash table for relation specific data. + * + * XXX Why is this renamed, leaving out the "join" part? Are we going to use + * it for other purposes? + * + * XXX Also, why change the API and not pass PlannerInfo? Seems pretty usual + * for planner functions. */ static void build_rel_hash(RelInfoList *list) @@ -422,6 +428,11 @@ build_rel_hash(RelInfoList *list) /* * find_rel_info * Find a base or join relation entry. + * + * XXX Why change the API and not pass PlannerInfo? Seems pretty usual + * for planner functions. + * + * XXX I don't understand why we need both this and find_join_rel. */ static void * find_rel_info(RelInfoList *list, Relids relids) diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 0ca7d5ab51e..018ce755720 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -88,6 +88,9 @@ typedef enum UpperRelationKind * present and valid when rel_hash is not NULL. Note that we still maintain * the list even when using the hash table for lookups; this simplifies life * for GEQO. + * + * XXX I wonder why we actually need a separate node, merely wrapping fields + * that already existed ... */ typedef struct RelInfoList { -- 2.38.1