From 2c1461be72f7d5fc4a0c89affc515f399926329e Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 4 Sep 2026 11:57:33 -0400 Subject: [PATCH v3 4/5] Change GEQO fitness comparisons to consider disabled_nodes. Prior to v18, the total cost of a plan was just one number, a floating-point value. Commit e22253467942fdb100087787c3e1e3a8620c54b2 introduced a second component, the number of disabled_nodes at or below that level of the plan tree. Unfortunately, I (rhaas) failed to realize that GEQO extracts the cost from each candidate plan and compares it directly as a measure of fitness. This means that, at least in principle, it's possible for GEQO to prefer a plan with more disabled_nodes and lower cost over one with fewer disabled_nodes and higher cost, which is incorrect. It may be possible for that to become a practical issue in v18, but it's much more likely to become an issue in v19 due to pg_plan_advice. Hence, since the straightforward fix breaks ABI compatibility, back-patch only to v19. --- src/backend/optimizer/geqo/geqo_eval.c | 18 +++++++----- src/backend/optimizer/geqo/geqo_main.c | 13 +++++---- src/backend/optimizer/geqo/geqo_misc.c | 19 ++++++++----- src/backend/optimizer/geqo/geqo_pool.c | 29 ++++++++----------- src/include/optimizer/geqo.h | 2 +- src/include/optimizer/geqo_gene.h | 39 +++++++++++++++++++++++++- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 82 insertions(+), 39 deletions(-) diff --git a/src/backend/optimizer/geqo/geqo_eval.c b/src/backend/optimizer/geqo/geqo_eval.c index 3061d68e19d..d7851faa4d5 100644 --- a/src/backend/optimizer/geqo/geqo_eval.c +++ b/src/backend/optimizer/geqo/geqo_eval.c @@ -48,18 +48,18 @@ static bool desirable_join(PlannerInfo *root, /* * geqo_eval * - * Returns cost of a query tree as an individual of the population. + * Returns the fitness of a query tree as an individual of the population. * - * If no legal join order can be extracted from the proposed tour, - * returns DBL_MAX. + * If no legal join order can be extracted from the proposed tour, returns + * the invalid fitness described in geqo_gene.h. */ -Cost +Fitness geqo_eval(PlannerInfo *root, Gene *tour, int num_gene) { MemoryContext mycontext; MemoryContext oldcxt; RelOptInfo *joinrel; - Cost fitness; + Fitness fitness; int savelength; struct HTAB *savehash; @@ -112,10 +112,14 @@ geqo_eval(PlannerInfo *root, Gene *tour, int num_gene) { Path *best_path = joinrel->cheapest_total_path; - fitness = best_path->total_cost; + fitness.disabled_nodes = best_path->disabled_nodes; + fitness.cost = best_path->total_cost; } else - fitness = DBL_MAX; + { + fitness.disabled_nodes = INT_MAX; + fitness.cost = DBL_MAX; + } /* * Restore join_rel_list to its former state, and put back original diff --git a/src/backend/optimizer/geqo/geqo_main.c b/src/backend/optimizer/geqo/geqo_main.c index 5d9b65cc48a..1b4a52d4c0f 100644 --- a/src/backend/optimizer/geqo/geqo_main.c +++ b/src/backend/optimizer/geqo/geqo_main.c @@ -133,10 +133,12 @@ geqo(PlannerInfo *root, int number_of_rels, List *initial_rels) * future (-> geqo_pool.c:spread_chromo ) */ #ifdef GEQO_DEBUG - elog(DEBUG1, "GEQO selected %d pool entries, best %.2f, worst %.2f", + elog(DEBUG1, "GEQO selected %d pool entries, best %.2f<%d>, worst %.2f<%d>", pool_size, - pool->data[0].worth, - pool->data[pool_size - 1].worth); + pool->data[0].worth.cost, + pool->data[0].worth.disabled_nodes, + pool->data[pool_size - 1].worth.cost, + pool->data[pool_size - 1].worth.disabled_nodes); #endif /* allocate chromosome momma and daddy memory */ @@ -267,8 +269,9 @@ geqo(PlannerInfo *root, int number_of_rels, List *initial_rels) #endif #ifdef GEQO_DEBUG - elog(DEBUG1, "GEQO best is %.2f after %d generations", - pool->data[0].worth, number_generations); + elog(DEBUG1, "GEQO best is %.2f<%d> after %d generations", + pool->data[0].worth.cost, pool->data[0].worth.disabled_nodes, + number_generations); #endif diff --git a/src/backend/optimizer/geqo/geqo_misc.c b/src/backend/optimizer/geqo/geqo_misc.c index 2fb59d6e899..768e9e5a301 100644 --- a/src/backend/optimizer/geqo/geqo_misc.c +++ b/src/backend/optimizer/geqo/geqo_misc.c @@ -44,10 +44,11 @@ avg_pool(Pool *pool) * Since the pool may contain multiple occurrences of DBL_MAX, divide by * pool->size before summing, not after, to avoid overflow. This loses a * little in speed and accuracy, but this routine is only used for debug - * printouts, so we don't care that much. + * printouts, so we don't care that much. For the same reason, we simply + * ignore disabled_nodes here. */ for (i = 0; i < pool->size; i++) - cumulative += pool->data[i].worth / pool->size; + cumulative += pool->data[i].worth.cost / pool->size; return cumulative; } @@ -79,7 +80,8 @@ print_pool(FILE *fp, Pool *pool, int start, int stop) fprintf(fp, "%d)\t", i); for (j = 0; j < pool->string_length; j++) fprintf(fp, "%d ", pool->data[i].string[j]); - fprintf(fp, "%g\n", pool->data[i].worth); + fprintf(fp, "%g<%d>\n", pool->data[i].worth.cost, + pool->data[i].worth.disabled_nodes); } fflush(fp); @@ -100,11 +102,14 @@ print_gen(FILE *fp, Pool *pool, int generation) lowest = pool->size > 1 ? pool->size - 2 : 0; fprintf(fp, - "%5d | Best: %g Worst: %g Mean: %g Avg: %g\n", + "%5d | Best: %g<%d> Worst: %g<%d> Mean: %g<%d> Avg: %g\n", generation, - pool->data[0].worth, - pool->data[lowest].worth, - pool->data[pool->size / 2].worth, + pool->data[0].worth.cost, + pool->data[0].worth.disabled_nodes, + pool->data[lowest].worth.cost, + pool->data[lowest].worth.disabled_nodes, + pool->data[pool->size / 2].worth.cost, + pool->data[pool->size / 2].worth.disabled_nodes, avg_pool(pool)); fflush(fp); diff --git a/src/backend/optimizer/geqo/geqo_pool.c b/src/backend/optimizer/geqo/geqo_pool.c index b0a8373628d..f1443bddd71 100644 --- a/src/backend/optimizer/geqo/geqo_pool.c +++ b/src/backend/optimizer/geqo/geqo_pool.c @@ -24,9 +24,6 @@ #include "postgres.h" -#include -#include - #include "optimizer/geqo_copy.h" #include "optimizer/geqo_pool.h" #include "optimizer/geqo_recombination.h" @@ -95,8 +92,9 @@ random_init_pool(PlannerInfo *root, Pool *pool) int bad = 0; /* - * We immediately discard any invalid individuals (those that geqo_eval - * returns DBL_MAX for), thereby not wasting pool space on them. + * We immediately discard any invalid individuals (those for which + * geqo_eval returns an invalid fitness), thereby not wasting pool space + * on them. * * If we fail to make any valid individuals after 10000 tries, give up; * this probably means something is broken, and we shouldn't just let @@ -108,7 +106,7 @@ random_init_pool(PlannerInfo *root, Pool *pool) init_tour(root, chromo[i].string, pool->string_length); pool->data[i].worth = geqo_eval(root, chromo[i].string, pool->string_length); - if (pool->data[i].worth < DBL_MAX) + if (fitness_is_valid(pool->data[i].worth)) i++; else { @@ -147,12 +145,7 @@ compare(const void *arg1, const void *arg2) const Chromosome *chromo1 = (const Chromosome *) arg1; const Chromosome *chromo2 = (const Chromosome *) arg2; - if (chromo1->worth == chromo2->worth) - return 0; - else if (chromo1->worth > chromo2->worth) - return 1; - else - return -1; + return fitness_compare(chromo1->worth, chromo2->worth); } /* @@ -198,7 +191,7 @@ spread_chromo(PlannerInfo *root, Chromosome *chromo, Pool *pool) tmp_chromo; /* new chromo is so bad we can't use it */ - if (chromo->worth > pool->data[pool->size - 1].worth) + if (fitness_compare(chromo->worth, pool->data[pool->size - 1].worth) > 0) return; /* do a binary search to find the index of the new chromo */ @@ -212,11 +205,11 @@ spread_chromo(PlannerInfo *root, Chromosome *chromo, Pool *pool) { /* these 4 cases find a new location */ - if (chromo->worth <= pool->data[top].worth) + if (fitness_compare(chromo->worth, pool->data[top].worth) <= 0) index = top; - else if (chromo->worth == pool->data[mid].worth) + else if (fitness_compare(chromo->worth, pool->data[mid].worth) == 0) index = mid; - else if (chromo->worth == pool->data[bot].worth) + else if (fitness_compare(chromo->worth, pool->data[bot].worth) == 0) index = bot; else if (bot - top <= 1) index = bot; @@ -227,13 +220,13 @@ spread_chromo(PlannerInfo *root, Chromosome *chromo, Pool *pool) * yet been found. */ - else if (chromo->worth < pool->data[mid].worth) + else if (fitness_compare(chromo->worth, pool->data[mid].worth) < 0) { bot = mid; mid = top + ((bot - top) / 2); } else - { /* (chromo->worth > pool->data[mid].worth) */ + { /* chromo is worse than pool->data[mid] */ top = mid; mid = top + ((bot - top) / 2); } diff --git a/src/include/optimizer/geqo.h b/src/include/optimizer/geqo.h index 59d1964a748..f8189a1bc60 100644 --- a/src/include/optimizer/geqo.h +++ b/src/include/optimizer/geqo.h @@ -95,7 +95,7 @@ extern RelOptInfo *geqo(PlannerInfo *root, int number_of_rels, List *initial_rels); /* routines in geqo_eval.c */ -extern Cost geqo_eval(PlannerInfo *root, Gene *tour, int num_gene); +extern Fitness geqo_eval(PlannerInfo *root, Gene *tour, int num_gene); extern RelOptInfo *gimme_tree(PlannerInfo *root, Gene *tour, int num_gene); #endif /* GEQO_H */ diff --git a/src/include/optimizer/geqo_gene.h b/src/include/optimizer/geqo_gene.h index 6001bd2d06e..f9dda8719ac 100644 --- a/src/include/optimizer/geqo_gene.h +++ b/src/include/optimizer/geqo_gene.h @@ -24,6 +24,9 @@ #ifndef GEQO_GENE_H #define GEQO_GENE_H +#include +#include + #include "nodes/nodes.h" /* @@ -32,10 +35,44 @@ */ typedef int Gene; +/* + * Fitness of a candidate join order. + * + * A tour for which no valid plan could be constructed is represented by + * disabled_nodes = INT_MAX and cost = DBL_MAX. + */ +typedef struct Fitness +{ + int disabled_nodes; + Cost cost; +} Fitness; + +/* + * Is this a valid tour? + */ +static inline bool +fitness_is_valid(Fitness fitness) +{ + return fitness.disabled_nodes < INT_MAX || fitness.cost < DBL_MAX; +} + +/* + * Which fitness is better? + */ +static inline int +fitness_compare(Fitness fitness1, Fitness fitness2) +{ + if (fitness1.disabled_nodes != fitness2.disabled_nodes) + return fitness1.disabled_nodes < fitness2.disabled_nodes ? -1 : 1; + if (fitness1.cost != fitness2.cost) + return fitness1.cost < fitness2.cost ? -1 : 1; + return 0; +} + typedef struct Chromosome { Gene *string; - Cost worth; + Fitness worth; } Chromosome; typedef struct Pool diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index fb3ee1b071b..98559e3e005 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -872,6 +872,7 @@ First FixedParallelExecutorState FixedParallelState FixedParamState +Fitness FlagMode Float FlushPosition -- 2.51.0