On 6/28/24 13:16, Tatsuro Yamada wrote:
> Hi Tomas and All,
>
> Attached file is a new patch including:
> 6) Add stats option to explain command
> 7) The patch really needs some docs (partly)
>
> >4) Add new node (resolve errors in cfbot and prepared statement)
>
> I tried adding a new node in pathnode.h, but it doesn't work well.
> So, it needs more time to implement it successfully because this is
> the first time to add a new node in it.
>
I'm not sure why it didn't work well, and I haven't tried adding the
struct myself so I might be missing something important, but m
assumption was the new struct would go to plannodes.h. The planning
works in phases:
parse -> build Path nodes -> pick cheapest Path -> create Plan
and it's the Plan that is printed by EXPLAIN. The pathnodes.h and
plannodes.h match this, so if it's expected to be in Plan it should go
to plannodes.h I think.
>
>> 8) Add regression test (stats_ext.sql)
>
>
> Actually, I am not yet able to add new test cases to stats_ext.sql.
Why is that not possible? Can you explain?
> Instead, I created a simple test (test.sql) and have attached it.
> Also, output.txt is the test result.
>
> To add new test cases to stats_ext.sql,
> I'd like to decide on a strategy for modifying it. In particular, there are
> 381 places where the check_estimated_rows function is used, so should I
> include the same number of tests, or should we include the bare minimum
> of tests that cover the code path? I think only the latter would be fine.
> Any advice is appreciated. :-D
>
I don't understand. My suggestion was to create a new function, similar
to check_estimated_rows(), that's get a query, do EXPLAIN and extract
the list of applied statistics. Similar to what check_estimated_rows()
does for number of rows.
I did not mean to suggest you modify check_estimated_rows() to extract
both the number of rows and statistics, nor to modify the existing tests
(that's not very useful, because there's only one extended statistics in
each of those tests, and by testing the estimate we implicitly test that
it's applied).
My suggestion is to add a couple new queries, with multiple statistics
and multiple clauses etc. And then test the patch on those. You could do
simple EXPLAIN (COSTS OFF), or add the new function to make it a bit
more stable (but maybe it's not worth it).
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company