public inbox for [email protected]
help / color / mirror / Atom feedFrom: Etsuro Fujita <[email protected]>
To: Amit Langote <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Rajkumar Raghuwanshi <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Date: Wed, 15 Aug 2018 20:35:55 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CA+TgmobR9XWEhmF3s_WsoLNnnjaFyS_xtbrVG3dCGiUgof20Nw@mail.gmail.com>
<CA+TgmoZSaKq-fYALn5jf6c_X3==Rb2s8eqLDwGpV=LNNhTXYwg@mail.gmail.com>
<[email protected]>
<CA+TgmoYYTCQL=TUYbUz2b=CsQeB4o1L2+_=zHynxGVmyKUtH1A@mail.gmail.com>
<[email protected]>
<CA+TgmoazFzdp5ZVwUjk_b2cuB90WCeeWtzt1ShSbXGyg67c+Zw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CA+TgmoZMdNiKUGV0=-QLV1f1-Xr0CDT+8y9wLktuEiQGkt64UQ@mail.gmail.com>
<CA+TgmoZjYHtkw-o_4=-GXb6obF1epyivr+WnZ1SQCnU6fgtf+w@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CA+TgmoYhgEkcR1E-BMS-2G8A4_5z0bbN6vCO+uR7HyQCOsWpEw@mail.gmail.com>
<[email protected]>
<CA+TgmoaNuWR-Krw9L2NOARNd7gJfHXwgOO8=8J4YAp+P4a1djA@mail.gmail.com>
<[email protected]>
<[email protected]>
(2018/08/15 13:04), Amit Langote wrote:
> On 2018/08/15 12:25, Etsuro Fujita wrote:
>> (2018/08/15 0:51), Robert Haas wrote:
>>> On Mon, Aug 13, 2018 at 12:32 PM, Etsuro Fujita
>>> <[email protected]> wrote:
>>>> One thing I noticed might be an improvement is to skip
>>>> build_joinrel_partition_info if the given joinrel will be to have
>>>> consider_partitionwise_join=false; in the previous patch, that function
>>>> created the joinrel's partition info such as part_scheme and part_rels if
>>>> the joinrel is considered as partitioned, independently of the flag
>>>> consider_partitionwise_join for it, but if that flag is false, we don't
>>>> generate PWJ paths for the joinrel, so we would not need to create that
>>>> partition info at all. This would not only avoid unnecessary
>>>> processing in
>>>> that function, but also make unnecessary the changes I made to
>>>> try_partitionwise_join, generate_partitionwise_join_paths,
>>>> apply_scanjoin_target_to_paths, and create_ordinary_grouping_paths. So I
>>>> updated the patch that way. Please find attached an updated version of
>>>> the
>>>> patch.
>>>
>>> I guess the question is whether there are (or might be in the future)
>>> other dependencies on part_scheme. For example, it looks like
>>> partition pruning uses it. I'm not sure whether partition pruning
>>> supports a plan like:
>>>
>>> Append
>>> -> Nested Loop
>>> -> Seq Scan on p1
>>> -> Index Scan on q1
>>> <repeat the above for p2/q2 etc.>
>>
>> I'm not sure that either, but if a join relation doesn't have part_scheme
>> set, it means that that relation is considered as non-partitioned, as in
>> the case when enable_partitionwise_join is off, so there would be no PWJ
>> paths generated for it, to begin with. So in that case, ISTM that we
>> don't need to worry about that at least for partition pruning.
>
> Fwiw, partition pruning works only for base rels, which applies to both
> planning-time pruning (pruning is performed only during base rel size
> estimation) and run-time pruning (we'll add pruning info to the Append
> plan only if the source AppendPath's parent rel is a base rel).
Thanks for that, Amit!
I looked into the question for the join or upper rel case, but couldn't
find any places in the PG11 code where we assume that a join or upper
rel has non-NULL part_scheme, as described below:
* Both try_partitionwise_join() and generate_partitionwise_join_paths()
check whether a join rel to be considered has non-NULL part_scheme,
through the IS_PARTITIONED_REL macro:
#define IS_PARTITIONED_REL(rel) \
((rel)->part_scheme && (rel)->boundinfo && (rel)->nparts > 0 && \
(rel)->part_rels && !(IS_DUMMY_REL(rel)))
If IS_PARTITIONED_REL, the former adds paths to child-joins, and the
latter builds PWJ paths; but both don't assume non-NULL part_scheme.
* apply_scanjoin_target_to_paths() checks whether the topmost scan/join
rel has non-NULL part_scheme, and if so, it recursively adjusts all
partitions; it doesn't assume non-NULL part_scheme.
* create_ordinary_grouping_paths() also checks whether the topmost
scan/join rel adjusted by apply_scanjoin_target_to_paths() has non-NULL
part_scheme, and if so, it considers PWA paths; it doesn't assume
non-NULL part_scheme either.
* add_paths_to_append_rel(), which is called from each of the above (ie,
generate_partitionwise_join_paths(), apply_scanjoin_target_to_paths(),
and create_partitionwise_grouping_paths() in
create_ordinary_grouping_paths() for creating PWJ paths, adjusting PWJ
paths, and creating PWA paths respectively) also checks whether a rel to
be considered has non-NULL part_scheme, but never assumes that. And
actually, if the rel's part_scheme is NULL, add_paths_to_append_rel()
wouldn't be called for the rel, because of the reason described above.
Thanks for the comments, Robert!
Best regards,
Etsuro Fujita
view thread (115+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox