Re: Is it worth adding Assert(false) for unknown paths in print_path()?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
Subject: Re: Is it worth adding Assert(false) for unknown paths in print_path()?
Date: 2023-09-28 14:23:43
Message-ID: 951421.1695911023@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> In [1] Andrey highlighted that I'd forgotten to add print_path()
> handling for TidRangePaths in bb437f995.

> I know the OPTIMIZER_DEBUG code isn't exactly well used. I never
> personally use it and I work quite a bit in the planner, however, if
> we're keeping it, I thought maybe we might get the memo of missing
> paths a bit sooner if we add an Assert(false) in the default cases.

FWIW, I'd argue for dropping print_path rather than continuing to
maintain it. I never use it, finding pprint() to serve the need
better and more reliably. However, assuming that we keep it ...

> Is the attached worthwhile?

... I think this is actually counterproductive. It will certainly
not help draw the notice of anyone who wouldn't otherwise pay
attention to print_path. Also, observe the extremely longstanding
policy decision in outNode's default: case:

/*
* This should be an ERROR, but it's too useful to be able to
* dump structures that outNode only understands part of.
*/
elog(WARNING, "could not dump unrecognized node type: %d",
(int) nodeTag(obj));
break;

The same argument applies to print_path, I should think.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-09-28 14:28:59 Re: Allow deleting enumerated values from an existing enumerated data type
Previous Message Kuwamura Masaki 2023-09-28 14:17:46 Re: Clarify where the severity level is defined