Re: BUG #19763: ManualBug2 NULL pointer dereference in make_build_data via examine_expression bypass of lookup_var_a

On Sun, Oct 18, 2026 at 10:43:03PM -0400, shihao zhong wrote:
> Here is a patch. It does for examine_expression() what 3b7a6fa1572
> did for examine_attribute().
>
> The test adds your typanalyze function to test_custom_types. It runs
> your repro and also a single expression object, which builds only
> expression statistics.

Thanks for the patch. A couple of comments as I have played with this
area in the recent past. I didn't think that this would be reachable
in practice, but well, that's a fancy typanalyze callback that you
have here.

+ /*
+ * The typanalyze function sees a different attstattarget here than in
+ * lookup_var_attr_stats(), so it may reject the expression now.
+ */
+ if (result->stats[idx] == NULL)
+ {
+ pfree(result);
+ return NULL;
+ }

I don't understand why this comment is adapted here; it just makes the
situation more confusing to me. This just points to the fact that
examine_expression() could return NULL depending on what the
typanalyze callback decides to return. What your patch does it fancy
enough in the callback by not having the function to be called twice.
Nothing like that would happen in practice, still I agree that the
backend should have more protections. I think that we should just
ditch this comment. It's plain enough that examine_expression() can
return NULL.

/* evaluate expressions (if the statistics object has any) */
data = make_build_data(onerel, stat, numrows, rows, stats, stattarget);
+ if (!data)
+ {
+ if (!IsAutoVacuumWorkerProcess())
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("statistics object \"%s.%s\" could not be computed for relation \"%s.%s\"",
+ stat->schema, stat->name,
+ get_namespace_name(onerel->rd_rel->relnamespace),
+ RelationGetRelationName(onerel)),
+ errtable(onerel)));
+ continue;
+ }

This warning is a duplicate, FWIW. Not optimal, just feels slightly
annoying. The patch is OK with this duplication, I guess..

If we begin to care about examine_expression(), I think that we ought
to also care about build_expr_data(), its second caller, no? It looks
like this could also be reached with a fancy undeterministic
typanalyze callback. I have not tested this hypothesis, but it would
be nice to make all the callers examine_expression() consistent.

+-- Same with a single expression, building only expression statistics.
+CREATE STATISTICS test_stats ON (func_int_custom(data)) FROM test_table;
+ANALYZE test_table;
+WARNING: statistics object "public.test_stats" could not be computed for relation "public.test_table"

This test is a duplicate of the first one. Let's remove it; it brings
nothing new.
--
Michael