pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jeff Davis <pgsql@j-davis.com>
Cc: 赵庭海(庭章) <zhaotinghai.zth@alibaba-inc.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Subject: Re: sublink [exists (select xxx group by grouping sets ())] causes an assertion error
Date: Fri, 22 Mar 2024 22:17:26 -0400
Message-ID: <1229327.1711160246@sss.pgh.pa.us> (raw)
In-Reply-To: <7242a03fe0b52e8dd24b21fc8973738105e64990.camel@j-davis.com>
References: <cfd84cb8-12fe-433a-a4bb-f460a4515f9c.zhaotinghai.zth@alibaba-inc.com>
	<932664.1711124920@sss.pgh.pa.us>
	<7242a03fe0b52e8dd24b21fc8973738105e64990.camel@j-davis.com>

Jeff Davis <pgsql@j-davis.com> writes:
> On Fri, 2024-03-22 at 12:28 -0400, Tom Lane wrote:
>> Thanks for the report.  I did some bisecting and found that the crash
>> appears at Jeff's commit c8aeaf3ab (which introduced this assertion)
>> and disappears at Heikki's c4649cce3 (which removed it).  So I would
>> say that the problem is "this assertion is wrong", and we should fix
>> the problem by fixing the assertion, not by hacking around in distant
>> calling code.  On the whole, since this code has been dead for
>> several versions, I'd be inclined to just remove the assertion.

> c4649cce3 didn't add additional calls to LogicalTapeSetBlocks(), so I'm
> not sure if the removal of the Assert was related to his changes, or if
> he just realized the assertion was wrong and removed it along the way?

My guess is he just zapped it because the code block was dependent
on the "tape" abstraction which was going away.  Heikki?

> Also, without the assertion, the word "should" in the comment is
> ambiguous (does it mean "must not" or something else), and it still
> exists in master. Do we care about the calculation being wrong if
> there's an unfinished write?

I think it's actually fine.  The callers of LogicalTapeSetBlocks only
use the number for statistics or trace reporting, so precision isn't
critical in the first place, but I think what they care about is the
amount of data that's really been written out to files.  The
comment should be clarified, for sure.

			regards, tom lane





view thread (4+ messages)

Message-ID: <1229327.1711160246@sss.pgh.pa.us>
Permalink:  ../1229327.1711160246@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1229327.1711160246@sss.pgh.pa.us

 · 

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: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql@j-davis.com, zhaotinghai.zth@alibaba-inc.com, pgsql-hackers@lists.postgresql.org, hlinnaka@iki.fi
  Subject: Re: sublink [exists (select xxx group by grouping sets ())] causes an assertion error
  In-Reply-To: <1229327.1711160246@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox