On Thu, Sep 5, 2024 at 9:40 AM Tender Wang <tndrw...@gmail.com> wrote: > 1. in setup_eager_aggregation(), before calling create_agg_clause_infos(), > it does > some checks if eager aggregation is available. Can we move those checks into > a function, > for example, can_eager_agg(), like can_partial_agg() does?
We can do this, but I'm not sure this would be better. > 2. I found that outside of joinrel.c we all use IS_DUMMY_REL, but in > joinrel.c, Tom always uses > is_dummy_rel(). Other commiters use IS_DUMMY_REL. They are essentially the same: IS_DUMMY_REL() is a macro that wraps is_dummy_rel(). I think they are interchangeable, and I don’t have a preference for which one is better. > 3. The attached patch does not consider FDW when creating a path for > grouped_rel or grouped_join. > Do we need to think about FDW? We may add support for foreign relations in the future, but for now, I think we'd better not expand the scope too much until we ensure that everything is working correctly. Thanks Richard
