On 09-05-2018 17:30, Alvaro Herrera wrote:
Marina Polyakova wrote:
Hello everyone in this thread!
I got a similar server crash as in [1] on the master branch since the
commit
9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails
because
the second argument ScalarArrayOpExpr is no
On 2018/05/09 22:43, Alvaro Herrera wrote:
> Amit Langote wrote:
>> On 2018/05/09 11:31, David Rowley wrote:
>>> On 9 May 2018 at 14:29, Amit Langote wrote:
On 2018/05/09 11:20, Michael Paquier wrote:
> While looking at this code, is there any reason to not make
> gen_partprune_steps
On Wed, May 09, 2018 at 10:39:07AM -0300, Alvaro Herrera wrote:
> Michael Paquier wrote:
>> This removes a useless default clause in partprune.c and it got
>> forgotten in the crowd. Just attaching it again here, and it can just
>> be applied on top of the rest.
>
> Done, thanks for insisting.
T
Marina Polyakova wrote:
> Hello everyone in this thread!
> I got a similar server crash as in [1] on the master branch since the commit
> 9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails because
> the second argument ScalarArrayOpExpr is not a Const or an ArrayExpr, but is
> an Ar
Amit Langote wrote:
> On 2018/05/09 11:31, David Rowley wrote:
> > On 9 May 2018 at 14:29, Amit Langote wrote:
> >> On 2018/05/09 11:20, Michael Paquier wrote:
> >>> While looking at this code, is there any reason to not make
> >>> gen_partprune_steps static? This is only used in partprune.c for
Michael Paquier wrote:
> Alvaro, could it be possible to consider as well the patch I posted
> here?
> https://www.postgresql.org/message-id/20180424012042.gd1...@paquier.xyz
>
> This removes a useless default clause in partprune.c and it got
> forgotten in the crowd. Just attaching it again her
On Wed, May 09, 2018 at 02:01:26PM +0900, Amit Langote wrote:
> On 2018/05/09 11:31, David Rowley wrote:
>> On 9 May 2018 at 14:29, Amit Langote wrote:
>>> On 2018/05/09 11:20, Michael Paquier wrote:
While looking at this code, is there any reason to not make
gen_partprune_steps static?
On 2018/05/09 11:31, David Rowley wrote:
> On 9 May 2018 at 14:29, Amit Langote wrote:
>> On 2018/05/09 11:20, Michael Paquier wrote:
>>> While looking at this code, is there any reason to not make
>>> gen_partprune_steps static? This is only used in partprune.c for now,
>>> so the intention is t
On 9 May 2018 at 14:29, Amit Langote wrote:
> On 2018/05/09 11:20, Michael Paquier wrote:
>> While looking at this code, is there any reason to not make
>> gen_partprune_steps static? This is only used in partprune.c for now,
>> so the intention is to make it available for future patches?
>
> Yea
On 2018/05/09 11:20, Michael Paquier wrote:
> While looking at this code, is there any reason to not make
> gen_partprune_steps static? This is only used in partprune.c for now,
> so the intention is to make it available for future patches?
Yeah, making it static might be a good idea. I had made
On Tue, May 08, 2018 at 07:05:46PM -0300, Alvaro Herrera wrote:
> The reason for this crash is that gen_partprune_steps_internal() is
> unable to generate any steps for the clause -- which is natural, since
> the operator is not in a btree opclass. There are various callers
> of gen_partprune_step
Hi.
On 2018/05/09 7:05, Alvaro Herrera wrote:
> So I found that this query also crashed (using your rig),
>
> create table coercepart (a varchar) partition by list (a);
> create table coercepart_ab partition of coercepart for values in ('ab');
> create table coercepart_bc partition of coercepart
So I found that this query also crashed (using your rig),
create table coercepart (a varchar) partition by list (a);
create table coercepart_ab partition of coercepart for values in ('ab');
create table coercepart_bc partition of coercepart for values in ('bc');
create table coercepart_cd partitio
Michael Paquier wrote:
> So the problem appears when an expression needs to use
> COERCION_PATH_ARRAYCOERCE for a type coercion from one type to another,
> which requires an execution state to be able to build the list of
> elements. The clause matching happens at planning state, so while there
>
On Tue, May 08, 2018 at 04:07:41PM +0900, Amit Langote wrote:
> I have to agree to go with this conservative approach for now. Although
> we might be able to evaluate the array elements by applying the coercion
> specified by ArrayCoerceExpr, let's save that as an improvement to be
> pursued later
Thank you Marina for the report and Michael for following up.
On 2018/05/07 16:56, Michael Paquier wrote:
> On Mon, May 07, 2018 at 10:37:10AM +0900, Michael Paquier wrote:
>> On Fri, May 04, 2018 at 12:32:23PM +0300, Marina Polyakova wrote:
>>> I got a similar server crash as in [1] on the master
On Mon, May 07, 2018 at 10:37:10AM +0900, Michael Paquier wrote:
> On Fri, May 04, 2018 at 12:32:23PM +0300, Marina Polyakova wrote:
> > I got a similar server crash as in [1] on the master branch since the commit
> > 9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails because
> > the
On 07-05-2018 4:37, Michael Paquier wrote:
On Fri, May 04, 2018 at 12:32:23PM +0300, Marina Polyakova wrote:
I got a similar server crash as in [1] on the master branch since the
commit
9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails
because
the second argument ScalarArrayOpEx
On Fri, May 04, 2018 at 12:32:23PM +0300, Marina Polyakova wrote:
> I got a similar server crash as in [1] on the master branch since the commit
> 9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails because
> the second argument ScalarArrayOpExpr is not a Const or an ArrayExpr, but is
Hello everyone in this thread!
I got a similar server crash as in [1] on the master branch since the
commit 9fdb675fc5d2de825414e05939727de8b120ae81 when the assertion fails
because the second argument ScalarArrayOpExpr is not a Const or an
ArrayExpr, but is an ArrayCoerceExpr (see [2]):
=#
On 2018/04/21 0:58, Alvaro Herrera wrote:
> Amit Langote wrote:
>
>> PS: git grep "partition by hash\|PARTITION BY HASH" on src/test indicates
>> that there are hash partitioning related tests in create_table,
>> foreign_key, and partition_join files as well. Do we want to use the
>> custom opcla
Amit Langote wrote:
> PS: git grep "partition by hash\|PARTITION BY HASH" on src/test indicates
> that there are hash partitioning related tests in create_table,
> foreign_key, and partition_join files as well. Do we want to use the
> custom opclass in those files as well?
By the way, let me sug
On Fri, Apr 13, 2018 at 10:50 AM, Alvaro Herrera
wrote:
> Robert Haas wrote:
>> On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera
>> wrote:
>> > Here's an idea. Why don't we move the function/opclass creation lines
>> > to insert.sql, without the DROPs, and use the same functions/opclasses
>> > i
Robert Haas wrote:
> On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera
> wrote:
> > Here's an idea. Why don't we move the function/opclass creation lines
> > to insert.sql, without the DROPs, and use the same functions/opclasses
> > in the three tests insert.sql, alter_table.sql, hash_part.sql and
On Fri, Apr 13, 2018 at 7:45 AM, Amit Langote
wrote:
> On 2018/04/13 1:47, Robert Haas wrote:
>> On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera
>> wrote:
>>> Here's an idea. Why don't we move the function/opclass creation lines
>>> to insert.sql, without the DROPs, and use the same functions/o
On 13 April 2018 at 14:15, Amit Langote wrote:
> On 2018/04/13 1:47, Robert Haas wrote:
>> On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera
>> wrote:
>>> Here's an idea. Why don't we move the function/opclass creation lines
>>> to insert.sql, without the DROPs, and use the same functions/opclass
On 2018/04/13 1:47, Robert Haas wrote:
> On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera
> wrote:
>> Here's an idea. Why don't we move the function/opclass creation lines
>> to insert.sql, without the DROPs, and use the same functions/opclasses
>> in the three tests insert.sql, alter_table.sql,
On Wed, Apr 11, 2018 at 8:35 AM, Alvaro Herrera wrote:
> Here's an idea. Why don't we move the function/opclass creation lines
> to insert.sql, without the DROPs, and use the same functions/opclasses
> in the three tests insert.sql, alter_table.sql, hash_part.sql and
> partition_prune.sql, i.e. n
On 2018/04/11 21:35, Alvaro Herrera wrote:
> Here's an idea. Why don't we move the function/opclass creation lines
> to insert.sql, without the DROPs, and use the same functions/opclasses
> in the three tests insert.sql, alter_table.sql, hash_part.sql and
> partition_prune.sql, i.e. not recreate w
Here's an idea. Why don't we move the function/opclass creation lines
to insert.sql, without the DROPs, and use the same functions/opclasses
in the three tests insert.sql, alter_table.sql, hash_part.sql and
partition_prune.sql, i.e. not recreate what are essentially the same
objects three times?
On 11 April 2018 at 21:22, Amit Langote wrote:
>> Also just wondering if it's worth adding some verification that we've
>> actually eliminated the correct partitions by backing the tests up
>> with a call to satisfies_hash_partition.
>>
>> I've attached a delta patch that applies to your v2 which
On Wed, Apr 11, 2018 at 2:52 PM, Amit Langote
wrote:
>>
>> I've attached a delta patch that applies to your v2 which does this.
>> Do you think it's worth doing?
>
> We can see check by inspection that individual values are in appropriate
> partitions, which is the point of having the inserts and
Hi David.
Thanks for the review.
On 2018/04/11 17:59, David Rowley wrote:
> On 11 April 2018 at 18:04, Amit Langote wrote:
>> Updated patch attached.
>
> Thanks for the updated patch.
>
> The only thing I'm not sure about is the chances you've made to the
> COALESCE function.
>
> +CREATE OR R
On 11 April 2018 at 18:04, Amit Langote wrote:
> Updated patch attached.
Thanks for the updated patch.
The only thing I'm not sure about is the chances you've made to the
COALESCE function.
+CREATE OR REPLACE FUNCTION pp_hashint4_noop(int4, int8) RETURNS int8 AS
+$$SELECT coalesce($1, $2)::int8
Thanks for the review.
On 2018/04/10 21:02, David Rowley wrote:
> On 10 April 2018 at 20:56, Amit Langote wrote:
>> On 2018/04/10 13:27, Ashutosh Bapat wrote:
>>> On Mon, Apr 9, 2018 at 8:56 PM, Robert Haas wrote:
CREATE OR REPLACE FUNCTION hashint4_noop(int4, int8) RETURNS int8 AS
$$S
Thanks for the comment.
On 2018/04/10 21:11, Ashutosh Bapat wrote:
> On Tue, Apr 10, 2018 at 5:32 PM, David Rowley
> wrote:
>> Apart from that confusion, looking at the patch:
>>
>> +CREATE OR REPLACE FUNCTION pp_hashint4_noop(int4, int8) RETURNS int8 AS
>> +$$SELECT coalesce($1)::int8$$ LANGUAGE
On Tue, Apr 10, 2018 at 5:32 PM, David Rowley
wrote:
> On 10 April 2018 at 20:56, Amit Langote wrote:
>> On 2018/04/10 13:27, Ashutosh Bapat wrote:
>>> On Mon, Apr 9, 2018 at 8:56 PM, Robert Haas wrote:
CREATE OR REPLACE FUNCTION hashint4_noop(int4, int8) RETURNS int8 AS
$$SELECT coale
On 10 April 2018 at 20:56, Amit Langote wrote:
> On 2018/04/10 13:27, Ashutosh Bapat wrote:
>> On Mon, Apr 9, 2018 at 8:56 PM, Robert Haas wrote:
>>> CREATE OR REPLACE FUNCTION hashint4_noop(int4, int8) RETURNS int8 AS
>>> $$SELECT coalesce($1,0)::int8$$ LANGUAGE sql IMMUTABLE;
>>> CREATE OPERATO
On 2018/04/10 13:27, Ashutosh Bapat wrote:
> On Mon, Apr 9, 2018 at 8:56 PM, Robert Haas wrote:
>> On Fri, Apr 6, 2018 at 11:41 PM, Tom Lane wrote:
>>> David Rowley writes:
Sounds like you're saying that if we have too many alternative files
then there's a chance that one could pass by
On Mon, Apr 9, 2018 at 8:56 PM, Robert Haas wrote:
> On Fri, Apr 6, 2018 at 11:41 PM, Tom Lane wrote:
>> David Rowley writes:
>>> Sounds like you're saying that if we have too many alternative files
>>> then there's a chance that one could pass by luck.
>>
>> Yeah, exactly: it passed, but did it
On Fri, Apr 6, 2018 at 11:41 PM, Tom Lane wrote:
> David Rowley writes:
>> Sounds like you're saying that if we have too many alternative files
>> then there's a chance that one could pass by luck.
>
> Yeah, exactly: it passed, but did it pass for the right reason?
>
> If there's just two expecte
Hi David.
On 2018/04/09 12:48, David Rowley wrote:
> While looking at the docs in [1], I saw that we still mention:
>
> 4. Ensure that the constraint_exclusion configuration parameter is not
> disabled in postgresql.conf. If it is, queries will not be optimized
> as desired.
>
> This is no longe
While looking at the docs in [1], I saw that we still mention:
4. Ensure that the constraint_exclusion configuration parameter is not
disabled in postgresql.conf. If it is, queries will not be optimized
as desired.
This is no longer true. The attached patch removed it.
[1] https://www.postgresql
Andres Freund wrote:
> On 2018-04-07 08:13:23 -0300, Alvaro Herrera wrote:
> > Andres Freund wrote:
> > > I've also attempted to fix rhinoceros's failure I remarked upon a couple
> > > hours ago in
> > > https://postgr.es/m/20180406210330.wmqw42wqgiick...@alap3.anarazel.de
> >
> > And this, too.
On 2018-04-07 08:13:23 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > I've also attempted to fix rhinoceros's failure I remarked upon a couple
> > hours ago in
> > https://postgr.es/m/20180406210330.wmqw42wqgiick...@alap3.anarazel.de
>
> And this, too. I was unsure if this was because we
Andres Freund wrote:
> Hi,
>
> On 2018-04-07 15:49:54 +1200, David Rowley wrote:
> > Right, I suggest we wait and see if all members go green again as a
> > result of 40e42e1024c, and if they're happy then we could maybe leave
> > it as is with the 2 alternatives output files.
>
> At least the fi
On Sat, Apr 7, 2018 at 1:39 PM, Tom Lane wrote:
> Amit Langote writes:
>> Given that the difference only appeared on animals that David pointed
>> out have big-endian architecture, it seems we'd only need two output
>> files.
>
> Dunno, I'm wondering whether 32 vs 64 bit will make a difference.
Amit Langote writes:
> Given that the difference only appeared on animals that David pointed
> out have big-endian architecture, it seems we'd only need two output
> files.
Dunno, I'm wondering whether 32 vs 64 bit will make a difference.
regards, tom lane
On Sat, Apr 7, 2018 at 1:09 PM, Andres Freund wrote:
> Hi,
>
> On 2018-04-07 15:49:54 +1200, David Rowley wrote:
>> Right, I suggest we wait and see if all members go green again as a
>> result of 40e42e1024c, and if they're happy then we could maybe leave
>> it as is with the 2 alternatives outpu
On 7 April 2018 at 15:18, Andres Freund wrote:
> I've pushed the two patches (collapsed). Trying to get the BF green-ish
> again...
termite has now gone green.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 7 April 2018 at 16:09, Andres Freund wrote:
> I've also attempted to fix rhinoceros's failure I remarked upon a couple
> hours ago in
> https://postgr.es/m/20180406210330.wmqw42wqgiick...@alap3.anarazel.de
Oh, thanks!
I had just been looking at that too...
--
David Rowley
On 7 April 2018 at 09:03, Andres Freund wrote:
> There's also
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2018-04-06%2020%3A45%3A01
> ***
> /opt/src/pgsql-git/build-farm-root/HEAD/pgsql.build/contrib/sepgsql/expected/misc.out
>2018-02-20 18:45:02.068665297 -080
Hi,
On 2018-04-07 15:49:54 +1200, David Rowley wrote:
> Right, I suggest we wait and see if all members go green again as a
> result of 40e42e1024c, and if they're happy then we could maybe leave
> it as is with the 2 alternatives output files.
At least the first previously borked animal came bac
On 7 April 2018 at 15:41, Tom Lane wrote:
> David Rowley writes:
>> Sounds like you're saying that if we have too many alternative files
>> then there's a chance that one could pass by luck.
>
> Yeah, exactly: it passed, but did it pass for the right reason?
>
> If there's just two expected-files
Hi,
On 2018-04-06 23:41:22 -0400, Tom Lane wrote:
> David Rowley writes:
> > Sounds like you're saying that if we have too many alternative files
> > then there's a chance that one could pass by luck.
>
> Yeah, exactly: it passed, but did it pass for the right reason?
>
> If there's just two ex
On 7 April 2018 at 15:41, Tom Lane wrote:
> I'm also wondering how come we had hash partitioning before and
> did not have this sort of problem. Is it just that we added a
> new test that's more sensitive to the details of the hashing
> (if so, could it be made less so)? Or is there actually mor
David Rowley writes:
> Sounds like you're saying that if we have too many alternative files
> then there's a chance that one could pass by luck.
Yeah, exactly: it passed, but did it pass for the right reason?
If there's just two expected-files, it's likely not a big problem,
but if you have a bu
On Sat, Apr 7, 2018 at 8:55 AM, David Rowley
wrote:
> On 7 April 2018 at 15:14, Ashutosh Bapat
> wrote:
>> On Sat, Apr 7, 2018 at 8:37 AM, David Rowley
>>> Why is writing tests that produce the same output required?
>>>
>>> We have many tests with alternative outputs. Look in
>>> src/tests/regres
On 7 April 2018 at 15:14, Ashutosh Bapat
wrote:
> On Sat, Apr 7, 2018 at 8:37 AM, David Rowley
>> Why is writing tests that produce the same output required?
>>
>> We have many tests with alternative outputs. Look in
>> src/tests/regress/expected for files matching _1.out
>>
>
> That's true, but w
On 7 April 2018 at 15:18, Andres Freund wrote:
> I've pushed the two patches (collapsed). Trying to get the BF green-ish
> again...
Thanks!
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 2018-04-07 15:04:37 +1200, David Rowley wrote:
> On 7 April 2018 at 15:00, Andres Freund wrote:
> > On 2018-04-07 14:42:53 +1200, David Rowley wrote:
> >> On 7 April 2018 at 13:31, David Rowley
> >> wrote:
> >> > Maybe the best solution is to pull those tests out of
> >> > partition_prune.sql
On Sat, Apr 7, 2018 at 8:37 AM, David Rowley
wrote:
> On 7 April 2018 at 15:03, Ashutosh Bapat
> wrote:
>> On Sat, Apr 7, 2018 at 7:25 AM, David Rowley
>>> The only alternative would be to change all the hash functions so that
>>> they normalise their endianness. It does not sound like something
On 7 April 2018 at 15:03, Ashutosh Bapat
wrote:
> On Sat, Apr 7, 2018 at 7:25 AM, David Rowley
>> The only alternative would be to change all the hash functions so that
>> they normalise their endianness. It does not sound like something that
>> will perform very well. Plus it would break everyone
On 7 April 2018 at 15:00, Andres Freund wrote:
> On 2018-04-07 14:42:53 +1200, David Rowley wrote:
>> On 7 April 2018 at 13:31, David Rowley wrote:
>> > Maybe the best solution is to pull those tests out of
>> > partition_prune.sql then create partition_prune_hash and just have an
>> > alternativ
On Sat, Apr 7, 2018 at 7:25 AM, David Rowley
wrote:
> On 7 April 2018 at 13:50, Amit Langote wrote:
>> On Sat, Apr 7, 2018 at 10:31 AM, David Rowley
>>> I looked at all the regression test diffs for each of the servers you
>>> mentioned and I verified that the diffs match on each of the 7
>>> ser
On 2018-04-07 14:42:53 +1200, David Rowley wrote:
> On 7 April 2018 at 13:31, David Rowley wrote:
> > Maybe the best solution is to pull those tests out of
> > partition_prune.sql then create partition_prune_hash and just have an
> > alternative .out file with the partitions which match on bigendi
On 7 April 2018 at 13:31, David Rowley wrote:
> Maybe the best solution is to pull those tests out of
> partition_prune.sql then create partition_prune_hash and just have an
> alternative .out file with the partitions which match on bigendian
> machines.
Here's 1 of 2. I thought it was best to ge
On 7 April 2018 at 13:50, Amit Langote wrote:
> On Sat, Apr 7, 2018 at 10:31 AM, David Rowley
>> I looked at all the regression test diffs for each of the servers you
>> mentioned and I verified that the diffs match on each of the 7
>> servers.
>>
>> Maybe the best solution is to pull those tests
On Sat, Apr 7, 2018 at 10:31 AM, David Rowley
wrote:
> On 7 April 2018 at 12:43, David Rowley wrote:
>> On 7 April 2018 at 12:35, Amit Langote wrote:
>>> So this same failure occurs on (noting the architecture):
>>>
>>> Seems to be due to that the hashing function used in partitioning
>>> gives
On 7 April 2018 at 12:43, David Rowley wrote:
> On 7 April 2018 at 12:35, Amit Langote wrote:
>> So this same failure occurs on (noting the architecture):
>>
>> Seems to be due to that the hashing function used in partitioning
>> gives different answer for a given set of partition key values than
On 7 April 2018 at 12:35, Amit Langote wrote:
> Thank you Alvaro for rest of the cleanup and committing.
+10!
> So this same failure occurs on (noting the architecture):
>
> ppc64:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=quokka&dt=2018-04-06%2020%3A09%3A52
>
> ia64:
> https://b
Thank you Alvaro for rest of the cleanup and committing.
On Sat, Apr 7, 2018 at 5:28 AM, Alvaro Herrera wrote:
> So I pushed this 25 minutes ago, and already there's a couple of
> buildfarm members complaining:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=quokka&dt=2018-04-06%2020%3A
Robert Haas wrote:
> On Fri, Apr 6, 2018 at 8:24 AM, Alvaro Herrera
> wrote:
> > I don't actually like very much the idea of putting all this code in
> > optimizer/util. This morning it occurred to me that we should create a new
> > src/backend/partitioning/ (and a src/include/partitioning/ to g
On Fri, Apr 6, 2018 at 8:24 AM, Alvaro Herrera wrote:
> I don't actually like very much the idea of putting all this code in
> optimizer/util. This morning it occurred to me that we should create a new
> src/backend/partitioning/ (and a src/include/partitioning/ to go with
> it) and drop a bunch
On 2018-04-06 17:28:00 -0300, Alvaro Herrera wrote:
> So I pushed this 25 minutes ago, and already there's a couple of
> buildfarm members complaining:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=quokka&dt=2018-04-06%2020%3A09%3A52
> https://buildfarm.postgresql.org/cgi-bin/show_log.p
So I pushed this 25 minutes ago, and already there's a couple of
buildfarm members complaining:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=quokka&dt=2018-04-06%2020%3A09%3A52
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=termite&dt=2018-04-06%2019%3A55%3A07
Both show exactly
Hi Alvaro,
On 04/06/2018 12:41 PM, Alvaro Herrera wrote:
Here's my proposed patch.
Idle thought: how about renaming the "constfalse" argument and variables
to "contradictory" or maybe just "contradict"?
Passes check-world.
New directories, and variable rename seems like a good idea; either
On Sat, Apr 7, 2018 at 1:41 AM, Alvaro Herrera wrote:
> Here's my proposed patch.
>
> Idle thought: how about renaming the "constfalse" argument and variables
> to "contradictory" or maybe just "contradict"?
Sounds fine to me.
Thanks,
Amit
Amit Langote wrote:
> Some comments on the code reorganizing part of the patch:
>
> * Did you intentionally not put PartitionBoundInfoData and its accessor
> macros in partition_internal.h. partprune.c would not need to include
> partition.h if we do that.
Not really.
After pondering this some
On Fri, Apr 6, 2018 at 11:38 PM, Alvaro Herrera wrote:
> Yeah, there is one sentence there I didn't quite understand and would
> like to add it to the rewritten version of the comment before I remove
> the whole ifdeffed-out comment.
>
> * PARTCLAUSE_MATCH_STEPS: *clause_steps set to list of
On Fri, Apr 6, 2018 at 11:54 PM, Alvaro Herrera wrote:
> Alvaro Herrera wrote:
>
>> Yeah. Looking at this function, I noticed it tests for BooleanTest, and
>> falls back to checking "not_clause" and a few equals. Does it make
>> sense if the clause is a SAOP? I added this assert:
>> Asser
Alvaro Herrera wrote:
> Yeah. Looking at this function, I noticed it tests for BooleanTest, and
> falls back to checking "not_clause" and a few equals. Does it make
> sense if the clause is a SAOP? I added this assert:
> Assert(IsA(clause, BooleanTest) ||
> IsA(clause, Bo
BTW, having both key_is_null and key_is_not_null output args to convey a
single bit of info is a bit lame. I'm removing it. We could do the
same with a single boolean, since the return value already indicates
it's a matching IS [NOT] NULL clause; we only need to indicate whether
the NOT is presen
David Rowley wrote:
> 2. I guess this will be removed before commit.
>
> +#if 0
>
> +#endif
Yeah, there is one sentence there I didn't quite understand and would
like to add it to the rewritten version of the comment before I remove
the whole ifdeffed-out comment.
* PARTCLAUSE_MATCH_STEP
Amit Langote wrote:
> Hi.
>
> On 2018/04/06 7:35, Alvaro Herrera wrote:
> > I seems pretty clear that putting get_matching_partitions() in
> > catalog/partition.c is totally the wrong thing; it belongs wholly in
> > partprune. I think the reason you put it there is that it requires
> > access to a
On 2018/04/06 8:33, Alvaro Herrera wrote:
>> @@ -1717,8 +1691,8 @@ expand_partitioned_rtentry(PlannerInfo *root,
>> RangeTblEntry *parentrte,
>> * parentrte already has the root partrel's updatedCols translated to
>> match
>> * the attribute ordering of parentrel.
>> */
>> -
Hi.
On 2018/04/06 7:35, Alvaro Herrera wrote:
> I seems pretty clear that putting get_matching_partitions() in
> catalog/partition.c is totally the wrong thing; it belongs wholly in
> partprune. I think the reason you put it there is that it requires
> access to a lot of internals that are static
On 6 April 2018 at 12:02, David Rowley wrote:
> On 6 April 2018 at 10:35, Alvaro Herrera wrote:
> The only other thing I noted on this pass is that we could get rid of:
>
> + /* go check the next clause. */
> + if (unsupported_clause)
> + break;
>
> and just "continue" instead of "break" in all c
On 6 April 2018 at 10:35, Alvaro Herrera wrote:
> I changed a lot of code also, but cosmetic changes only.
>
> I'll clean this up a bit more now, and try to commit shortly (or early
> tomorrow); wanted to share current status now in case I have to rush
> out.
I made a complete pass over the patch
> @@ -1717,8 +1691,8 @@ expand_partitioned_rtentry(PlannerInfo *root,
> RangeTblEntry *parentrte,
>* parentrte already has the root partrel's updatedCols translated to
> match
>* the attribute ordering of parentrel.
>*/
> - if (!*part_cols_updated)
> - *par
Amit Langote wrote:
> >> 1. Still not sure about RelOptInfo->has_default_part. This flag is
> >> only looked at in generate_partition_pruning_steps. The RelOptInfo and
> >> the boundinfo is available to look at, it's just that the
> >> partition_bound_has_default macro is defined in partition.c ra
Hi.
On 2018/04/05 0:45, Jesper Pedersen wrote:
> Hi,
>
> On 04/04/2018 09:29 AM, David Rowley wrote:
>> Thanks for updating. I've made a pass over v49 and I didn't find very
>> much wrong with it.
>>
>> The only real bug I found was a missing IsA(rinfo->clause, Const) in
>> the pseudoconstant che
Hi,
On 04/04/2018 09:29 AM, David Rowley wrote:
Thanks for updating. I've made a pass over v49 and I didn't find very
much wrong with it.
The only real bug I found was a missing IsA(rinfo->clause, Const) in
the pseudoconstant check inside
generate_partition_pruning_steps_internal.
Most of the
On 4 April 2018 at 19:04, Amit Langote wrote:
> On 2018/04/04 14:42, Amit Langote wrote:
>> Attached v48.
>
> I had forgotten to remove the static_pruning parameter I had added in the
> v47, because it is no longer used. Static pruning now occurs even if a
> step contains all Params, in which cas
On 4 April 2018 at 19:04, Amit Langote wrote:
> Attached v49.
Thank for including the changes. I'll look now.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 4 April 2018 at 17:42, Amit Langote wrote:
> I'm not sure about the following change in your patch:
>
> - if (!result->scan_null)
> - result->scan_null = step_result->scan_null;
> - if (!result->scan_default)
> - result->sc
On 4 April 2018 at 16:00, Tom Lane wrote:
> David Rowley writes:
>> It's true that the const simplification code will generally rewrite
>> most NOT(clause) to use the negator operator, but if the operator does
>> not have a negator it can't do this.
>> ...
>> At the moment pruning does not work f
David Rowley writes:
> It's true that the const simplification code will generally rewrite
> most NOT(clause) to use the negator operator, but if the operator does
> not have a negator it can't do this.
> ...
> At the moment pruning does not work for this case at all. Perhaps it should?
It's hard
On 4 April 2018 at 13:13, David Rowley wrote:
> There might be another issue with the patch too, but I'll send a
> separate email about that.
In the current version of the patch the following comment exists:
/*
* Fall-through for a NOT clause, which is handled in
* match_clause_to_partition_key(
On 4 April 2018 at 11:22, David Rowley wrote:
> On 4 April 2018 at 09:47, David Rowley wrote:
>> I think it would be better to just have special handling in
>> get_matching_list_bound so that it knows it's performing <>
>> elimination. I'd thought about passing some other opstrategy but the
>> on
1 - 100 of 277 matches
Mail list logo
