public inbox for [email protected]
help / color / mirror / Atom feedRe: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
42+ messages / 9 participants
[nested] [flat]
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-04 10:41 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Peter Eisentraut @ 2019-01-04 10:41 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 21/11/2018 14:28, Christoph Berg wrote:
> The context here is that we want to use the *FLAGS from pg_config for
> compiling PG extension packages, but add additional *FLAGS from the
> extension build environment. Merging the pg_config CFLAGS with the
> environment CFLAGS seemed hard/weird/error-prone, so what we are doing
> now is https://salsa.debian.org/postgresql/postgresql-common/blob/master/pg_buildext#L53-55
>
> if [ "${CFLAGS:-}" ]; then
> export COPT="$CFLAGS"
> fi
Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS
in a safe way. Maybe we should add an equivalent for CFLAGS? It's just
that it hasn't been needed so far.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-04 13:33 Michael Paquier <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2019-01-04 13:33 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Fri, Jan 04, 2019 at 11:41:15AM +0100, Peter Eisentraut wrote:
> Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS
> in a safe way. Maybe we should add an equivalent for CFLAGS? It's just
> that it hasn't been needed so far.
+1. Wouldn't it make sense to also have PG_LDFLAGS? In some stuff I
maintain, I have been abusing of PG_CPPFLAGS to pass some flags, which
is not really right. We also have contrib/passwordcheck/Makefile
abuse of it to set -DUSE_CRACKLIB..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-07 09:17 Christoph Berg <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Christoph Berg @ 2019-01-07 09:17 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Re: Michael Paquier 2019-01-04 <[email protected]>
> On Fri, Jan 04, 2019 at 11:41:15AM +0100, Peter Eisentraut wrote:
> > Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS
> > in a safe way. Maybe we should add an equivalent for CFLAGS? It's just
> > that it hasn't been needed so far.
>
> +1. Wouldn't it make sense to also have PG_LDFLAGS? In some stuff I
> maintain, I have been abusing of PG_CPPFLAGS to pass some flags, which
> is not really right. We also have contrib/passwordcheck/Makefile
> abuse of it to set -DUSE_CRACKLIB..
I'll buy whatever version that works. I'm using CFLAGS at the moment,
but could easily switch to other variables.
Note that the missing bit here is the CXX part, adding PG_CFLAGS alone
wouldn't help.
Updated patch attached.
Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-07 09:32 Christoph Berg <[email protected]>
parent: Christoph Berg <[email protected]>
0 siblings, 2 replies; 42+ messages in thread
From: Christoph Berg @ 2019-01-07 09:32 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Re: To Michael Paquier 2019-01-07 <[email protected]>
> Updated patch attached.
Here's another revision that doesn't add an extra CXXOPT variable but
just extends CXXFLAGS whenever COPT or PROFILE are set, which seems
more usable.
It also updates the documentation.
Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-08 01:14 Michael Paquier <[email protected]>
parent: Christoph Berg <[email protected]>
1 sibling, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2019-01-08 01:14 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Mon, Jan 07, 2019 at 10:32:20AM +0100, Christoph Berg wrote:
> Here's another revision that doesn't add an extra CXXOPT variable but
> just extends CXXFLAGS whenever COPT or PROFILE are set, which seems
> more usable.
>
> It also updates the documentation.
The documentation is not fully updated. It still lacks the
description for each new field.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-08 01:18 Andres Freund <[email protected]>
parent: Christoph Berg <[email protected]>
1 sibling, 1 reply; 42+ messages in thread
From: Andres Freund @ 2019-01-08 01:18 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2019-01-07 10:32:20 +0100, Christoph Berg wrote:
> Re: To Michael Paquier 2019-01-07 <[email protected]>
> > Updated patch attached.
>
> Here's another revision that doesn't add an extra CXXOPT variable but
> just extends CXXFLAGS whenever COPT or PROFILE are set, which seems
> more usable.
Why does that seem more usable? How's that supposed to be used for flags
that aren't valid for both languages?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-08 08:09 Christoph Berg <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Christoph Berg @ 2019-01-08 08:09 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Re: Andres Freund 2019-01-08 <[email protected]>
> > Here's another revision that doesn't add an extra CXXOPT variable but
> > just extends CXXFLAGS whenever COPT or PROFILE are set, which seems
> > more usable.
>
> Why does that seem more usable? How's that supposed to be used for flags
> that aren't valid for both languages?
The existing COPT and PROFILE are already catch-all type flags that
add to CFLAGS and LDFLAGS. Possibly we should leave those alone and
only add PG_CXXFLAGS and PG_LDFLAGS?
Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-17 05:55 Michael Paquier <[email protected]>
parent: Christoph Berg <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2019-01-17 05:55 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Tue, Jan 08, 2019 at 09:09:56AM +0100, Christoph Berg wrote:
> Re: Andres Freund 2019-01-08 <[email protected]>
>>> Here's another revision that doesn't add an extra CXXOPT variable but
>>> just extends CXXFLAGS whenever COPT or PROFILE are set, which seems
>>> more usable.
>>
>> Why does that seem more usable? How's that supposed to be used for flags
>> that aren't valid for both languages?
>
> The existing COPT and PROFILE are already catch-all type flags that
> add to CFLAGS and LDFLAGS. Possibly we should leave those alone and
> only add PG_CXXFLAGS and PG_LDFLAGS?
Personally I see pgxs as something completely different than what COPT
and PROFILE are as we are talking about two different facilities: one
which is part of the core installation, and the other which can be
used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and
PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like
the better long-term move in terms of pluggability. My 2c.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-22 06:26 Michael Paquier <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2019-01-22 06:26 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Thu, Jan 17, 2019 at 02:55:39PM +0900, Michael Paquier wrote:
> Personally I see pgxs as something completely different than what COPT
> and PROFILE are as we are talking about two different facilities: one
> which is part of the core installation, and the other which can be
> used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and
> PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like
> the better long-term move in terms of pluggability. My 2c.
It's been a couple of days since this message, and while my opinion
has not really changed, there are many other opinions. So perhaps we
could reduce the proposal to a strict minimum and find an agreement
for the options that we think are absolutely worth adding? Even if we
cannot agree on what COPT of PROFILE should do more, perhaps we could
still agree with only a portion of the flags we think are worth it?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-22 20:00 Andres Freund <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Andres Freund @ 2019-01-22 20:00 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 2019-01-22 15:26:21 +0900, Michael Paquier wrote:
> On Thu, Jan 17, 2019 at 02:55:39PM +0900, Michael Paquier wrote:
> > Personally I see pgxs as something completely different than what COPT
> > and PROFILE are as we are talking about two different facilities: one
> > which is part of the core installation, and the other which can be
> > used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and
> > PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like
> > the better long-term move in terms of pluggability. My 2c.
>
> It's been a couple of days since this message, and while my opinion
> has not really changed, there are many other opinions. So perhaps we
> could reduce the proposal to a strict minimum and find an agreement
> for the options that we think are absolutely worth adding? Even if we
> cannot agree on what COPT of PROFILE should do more, perhaps we could
> still agree with only a portion of the flags we think are worth it?
I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the
fence. I personally think the pgxs stuff is a bit separate, and I'm
doubtful we ought to backpatch that. I'm basically planning to apply
https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de
to 11-, minus the PGXS stuff. If we want that, we ought to apply it to
master only IMO.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-22 20:10 Alvaro Herrera <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Alvaro Herrera @ 2019-01-22 20:10 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 2019-Jan-22, Andres Freund wrote:
> I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the
> fence. I personally think the pgxs stuff is a bit separate, and I'm
> doubtful we ought to backpatch that. I'm basically planning to apply
> https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de
> to 11-, minus the PGXS stuff. If we want that, we ought to apply it to
> master only IMO.
I don't understand why you don't want to backpatch the PGXS bits. Is
there something working today that would be broken by it? I think
you're worried about places that invoke makefiles with PG_CXXFLAGS set
and expecting the value not to be propagated. Is that a scenario we
need to worry about?
The patch neglects to update extend.sgml with the new pgxs variable,
though.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-22 20:19 Andres Freund <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Andres Freund @ 2019-01-22 20:19 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Michael Paquier <[email protected]>; Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2019-01-22 17:10:58 -0300, Alvaro Herrera wrote:
> On 2019-Jan-22, Andres Freund wrote:
>
> > I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the
> > fence. I personally think the pgxs stuff is a bit separate, and I'm
> > doubtful we ought to backpatch that. I'm basically planning to apply
> > https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de
> > to 11-, minus the PGXS stuff. If we want that, we ought to apply it to
> > master only IMO.
>
> I don't understand why you don't want to backpatch the PGXS bits.
Largely because I think it's an independent patch from the CXXOPT need
from Christopher / Debian packaging. It's a larger patch, that needs
more docs etc. If whoever applies that wants to backpatch it - I'm not
going to protest, I just wouldn't myself, unless somebody pipes up that
it'd help them.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-22 21:11 Alvaro Herrera <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Alvaro Herrera @ 2019-01-22 21:11 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hello
On 2019-Jan-22, Andres Freund wrote:
> On 2019-01-22 17:10:58 -0300, Alvaro Herrera wrote:
> > I don't understand why you don't want to backpatch the PGXS bits.
>
> Largely because I think it's an independent patch from the CXXOPT need
> from Christopher / Debian packaging. It's a larger patch, that needs
> more docs etc. If whoever applies that wants to backpatch it - I'm not
> going to protest, I just wouldn't myself, unless somebody pipes up that
> it'd help them.
Ah, I see. No arguments against.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-23 00:47 Michael Paquier <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2019-01-23 00:47 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Andres Freund <[email protected]>; Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Tue, Jan 22, 2019 at 06:11:23PM -0300, Alvaro Herrera wrote:
> On 2019-Jan-22, Andres Freund wrote:
>> Largely because I think it's an independent patch from the CXXOPT need
>> from Christopher / Debian packaging. It's a larger patch, that needs
>> more docs etc. If whoever applies that wants to backpatch it - I'm not
>> going to protest, I just wouldn't myself, unless somebody pipes up that
>> it'd help them.
>
> Ah, I see. No arguments against.
Thanks Andres and Alvaro for the input. No issues with the way of
Andres proposes.
The new PGXS flags would be I think useful to make sure
that things for CFLAGS and LDFLAGS get propagated without having to
hijack the original flags, so I can handle that part. Which one would
be wanted though?
- PG_CXXFLAGS
- PG_LDFLAGS
- PG_CFLAGS
I'd see value in all of them, still everybody has likely a different
opinion, so I would not mind discarding the ones are not thought as
that much useful. New PGXS infrastructure usually finds only its way
on HEAD, so I'd rather not back-patch that part. No issues with the
back-patch portion for CXXOPT from me as that helps Debian.
Thanks,
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-29 15:18 Christoph Berg <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 2 replies; 42+ messages in thread
From: Christoph Berg @ 2019-01-29 15:18 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Andres Freund <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Re: Michael Paquier 2019-01-23 <[email protected]>
> >> Largely because I think it's an independent patch from the CXXOPT need
> >> from Christopher / Debian packaging. It's a larger patch, that needs
> >> more docs etc. If whoever applies that wants to backpatch it - I'm not
> >> going to protest, I just wouldn't myself, unless somebody pipes up that
> >> it'd help them.
> >
> > Ah, I see. No arguments against.
Fwiw I'm not attached to using COPT and friends, I just happend to
pick these because that was one way that worked. With what I've
learned now, PG_*FLAGS is the better approach.
> The new PGXS flags would be I think useful to make sure
> that things for CFLAGS and LDFLAGS get propagated without having to
> hijack the original flags, so I can handle that part. Which one would
> be wanted though?
> - PG_CXXFLAGS
> - PG_LDFLAGS
> - PG_CFLAGS
>
> I'd see value in all of them, still everybody has likely a different
> opinion, so I would not mind discarding the ones are not thought as
> that much useful. New PGXS infrastructure usually finds only its way
> on HEAD, so I'd rather not back-patch that part. No issues with the
> back-patch portion for CXXOPT from me as that helps Debian.
The attached patch adds these three.
Re backpatching, I would at least need them in PG11 because that's
what is going to be released with Debian buster. An official backpatch
to all supported versions would be nice, but I could also sneak in
that change into the Debian packages without breaking anything.
Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-30 01:40 Michael Paquier <[email protected]>
parent: Christoph Berg <[email protected]>
1 sibling, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2019-01-30 01:40 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Alvaro Herrera <[email protected]>; Andres Freund <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Tue, Jan 29, 2019 at 04:18:46PM +0100, Christoph Berg wrote:
> Re backpatching, I would at least need them in PG11 because that's
> what is going to be released with Debian buster. An official backpatch
> to all supported versions would be nice, but I could also sneak in
> that change into the Debian packages without breaking anything.
The change is non-invasive, so if that helps to make packaging easier
and more consistent as a whole, then I could do the back-patch.
Except if somebody has an objection?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-30 01:51 Andres Freund <[email protected]>
parent: Christoph Berg <[email protected]>
1 sibling, 1 reply; 42+ messages in thread
From: Andres Freund @ 2019-01-30 01:51 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2019-01-29 16:18:46 +0100, Christoph Berg wrote:
> Re: Michael Paquier 2019-01-23 <[email protected]>
> > >> Largely because I think it's an independent patch from the CXXOPT need
> > >> from Christopher / Debian packaging. It's a larger patch, that needs
> > >> more docs etc. If whoever applies that wants to backpatch it - I'm not
> > >> going to protest, I just wouldn't myself, unless somebody pipes up that
> > >> it'd help them.
> > >
> > > Ah, I see. No arguments against.
>
> Fwiw I'm not attached to using COPT and friends, I just happend to
> pick these because that was one way that worked. With what I've
> learned now, PG_*FLAGS is the better approach.
I'm confused - that doesn't allow to inject flags to all in-core built
files? So how does that fix your problem fully? Say
e.g. llvmjit_inline.cpp won't get the flag, and thus not be
reproducible?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-30 13:41 Christoph Berg <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Christoph Berg @ 2019-01-30 13:41 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Re: Andres Freund 2019-01-30 <[email protected]>
> I'm confused - that doesn't allow to inject flags to all in-core built
> files? So how does that fix your problem fully? Say
> e.g. llvmjit_inline.cpp won't get the flag, and thus not be
> reproducible?
The original itch being scratched here is extensions written in C++,
which need -ffile-prefix-map (or -fdebug-prefix-map) injected into
CXXFLAGS on top of the flags encoded in pgxs.mk. I originally picked
COPT because that works for injecting flags into pgxs.mk as well, but
PG_*FLAGS is better.
For the server build itself, it shouldn't be necessary to inject flags
because they are passed directly to ./configure. Admittedly I haven't
looked at reproducibility of PG11 yet as clang is still missing the
prefix-map feature, but https://reviews.llvm.org/D49466 is making good
progress.
Do we still want some CXXOPT flag for the server build? I can write a
patch, but someone else would need to do the bikeshedding how to name
it, and which of the existing knobs would set CXXFLAGS along. I don't
think I need that feature.
Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-31 00:44 Michael Paquier <[email protected]>
parent: Christoph Berg <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2019-01-31 00:44 UTC (permalink / raw)
To: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Wed, Jan 30, 2019 at 02:41:01PM +0100, Christoph Berg wrote:
> Do we still want some CXXOPT flag for the server build? I can write a
> patch, but someone else would need to do the bikeshedding how to name
> it, and which of the existing knobs would set CXXFLAGS along. I don't
> think I need that feature.
If we don't directly need it, let's not add it now but let's revisit
the need if it proves necessary. I propose to just commit the last
patch you sent, and back-patch to ease integration with existing
extensions. Any objections?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-31 01:18 Tom Lane <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Tom Lane @ 2019-01-31 01:18 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
Michael Paquier <[email protected]> writes:
> On Wed, Jan 30, 2019 at 02:41:01PM +0100, Christoph Berg wrote:
>> Do we still want some CXXOPT flag for the server build? I can write a
>> patch, but someone else would need to do the bikeshedding how to name
>> it, and which of the existing knobs would set CXXFLAGS along. I don't
>> think I need that feature.
> If we don't directly need it, let's not add it now but let's revisit
> the need if it proves necessary.
+1
> I propose to just commit the last
> patch you sent, and back-patch to ease integration with existing
> extensions. Any objections?
A thought here:
ifdef PG_CPPFLAGS
override CPPFLAGS := $(PG_CPPFLAGS) $(CPPFLAGS)
endif
+ifdef PG_CFLAGS
+override CFLAGS := $(PG_CFLAGS) $(CFLAGS)
+endif
+ifdef PG_CXXFLAGS
+override CXXFLAGS := $(PG_CXXFLAGS) $(CXXFLAGS)
+endif
+ifdef PG_LDFLAGS
+override LDFLAGS := $(PG_LDFLAGS) $(LDFLAGS)
+endif
This looks a bit copy-and-paste-y to me, in particular no thought
has been taken for the order of flags. We found in configure that
it's better to add user-specified CFLAGS at the *end*, even though
injecting user-specified CPPFLAGS at the beginning is the right
thing. This is because in, eg, "-O2 -O0" the last flag wins.
Presumably the same goes for CXXFLAGS. I think it's right to
put user LDFLAGS first, though. (The argument for CPPFLAGS and
LDFLAGS is that you want the user's -I and -L flags to go first.)
regards, tom lane
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
@ 2019-01-31 02:19 Michael Paquier <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2019-01-31 02:19 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Christoph Berg <[email protected]>; Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Wed, Jan 30, 2019 at 08:18:31PM -0500, Tom Lane wrote:
> This looks a bit copy-and-paste-y to me, in particular no thought
> has been taken for the order of flags. We found in configure that
> it's better to add user-specified CFLAGS at the *end*, even though
> injecting user-specified CPPFLAGS at the beginning is the right
> thing. This is because in, eg, "-O2 -O0" the last flag wins.
> Presumably the same goes for CXXFLAGS. I think it's right to
> put user LDFLAGS first, though. (The argument for CPPFLAGS and
> LDFLAGS is that you want the user's -I and -L flags to go first.)
Ah yes, good point about CFLAGS and LDFLAGS. It would be better to
add a comment about that and document the difference, aka "prepend" or
"append" the flag values.
CXXFLAGS applies to compiler options like -g -O2 which you would like
to enforce for the C++ compiler, so it seems to me that like CFLAGS
the custom values should be added at the end and not at the beginning,
no?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH 3/6] Be less verbose on variable names
@ 2019-11-28 22:30 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Alvaro Herrera @ 2019-11-28 22:30 UTC (permalink / raw)
---
src/backend/commands/typecmds.c | 23 +++++++++-------------
src/backend/utils/adt/pg_upgrade_support.c | 4 ++--
src/include/catalog/binary_upgrade.h | 4 ++--
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 1b012c9cad..4ca2d3364b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -85,8 +85,8 @@ typedef struct
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_array_pg_type_oid = InvalidOid;
-Oid binary_upgrade_next_multirange_pg_type_oid = InvalidOid;
-Oid binary_upgrade_next_multirange_array_pg_type_oid = InvalidOid;
+Oid binary_upgrade_next_mrng_pg_type_oid = InvalidOid;
+Oid binary_upgrade_next_mrng_array_pg_type_oid = InvalidOid;
static void makeRangeConstructors(const char *name, Oid namespace,
Oid rangeOid, Oid subtype);
@@ -1529,13 +1529,9 @@ DefineRange(CreateRangeStmt *stmt)
/* alignment must be 'i' or 'd' for ranges */
alignment = (subtypalign == 'd') ? 'd' : 'i';
- /* Allocate OID for array type */
+ /* Allocate OID for array type, its multirange and its multirange array */
rangeArrayOid = AssignTypeArrayOid();
-
- /* Allocate OID for multirange type */
multirangeOid = AssignTypeMultirangeOid();
-
- /* Allocate OID for multirange array type */
multirangeArrayOid = AssignTypeMultirangeArrayOid();
/* Create the pg_type entry */
@@ -1574,7 +1570,6 @@ DefineRange(CreateRangeStmt *stmt)
Assert(typoid == address.objectId);
/* Create the multirange that goes with it */
-
multirangeTypeName = makeMultirangeTypeName(typeName, typeNamespace);
mltrngaddress =
@@ -2313,13 +2308,13 @@ AssignTypeMultirangeOid(void)
/* Use binary-upgrade override for pg_type.oid? */
if (IsBinaryUpgrade)
{
- if (!OidIsValid(binary_upgrade_next_multirange_pg_type_oid))
+ if (!OidIsValid(binary_upgrade_next_mrng_pg_type_oid))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_type multirange OID value not set when in binary upgrade mode")));
- type_multirange_oid = binary_upgrade_next_multirange_pg_type_oid;
- binary_upgrade_next_multirange_pg_type_oid = InvalidOid;
+ type_multirange_oid = binary_upgrade_next_mrng_pg_type_oid;
+ binary_upgrade_next_mrng_pg_type_oid = InvalidOid;
}
else
{
@@ -2346,13 +2341,13 @@ AssignTypeMultirangeArrayOid(void)
/* Use binary-upgrade override for pg_type.oid? */
if (IsBinaryUpgrade)
{
- if (!OidIsValid(binary_upgrade_next_multirange_array_pg_type_oid))
+ if (!OidIsValid(binary_upgrade_next_mrng_array_pg_type_oid))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_type multirange array OID value not set when in binary upgrade mode")));
- type_multirange_array_oid = binary_upgrade_next_multirange_array_pg_type_oid;
- binary_upgrade_next_multirange_array_pg_type_oid = InvalidOid;
+ type_multirange_array_oid = binary_upgrade_next_mrng_array_pg_type_oid;
+ binary_upgrade_next_mrng_array_pg_type_oid = InvalidOid;
}
else
{
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index d980b96f48..418c26c81b 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -57,7 +57,7 @@ binary_upgrade_set_next_multirange_pg_type_oid(PG_FUNCTION_ARGS)
Oid typoid = PG_GETARG_OID(0);
CHECK_IS_BINARY_UPGRADE;
- binary_upgrade_next_multirange_pg_type_oid = typoid;
+ binary_upgrade_next_mrng_pg_type_oid = typoid;
PG_RETURN_VOID();
}
@@ -68,7 +68,7 @@ binary_upgrade_set_next_multirange_array_pg_type_oid(PG_FUNCTION_ARGS)
Oid typoid = PG_GETARG_OID(0);
CHECK_IS_BINARY_UPGRADE;
- binary_upgrade_next_multirange_array_pg_type_oid = typoid;
+ binary_upgrade_next_mrng_array_pg_type_oid = typoid;
PG_RETURN_VOID();
}
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index 2b6e87bb84..ba132ddf23 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -16,8 +16,8 @@
extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
-extern PGDLLIMPORT Oid binary_upgrade_next_multirange_pg_type_oid;
-extern PGDLLIMPORT Oid binary_upgrade_next_multirange_array_pg_type_oid;
+extern PGDLLIMPORT Oid binary_upgrade_next_mrng_pg_type_oid;
+extern PGDLLIMPORT Oid binary_upgrade_next_mrng_array_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_heap_pg_class_oid;
--
2.20.1
--+QahgC5+KEYLbs62
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0004-Protect-comment-against-pgindent.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index ce47a158ae..ddcdbbaf7e 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2308,7 +2308,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5c2a7b7422..9c7b8bf162 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -806,10 +806,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -840,15 +837,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2060,7 +2052,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2072,7 +2063,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 248ff90904b..04ad14f70b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 94a5e2da17c..bbdaa591891 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index ce47a158ae..ddcdbbaf7e 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2308,7 +2308,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5c2a7b7422..9c7b8bf162 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -806,10 +806,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -840,15 +837,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2060,7 +2052,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2072,7 +2063,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 248ff90904b..04ad14f70b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 94a5e2da17c..bbdaa591891 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 248ff90904b..04ad14f70b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -287,7 +287,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0010-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 10c1c3b616b..a1ec50ab7a8 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2248,7 +2248,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 94a5e2da17c..bbdaa591891 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f1d0d4b78e3..e35bd36e710 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -787,10 +787,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -821,15 +818,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -1988,7 +1980,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2000,7 +1991,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 849cac3947..cf4387f443 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2250,7 +2250,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed4..282dcb9791 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b9ba4f9fb3..bcf1497f67 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -795,10 +795,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -829,15 +826,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2025,7 +2017,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2037,7 +2028,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* [PATCH v12 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres
@ 2024-02-12 23:13 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Melanie Plageman @ 2024-02-12 23:13 UTC (permalink / raw)
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult as a means of
communication between table_scan_bitmap_next_tuple() and
table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2...
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 973734e9ffa..2cb5fb18675 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2335,7 +2335,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 49938c9ed41..282dcb97919 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -286,7 +286,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 19e99c81092..ff3e8bcdd6f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -802,10 +802,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -836,15 +833,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2050,7 +2042,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2062,7 +2053,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v12-0009-Make-table_scan_bitmap_next_block-async-friendly.patch"
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: Wrong results with grouping sets
@ 2024-07-06 02:37 Tom Lane <[email protected]>
0 siblings, 1 reply; 42+ messages in thread
From: Tom Lane @ 2024-07-06 02:37 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers
Richard Guo <[email protected]> writes:
> BTW, from catversion.h I read:
> * Another common reason for a catversion update is a change in parsetree
> * external representation, since serialized parsetrees appear in stored
> * rules and new-style SQL functions. Almost any change in primnodes.h or
> * parsenodes.h will warrant a catversion update.
> Since this patchset changes the querytree produced by the parser, does
> this indicate that a catversion bump is needed?
Yes, it would.
regards, tom lane
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: Wrong results with grouping sets
@ 2024-07-10 01:11 Richard Guo <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 42+ messages in thread
From: Richard Guo @ 2024-07-10 01:11 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers
On Sat, Jul 6, 2024 at 10:37 AM Tom Lane <[email protected]> wrote:
> Richard Guo <[email protected]> writes:
> > BTW, from catversion.h I read:
>
> > * Another common reason for a catversion update is a change in parsetree
> > * external representation, since serialized parsetrees appear in stored
> > * rules and new-style SQL functions. Almost any change in primnodes.h or
> > * parsenodes.h will warrant a catversion update.
>
> > Since this patchset changes the querytree produced by the parser, does
> > this indicate that a catversion bump is needed?
>
> Yes, it would.
Thank you for confirming.
Thanks
Richard
^ permalink raw reply [nested|flat] 42+ messages in thread
end of thread, other threads:[~2024-07-10 01:11 UTC | newest]
Thread overview: 42+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-01-04 10:41 Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well Peter Eisentraut <[email protected]>
2019-01-04 13:33 ` Michael Paquier <[email protected]>
2019-01-07 09:17 ` Christoph Berg <[email protected]>
2019-01-07 09:32 ` Christoph Berg <[email protected]>
2019-01-08 01:14 ` Michael Paquier <[email protected]>
2019-01-08 01:18 ` Andres Freund <[email protected]>
2019-01-08 08:09 ` Christoph Berg <[email protected]>
2019-01-17 05:55 ` Michael Paquier <[email protected]>
2019-01-22 06:26 ` Michael Paquier <[email protected]>
2019-01-22 20:00 ` Andres Freund <[email protected]>
2019-01-22 20:10 ` Alvaro Herrera <[email protected]>
2019-01-22 20:19 ` Andres Freund <[email protected]>
2019-01-22 21:11 ` Alvaro Herrera <[email protected]>
2019-01-23 00:47 ` Michael Paquier <[email protected]>
2019-01-29 15:18 ` Christoph Berg <[email protected]>
2019-01-30 01:40 ` Michael Paquier <[email protected]>
2019-01-30 01:51 ` Andres Freund <[email protected]>
2019-01-30 13:41 ` Christoph Berg <[email protected]>
2019-01-31 00:44 ` Michael Paquier <[email protected]>
2019-01-31 01:18 ` Tom Lane <[email protected]>
2019-01-31 02:19 ` Michael Paquier <[email protected]>
2019-11-28 22:30 [PATCH 3/6] Be less verbose on variable names Alvaro Herrera <[email protected]>
2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v9 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v12 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v7 08/13] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v8 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v10 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-02-12 23:13 [PATCH v6 09/14] Remove table_scan_bitmap_next_tuple parameter tbmres Melanie Plageman <[email protected]>
2024-07-06 02:37 Re: Wrong results with grouping sets Tom Lane <[email protected]>
2024-07-10 01:11 ` Re: Wrong results with grouping sets Richard Guo <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox