Обсуждение: Is it worth adding Assert(false) for iso-8859-1 paths in print_path()?
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. Is the attached worthwhile? David [1] https://www.postgresql.org/message-id/379082d6-1b6a-4cd6-9ecf-7157d8c08635@postgrespro.ru
Вложения
On 2023-Sep-29, David Rowley wrote: > 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. > > Is the attached worthwhile? Hmm, if we had a buildfarm animal with OPTIMIZER_DEBUG turned on, then I agree it would catch the omission quickly. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "¿Cómo puedes confiar en algo que pagas y que no ves, y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
David Rowley <dgrowleyml@gmail.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
On Fri, 29 Sept 2023 at 03:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 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. Then perhaps we just need to open a thread with an appropriate subject to check if anyone finds it useful and if we don't get any response after some number of weeks, just remove it from master. David