On Fri, 29 Mar 2024 at 08:53, Tom Lane wrote:
> On third thought, I'm not at all convinced that we even want to
> invent this struct as compared to just adding another parameter
> to subquery_planner. The problem with a struct is what happens
> the next time we need to add a parameter? If we add
I wrote:
> David Rowley writes:
>> Maybe something with "Parameters" in the name?
> SubqueryParameters might be OK. Or SubqueryPlannerExtra?
> Since this is a bespoke struct that will probably only ever
> be used with subquery_planner, naming it after that function
> seems like a good idea.
On
David Rowley writes:
> The problem is informing the UNION child query about what it is. I
> thought I could do root->parent_root->parse->setOperations for a UNION
> child to know what it is, but that breaks for a query such as:
Yeah, having grouping_planner poke into the parent level
doesn't see
On Thu, 28 Mar 2024 at 15:56, Tom Lane wrote:
> I haven't studied the underlying problem yet, so I'm not quite
> buying into whether we need this struct at all ...
The problem is, when planning a UNION child query, we want to try and
produce some Paths that would suit the top-level UNION query so
Richard Guo writes:
> On Wed, Mar 27, 2024 at 6:34 PM David Rowley wrote:
>> The attached is roughly what I had in mind. I've not taken the time
>> to see what comments need to be updated, so the attached aims only to
>> assist discussion.
> I like this idea.
I haven't studied the underlying p
On Wed, Mar 27, 2024 at 6:34 PM David Rowley wrote:
> On Wed, 27 Mar 2024 at 22:47, David Rowley wrote:
> > I did wonder when first working on this patch if subquery_planner()
> > should grow an extra parameter, or maybe consolidate some existing
> > ones by passing some struct that provides the
On Wed, 27 Mar 2024 at 22:47, David Rowley wrote:
> I did wonder when first working on this patch if subquery_planner()
> should grow an extra parameter, or maybe consolidate some existing
> ones by passing some struct that provides the planner with a bit more
> context about the query. A few of
On Wed, 27 Mar 2024 at 16:15, Richard Guo wrote:
>if (root->parent_root != NULL &&
>root->parent_root->parse->setOperations != NULL &&
>IsA(root->parent_root->parse->setOperations, SetOperationStmt))
>qp_extra.setop =
>(SetOperationStmt *) root->parent_root-
On Wed, Mar 27, 2024 at 6:23 AM David Rowley wrote:
> Because this field is set, it plans the CTE thinking it's a UNION
> child and breaks when it can't find a SortGroupClause for the CTE's
> target list item.
Right. The problem here is that we mistakenly think that the CTE query
is a subquery
On Wed, 27 Mar 2024 at 06:00, Alexander Lakhin wrote:
> SELECT count(*) FROM (
>WITH q1(x) AS (SELECT 1)
>SELECT FROM q1 UNION SELECT FROM q1
> ) qu;
>
> TRAP: failed Assert("lg != NULL"), File: "planner.c", Line: 7941, PID: 1133017
Thanks for finding that.
There's something weird going
Hello David,
25.03.2024 04:43, David Rowley wrote:
I didn't see that as a reason not to push this patch as this occurs
both with and without this change, so I've now pushed this patch.
Please look at a new assertion failure, that is triggered by the following
query:
SELECT count(*) FROM (
WI
On Mon, Mar 25, 2024 at 9:44 AM David Rowley wrote:
> It seems ok that
> the ec_indexes are not set for the top-level set RelOptInfo as
> get_eclass_for_sort_expr() does not make use of ec_indexes while
> searching for an existing EquivalenceClass. Maybe we should fix this
> varno == 0 hack and
On Tue, 12 Mar 2024 at 23:21, David Rowley wrote:
> I've attached v3.
I spent quite a bit more time looking at this.
I discovered that the dNumGroups wasn't being set as it should have
been for INTERSECT and EXCEPT queries. There was a plan change as a
result of this. I've fixed this by adjust
On Mon, 11 Mar 2024 at 19:56, Richard Guo wrote:
> * There are cases where the setop_pathkeys of a subquery does not match
> the union_pathkeys generated in generate_union_paths() for sorting by
> the whole target list. In such cases, even if we have explicitly sorted
> the paths of the subquery
On Fri, Mar 8, 2024 at 11:31 AM David Rowley wrote:
> On Fri, 8 Mar 2024 at 00:39, Richard Guo wrote:
> > I would like to have another look, but it might take several days.
> > Would that be too late?
>
> Please look. Several days is fine. I'd like to start looking on Monday
> or Tuesday next we
On Fri, 8 Mar 2024 at 00:39, Richard Guo wrote:
> I would like to have another look, but it might take several days.
> Would that be too late?
Please look. Several days is fine. I'd like to start looking on Monday
or Tuesday next week.
Thanks
David
On Thu, Mar 7, 2024 at 7:16 PM David Rowley wrote:
> On Thu, 15 Feb 2024 at 17:30, David Rowley wrote:
> >
> > On Tue, 6 Feb 2024 at 22:05, Richard Guo wrote:
> > > I'm thinking that maybe it'd be better to move the work of sorting the
> > > subquery's paths to the outer query level, specifical
On Thu, 15 Feb 2024 at 17:30, David Rowley wrote:
>
> On Tue, 6 Feb 2024 at 22:05, Richard Guo wrote:
> > I'm thinking that maybe it'd be better to move the work of sorting the
> > subquery's paths to the outer query level, specifically within the
> > build_setop_child_paths() function, just befo
David Rowley writes:
>>
>> The two if-clauses looks unnecessary, it should be handled by
>> pathkeys_count_contained_in already. The same issue exists in
>> pathkeys_useful_for_ordering as well. Attached patch fix it in master.
>
> I agree. I'd rather not have those redundant checks in
> pathk
On Wed, 7 Feb 2024 at 12:05, Andy Fan wrote:
> +static int
> +pathkeys_useful_for_setop(PlannerInfo *root, List *pathkeys)
> +{
> + int n_common_pathkeys;
> +
> + if (root->setop_pathkeys == NIL)
> + return 0; /* no specia
On Tue, 6 Feb 2024 at 22:05, Richard Guo wrote:
> I'm thinking that maybe it'd be better to move the work of sorting the
> subquery's paths to the outer query level, specifically within the
> build_setop_child_paths() function, just before we stick SubqueryScanPath
> on top of the subquery's paths
Hi,
> * I think we should update truncate_useless_pathkeys() to account for
> the ordering requested by the query's set operation;
Nice catch.
> I'm thinking that maybe it'd be better to move the work of sorting the
> subquery's paths to the outer query level, specifically within the
> build_se
On Fri, Nov 24, 2023 at 6:29 AM David Rowley wrote:
> I've attached the updated patch. This one is probably ready for
> someone to test out. There will be more work to do, however.
I just started reviewing this patch and haven't looked through all the
details yet. Here are some feedbacks that
On Fri, 24 Nov 2023 at 18:43, Ashutosh Bapat
wrote:
>
> On Fri, Nov 24, 2023 at 3:59 AM David Rowley wrote:
> > For now, as a temporary fix, I've just #ifdef'd out the code in
> > add_path() that's pfreeing the old path. I have drafted a patch that
> > refcounts Paths, but I'm unsure if that's t
On Fri, Nov 24, 2023 at 3:59 AM David Rowley wrote:
>
> Another problem I hit was add_path() pfreeing a Path that I needed.
> This was happening due to how I'm building the final paths in the
> subquery when setop_pathkeys are set. Because I want to always
> include the cheapest_input_path to all
On Thu, 2 Nov 2023 at 12:42, David Rowley wrote:
> One part that still needs work is the EquivalanceClass building.
> Because we only build the final targetlist for the Append after
> planning all the append child queries, I ended up having to populate
> the EquivalanceClasses backwards, i.e child
The upper planner was pathified many years ago now. That was a large
chunk of work and because of that, the union planner was not properly
pathified in that effort. A small note was left in
recurse_set_operations() to mention about future work.
You can see this lack of pathification in make_unio
27 matches
Mail list logo
