public inbox for [email protected]
help / color / mirror / Atom feedRe: GUC for temporarily disabling event triggers
28+ messages / 6 participants
[nested] [flat]
* Re: GUC for temporarily disabling event triggers
@ 2023-04-02 19:48 Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Justin Pryzby @ 2023-04-02 19:48 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Mikhail Gribkov <[email protected]>; [email protected]
On Mon, Mar 06, 2023 at 01:24:56PM +0100, Daniel Gustafsson wrote:
> > On 27 Jan 2023, at 15:00, Mikhail Gribkov <[email protected]> wrote:
>
> > There is a complete framework for disabling various types of the event triggers separately, but, the list of valid GUC values only include 'all' and 'none'. Why not adding support for all the event trigger types separately? Everything is already there in the patch; the only thing needed is expanding couple of enums. It's cheap in terms of code size and even cheaper in terms of performance. And moreover - it would be a good example for anyone adding new trigger types.
>
> I can't exactly recall my reasoning, but I do think you're right that if we're
> to have this GUC it should support the types of existing EVTs. The updated v4
> implements that as well as a rebase on top of master and fixing a typo
> discovered upthread.
>
+ gettext_noop("Disable event triggers for the duration of the session."),
Why does is it say "for the duration of the session" ?
It's possible to disable ignoring, and within the same session.
GUCs are typically "for the duration of the session" .. but they don't
say so (and don't need to).
+ elog(ERROR, "unsupport event trigger: %d", event);
typo: unsupported
+ Allows to temporarily disable event triggers from executing in order
=> Allow temporarily disabling execution of event triggers ..
+ to troubleshoot and repair faulty event triggers. The value matches
+ the type of event trigger to be ignored:
+ <literal>ddl_command_start</literal>, <literal>ddl_command_end</literal>,
+ <literal>table_rewrite</literal> and <literal>sql_drop</literal>.
+ Additionally, all event triggers can be disabled by setting it to
+ <literal>all</literal>. Setting the value to <literal>none</literal>
+ will ensure that all event triggers are enabled, this is the default
It doesn't technically "ensure that they're enabled", since they can be
disabled by ALTER. Better to say that it "doesn't disable any even triggers".
--
Justin
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
@ 2023-04-03 12:45 ` Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Gustafsson @ 2023-04-03 12:45 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Mikhail Gribkov <[email protected]>; [email protected]
> On 2 Apr 2023, at 21:48, Justin Pryzby <[email protected]> wrote:
> + gettext_noop("Disable event triggers for the duration of the session."),
>
> Why does is it say "for the duration of the session" ?
>
> It's possible to disable ignoring, and within the same session.
> GUCs are typically "for the duration of the session" .. but they don't
> say so (and don't need to).
>
> + elog(ERROR, "unsupport event trigger: %d", event);
>
> typo: unsupported
>
> + Allows to temporarily disable event triggers from executing in order
>
> => Allow temporarily disabling execution of event triggers ..
>
> + to troubleshoot and repair faulty event triggers. The value matches
> + the type of event trigger to be ignored:
> + <literal>ddl_command_start</literal>, <literal>ddl_command_end</literal>,
> + <literal>table_rewrite</literal> and <literal>sql_drop</literal>.
> + Additionally, all event triggers can be disabled by setting it to
> + <literal>all</literal>. Setting the value to <literal>none</literal>
> + will ensure that all event triggers are enabled, this is the default
>
> It doesn't technically "ensure that they're enabled", since they can be
> disabled by ALTER. Better to say that it "doesn't disable any even triggers".
All comments above addressed in the attached v5, thanks for review!
--
Daniel Gustafsson
Attachments:
[application/octet-stream] v5-0001-Add-GUC-for-temporarily-disabling-event-triggers.patch (12.5K, ../../[email protected]/2-v5-0001-Add-GUC-for-temporarily-disabling-event-triggers.patch)
download | inline diff:
From a775df518b78a82a2d95637c07709bb8a7821cb4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 3 Apr 2023 14:33:03 +0200
Subject: [PATCH v5] Add GUC for temporarily disabling event triggers
In order to troubleshoot misbehaving or buggy event triggers, the
documented advice is to enter single-user mode. In an attempt to
reduce the number of situations where single-user mode is required
for non-extraordinary maintenance, this GUC allows to temporarily
suspend event triggers.
This was originally extracted from a larger patchset which aimed
at supporting event triggers on login events.
Reviewed-by: Ted Yu <[email protected]>
Reviewed-by: Mikhail Gribkov <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/config.sgml | 23 ++++++++
doc/src/sgml/ref/create_event_trigger.sgml | 4 +-
src/backend/commands/event_trigger.c | 53 ++++++++++++++++---
src/backend/utils/misc/guc_tables.c | 21 ++++++++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/commands/event_trigger.h | 12 +++++
src/test/regress/expected/event_trigger.out | 29 ++++++++++
src/test/regress/sql/event_trigger.sql | 32 +++++++++++
8 files changed, 167 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index fcb53c6997..989ab42701 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9547,6 +9547,29 @@ SET XML OPTION { DOCUMENT | CONTENT };
</listitem>
</varlistentry>
+ <varlistentry id="guc-ignore-event-trigger" xreflabel="ignore_event_trigger">
+ <term><varname>ignore_event_trigger</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>ignore_event_trigger</varname></primary>
+ <secondary>configuration parameter</secondary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Allow temporarily disabling execution of event triggers in order
+ to troubleshoot and repair faulty event triggers. The value matches
+ the type of event trigger to be ignored:
+ <literal>ddl_command_start</literal>, <literal>ddl_command_end</literal>,
+ <literal>table_rewrite</literal> and <literal>sql_drop</literal>.
+ Additionally, all event triggers can be disabled by setting it to
+ <literal>all</literal>. Setting the value to <literal>none</literal>
+ will not disable any event triggers, this is the default value. Only
+ superusers and users with the appropriate <literal>SET</literal>
+ privilege can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
<sect2 id="runtime-config-client-format">
diff --git a/doc/src/sgml/ref/create_event_trigger.sgml b/doc/src/sgml/ref/create_event_trigger.sgml
index 22c8119198..f6922c3de3 100644
--- a/doc/src/sgml/ref/create_event_trigger.sgml
+++ b/doc/src/sgml/ref/create_event_trigger.sgml
@@ -123,7 +123,9 @@ CREATE EVENT TRIGGER <replaceable class="parameter">name</replaceable>
Event triggers are disabled in single-user mode (see <xref
linkend="app-postgres"/>). If an erroneous event trigger disables the
database so much that you can't even drop the trigger, restart in
- single-user mode and you'll be able to do that.
+ single-user mode and you'll be able to do that. Event triggers can also be
+ temporarily disabled for such troubleshooting, see
+ <xref linkend="guc-ignore-event-trigger"/>.
</para>
</refsect1>
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index d4b00d1a82..0eaf1f4553 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -72,6 +72,9 @@ typedef struct EventTriggerQueryState
static EventTriggerQueryState *currentEventTriggerState = NULL;
+/* GUC parameter */
+int ignore_event_trigger = IGNORE_EVENT_TRIGGER_NONE;
+
/* Support for dropped objects */
typedef struct SQLDropObject
{
@@ -100,6 +103,7 @@ static void validate_table_rewrite_tags(const char *filtervar, List *taglist);
static void EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata);
static const char *stringify_grant_objtype(ObjectType objtype);
static const char *stringify_adefprivs_objtype(ObjectType objtype);
+static bool ignore_event_trigger_check(EventTriggerEvent event);
/*
* Create an event trigger.
@@ -657,8 +661,11 @@ EventTriggerDDLCommandStart(Node *parsetree)
* wherein event triggers are disabled. (Or we could implement
* heapscan-and-sort logic for that case, but having disaster recovery
* scenarios depend on code that's otherwise untested isn't appetizing.)
+ *
+ * Additionally, event triggers can be disabled with a superuser-only GUC
+ * to make fixing database easier as per 1 above.
*/
- if (!IsUnderPostmaster)
+ if (!IsUnderPostmaster || ignore_event_trigger_check(EVT_DDLCommandStart))
return;
runlist = EventTriggerCommonSetup(parsetree,
@@ -692,9 +699,9 @@ EventTriggerDDLCommandEnd(Node *parsetree)
/*
* See EventTriggerDDLCommandStart for a discussion about why event
- * triggers are disabled in single user mode.
+ * triggers are disabled in single user mode or via GUC.
*/
- if (!IsUnderPostmaster)
+ if (!IsUnderPostmaster || ignore_event_trigger_check(EVT_DDLCommandEnd))
return;
/*
@@ -740,9 +747,9 @@ EventTriggerSQLDrop(Node *parsetree)
/*
* See EventTriggerDDLCommandStart for a discussion about why event
- * triggers are disabled in single user mode.
+ * triggers are disabled in single user mode or via a GUC.
*/
- if (!IsUnderPostmaster)
+ if (!IsUnderPostmaster || ignore_event_trigger_check(EVT_SQLDrop))
return;
/*
@@ -811,9 +818,9 @@ EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason)
/*
* See EventTriggerDDLCommandStart for a discussion about why event
- * triggers are disabled in single user mode.
+ * triggers are disabled in single user mode or via a GUC.
*/
- if (!IsUnderPostmaster)
+ if (!IsUnderPostmaster || ignore_event_trigger_check(EVT_TableRewrite))
return;
/*
@@ -2175,3 +2182,35 @@ stringify_adefprivs_objtype(ObjectType objtype)
return "???"; /* keep compiler quiet */
}
+
+
+/*
+ * Checks whether the specified event is ignored by the ignore_event_trigger
+ * GUC or not.
+ */
+static bool
+ignore_event_trigger_check(EventTriggerEvent event)
+{
+ if (ignore_event_trigger == IGNORE_EVENT_TRIGGER_NONE)
+ return false;
+
+ if (ignore_event_trigger == IGNORE_EVENT_TRIGGER_ALL)
+ return true;
+
+ switch (event)
+ {
+ case EVT_DDLCommandStart:
+ return ignore_event_trigger == IGNORE_EVENT_TRIGGER_DDL_START;
+ case EVT_DDLCommandEnd:
+ return ignore_event_trigger == IGNORE_EVENT_TRIGGER_DDL_END;
+ case EVT_SQLDrop:
+ return ignore_event_trigger == IGNORE_EVENT_TRIGGER_SQL_DROP;
+ case EVT_TableRewrite:
+ return ignore_event_trigger == IGNORE_EVENT_TRIGGER_TABLE_REWRITE;
+
+ default:
+ elog(ERROR, "unsupported event trigger: %d", (int) event);
+ }
+
+ return false; /* unreachable */
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8062589efd..377a755782 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -37,6 +37,7 @@
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
+#include "commands/event_trigger.h"
#include "commands/tablespace.h"
#include "commands/trigger.h"
#include "commands/user.h"
@@ -471,6 +472,16 @@ static const struct config_enum_entry wal_compression_options[] = {
{NULL, 0, false}
};
+static const struct config_enum_entry ignore_event_trigger_options[] = {
+ {"none", IGNORE_EVENT_TRIGGER_NONE, false},
+ {"all", IGNORE_EVENT_TRIGGER_ALL, false},
+ {"ddl_command_start", IGNORE_EVENT_TRIGGER_DDL_START, false},
+ {"ddl_command_end", IGNORE_EVENT_TRIGGER_DDL_END, false},
+ {"table_rewrite", IGNORE_EVENT_TRIGGER_TABLE_REWRITE, false},
+ {"sql_drop", IGNORE_EVENT_TRIGGER_SQL_DROP, false},
+ {NULL, 0, false}
+};
+
/*
* Options for enum values stored in other modules
*/
@@ -4810,6 +4821,16 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"ignore_event_trigger", PGC_SUSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Disable event triggers."),
+ NULL
+ },
+ &ignore_event_trigger,
+ IGNORE_EVENT_TRIGGER_NONE, ignore_event_trigger_options,
+ NULL, NULL, NULL
+ },
+
{
{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
gettext_noop("Sets the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee49ca3937..b17a98eba5 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -706,6 +706,7 @@
#xmloption = 'content'
#gin_pending_list_limit = 4MB
#createrole_self_grant = '' # set and/or inherit
+#ignore_event_trigger = none
# - Locale and Formatting -
diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h
index 5ed6ece555..85b93a8320 100644
--- a/src/include/commands/event_trigger.h
+++ b/src/include/commands/event_trigger.h
@@ -29,6 +29,18 @@ typedef struct EventTriggerData
CommandTag tag;
} EventTriggerData;
+typedef enum ignore_event_trigger_events
+{
+ IGNORE_EVENT_TRIGGER_NONE,
+ IGNORE_EVENT_TRIGGER_DDL_START,
+ IGNORE_EVENT_TRIGGER_DDL_END,
+ IGNORE_EVENT_TRIGGER_TABLE_REWRITE,
+ IGNORE_EVENT_TRIGGER_SQL_DROP,
+ IGNORE_EVENT_TRIGGER_ALL
+} IgnoreEventTriggersEvents;
+
+extern PGDLLIMPORT int ignore_event_trigger;
+
#define AT_REWRITE_ALTER_PERSISTENCE 0x01
#define AT_REWRITE_DEFAULT_VAL 0x02
#define AT_REWRITE_COLUMN_REWRITE 0x04
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 5a10958df5..34c3b80e4f 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -614,3 +614,32 @@ SELECT
DROP EVENT TRIGGER start_rls_command;
DROP EVENT TRIGGER end_rls_command;
DROP EVENT TRIGGER sql_drop_command;
+-- Check the GUC for disabling event triggers
+CREATE FUNCTION test_event_trigger_guc() RETURNS event_trigger
+LANGUAGE plpgsql AS $$
+DECLARE
+ obj record;
+BEGIN
+ FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()
+ LOOP
+ RAISE NOTICE '% dropped %', tg_tag, obj.object_type;
+ END LOOP;
+END;
+$$;
+CREATE EVENT TRIGGER test_event_trigger_guc
+ ON sql_drop
+ WHEN TAG IN ('DROP POLICY') EXECUTE FUNCTION test_event_trigger_guc();
+SET ignore_event_trigger = 'none';
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+DROP POLICY pguc ON event_trigger_test;
+NOTICE: DROP POLICY dropped policy
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'sql_drop';
+DROP POLICY pguc ON event_trigger_test;
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'all';
+DROP POLICY pguc ON event_trigger_test;
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'ddl_command_start';
+DROP POLICY pguc ON event_trigger_test;
+NOTICE: DROP POLICY dropped policy
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 1aeaddbe71..eb18d23fd3 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -471,3 +471,35 @@ SELECT
DROP EVENT TRIGGER start_rls_command;
DROP EVENT TRIGGER end_rls_command;
DROP EVENT TRIGGER sql_drop_command;
+
+-- Check the GUC for disabling event triggers
+CREATE FUNCTION test_event_trigger_guc() RETURNS event_trigger
+LANGUAGE plpgsql AS $$
+DECLARE
+ obj record;
+BEGIN
+ FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()
+ LOOP
+ RAISE NOTICE '% dropped %', tg_tag, obj.object_type;
+ END LOOP;
+END;
+$$;
+CREATE EVENT TRIGGER test_event_trigger_guc
+ ON sql_drop
+ WHEN TAG IN ('DROP POLICY') EXECUTE FUNCTION test_event_trigger_guc();
+
+SET ignore_event_trigger = 'none';
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+DROP POLICY pguc ON event_trigger_test;
+
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'sql_drop';
+DROP POLICY pguc ON event_trigger_test;
+
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'all';
+DROP POLICY pguc ON event_trigger_test;
+
+CREATE POLICY pguc ON event_trigger_test USING (FALSE);
+SET ignore_event_trigger = 'ddl_command_start';
+DROP POLICY pguc ON event_trigger_test;
--
2.32.1 (Apple Git-133)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
@ 2023-04-03 13:09 ` Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Robert Haas @ 2023-04-03 13:09 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson <[email protected]> wrote:
> All comments above addressed in the attached v5, thanks for review!
I continue to think it's odd that the sense of this is inverted as
compared with row_security.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
@ 2023-04-03 13:15 ` Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Gustafsson @ 2023-04-03 13:15 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
> On 3 Apr 2023, at 15:09, Robert Haas <[email protected]> wrote:
>
> On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson <[email protected]> wrote:
>> All comments above addressed in the attached v5, thanks for review!
>
> I continue to think it's odd that the sense of this is inverted as
> compared with row_security.
I'm not sure I follow. Do you propose that the GUC enables classes of event
triggers, the default being "all" (or similar) and one would remove the type of
EVT for which debugging is needed? That doesn't seem like a bad idea, just one
that hasn't come up in the discussion (and I didn't think about).
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
@ 2023-04-03 14:09 ` Robert Haas <[email protected]>
2023-04-03 21:35 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Robert Haas @ 2023-04-03 14:09 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson <[email protected]> wrote:
> > On 3 Apr 2023, at 15:09, Robert Haas <[email protected]> wrote:
> > On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson <[email protected]> wrote:
> >> All comments above addressed in the attached v5, thanks for review!
> >
> > I continue to think it's odd that the sense of this is inverted as
> > compared with row_security.
>
> I'm not sure I follow. Do you propose that the GUC enables classes of event
> triggers, the default being "all" (or similar) and one would remove the type of
> EVT for which debugging is needed? That doesn't seem like a bad idea, just one
> that hasn't come up in the discussion (and I didn't think about).
Right. Although to be fair, that idea doesn't sound as good if we're
going to have settings other than "on" or "off". If this is just
disable_event_triggers = on | off, then why not flip the sense around
and make it event_triggers = off | on, just as we do for row_security?
But if we're going to allow specific types of event triggers to be
disabled, and we think it's likely that people will want to disable
one specific type of event trigger while leaving the others alone,
that might not be very convenient, because you could end up having to
list all the things you do want instead of the one thing you don't
want. On the third hand, in other contexts, I've often advocating for
giving options a positive sense (what are we going to do?) rather than
a negative sense (what are we not going to do?). For example, the
TIMING option to EXPLAIN was originally proposed with a name like
DISABLE_TIMING or something, and the value inverted, and I said let's
not do that. And similarly in other places. A case where I didn't
follow that principle is VACUUM (DISABLE_PAGE_SKIPPING) which now
seems like a wart to me. Why isn't it VACUUM (PAGE_SKIPPING) again
with the opposite value?
I'm not sure what the best thing to do is here, I just think it
deserves some thought.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
@ 2023-04-03 21:35 ` Daniel Gustafsson <[email protected]>
2023-04-05 08:10 ` Re: GUC for temporarily disabling event triggers Michael Paquier <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Gustafsson @ 2023-04-03 21:35 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
> On 3 Apr 2023, at 16:09, Robert Haas <[email protected]> wrote:
> On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson <[email protected]> wrote:
>>> On 3 Apr 2023, at 15:09, Robert Haas <[email protected]> wrote:
>>> I continue to think it's odd that the sense of this is inverted as
>>> compared with row_security.
>>
>> I'm not sure I follow. Do you propose that the GUC enables classes of event
>> triggers, the default being "all" (or similar) and one would remove the type of
>> EVT for which debugging is needed? That doesn't seem like a bad idea, just one
>> that hasn't come up in the discussion (and I didn't think about).
>
> Right. Although to be fair, that idea doesn't sound as good if we're
> going to have settings other than "on" or "off".
Yeah. The patch as it stands allow for disabling specific types rather than
all-or-nothing, which is why the name was "ignore".
> I'm not sure what the best thing to do is here, I just think it
> deserves some thought.
Absolutely, the discussion is much appreciated. Having done some thinking I
think I'm still partial to framing it as a disabling GUC rather than an
enabling; with the act of setting it being "As an admin I want to skip
execution of all evt's of type X".
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 21:35 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
@ 2023-04-05 08:10 ` Michael Paquier <[email protected]>
2023-04-05 08:57 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Michael Paquier @ 2023-04-05 08:10 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Robert Haas <[email protected]>; Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
On Mon, Apr 03, 2023 at 11:35:14PM +0200, Daniel Gustafsson wrote:
> Yeah. The patch as it stands allow for disabling specific types rather than
> all-or-nothing, which is why the name was "ignore".
FWIW, I agree with Robert's points here:
- disable_event_triggers or ignore_event_triggers = off leads to a
double-negative meaning, which is a positive. Depending on one's
native language that can be confusing.
- Being able to write a list of event triggers working would be much
more interesting than just individual elements.
- There may be an argument for negated patterns? Say,
a "!sql_drop,!ddl_command_start" would cause sql_drop and
ddl_command_start to be disabled with all the others enabled, and one
should not ne able to mix negated and non-negated patterns.
A few days before the end of the commit fest, perhaps you'd better
head towards having only an event_trigger = on | off or all | none and
consider expanding that later on? From what I get at the top of the
thread, this would satisfy the main use case you seemed to worry
about to begin with.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 21:35 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-05 08:10 ` Re: GUC for temporarily disabling event triggers Michael Paquier <[email protected]>
@ 2023-04-05 08:57 ` Daniel Gustafsson <[email protected]>
2023-04-05 14:30 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Gustafsson @ 2023-04-05 08:57 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Robert Haas <[email protected]>; Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
> On 5 Apr 2023, at 10:10, Michael Paquier <[email protected]> wrote:
>
> On Mon, Apr 03, 2023 at 11:35:14PM +0200, Daniel Gustafsson wrote:
>> Yeah. The patch as it stands allow for disabling specific types rather than
>> all-or-nothing, which is why the name was "ignore".
>
> FWIW, I agree with Robert's points here:
> - disable_event_triggers or ignore_event_triggers = off leads to a
> double-negative meaning, which is a positive. Depending on one's
> native language that can be confusing.
I agree that ignore=off would be suboptimal, but the patch doesn't have that
but instead ignore_event_trigger=none for that case, which I personally don't
think carries the same issue.
> - Being able to write a list of event triggers working would be much
> more interesting than just individual elements.
> - There may be an argument for negated patterns? Say,
> a "!sql_drop,!ddl_command_start" would cause sql_drop and
> ddl_command_start to be disabled with all the others enabled, and one
> should not ne able to mix negated and non-negated patterns.
I'm not convinced that it's in our interest to offer a GUC to configure the
cluster by selectively turning off SQL features. The ones we have for planner
tuning which is a different beast. At the very least it should be in a thread
covering that topic, as it might be a bit hidden here.
The use case envisioned here is to allow an admin to log in to a database with
a broken EVT without having to use single user mode. Basically, it should be a
convenient way of temporarily halting the execution of buggy code, not a
vehicle for permanent cluster config (even though in light of the above para
it's clear that it can be misused like that). Maybe there should be a log
event highlighting that the cluster is running with an EVT type ignored?
And/or logging the names of the EVT's that otherwise would've been executed?
> A few days before the end of the commit fest, perhaps you'd better
> head towards having only an event_trigger = on | off or all | none and
> consider expanding that later on? From what I get at the top of the
> thread, this would satisfy the main use case you seemed to worry
> about to begin with.
If there are concerns with any part of the patch at this point, and the
comments above indicate that, I'd say it's better to push this to the v17
cycle.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 21:35 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-05 08:10 ` Re: GUC for temporarily disabling event triggers Michael Paquier <[email protected]>
2023-04-05 08:57 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
@ 2023-04-05 14:30 ` Robert Haas <[email protected]>
2023-04-05 14:43 ` Re: GUC for temporarily disabling event triggers Tom Lane <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Robert Haas @ 2023-04-05 14:30 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
On Wed, Apr 5, 2023 at 4:57 AM Daniel Gustafsson <[email protected]> wrote:
> > - Being able to write a list of event triggers working would be much
> > more interesting than just individual elements.
> > - There may be an argument for negated patterns? Say,
> > a "!sql_drop,!ddl_command_start" would cause sql_drop and
> > ddl_command_start to be disabled with all the others enabled, and one
> > should not ne able to mix negated and non-negated patterns.
>
> I'm not convinced that it's in our interest to offer a GUC to configure the
> cluster by selectively turning off SQL features. The ones we have for planner
> tuning which is a different beast. At the very least it should be in a thread
> covering that topic, as it might be a bit hidden here.
But, isn't that exactly what you're proposing?
I mean if this was just event_triggers = on | off it would be exactly
like row_security and as far as I'm concerned there would be nothing
to debate. But it sounded like you wanted something finer-grained that
could disable certain kinds of event triggers. That's also what
MIchael is proposing, just with different syntax. In other words,
where you would say ignore_event_triggers = sql_drop, he'd say
event_triggers = !sql_drop.
Maybe we should back up and ask why we need more than "on" and "off".
If somebody is using this feature in any form more than very
occasionally, they should really go home and reconsider their database
schema.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: GUC for temporarily disabling event triggers
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 13:15 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
2023-04-03 21:35 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-05 08:10 ` Re: GUC for temporarily disabling event triggers Michael Paquier <[email protected]>
2023-04-05 08:57 ` Re: GUC for temporarily disabling event triggers Daniel Gustafsson <[email protected]>
2023-04-05 14:30 ` Re: GUC for temporarily disabling event triggers Robert Haas <[email protected]>
@ 2023-04-05 14:43 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Tom Lane @ 2023-04-05 14:43 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; Mikhail Gribkov <[email protected]>; [email protected]
Robert Haas <[email protected]> writes:
> Maybe we should back up and ask why we need more than "on" and "off".
> If somebody is using this feature in any form more than very
> occasionally, they should really go home and reconsider their database
> schema.
+1 ... this seems perhaps overdesigned.
regards, tom lane
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH v12 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da8..49938c9ed41 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v12-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2024-02-13 15:17 UTC | newest]
Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-02 19:48 Re: GUC for temporarily disabling event triggers Justin Pryzby <[email protected]>
2023-04-03 12:45 ` Daniel Gustafsson <[email protected]>
2023-04-03 13:09 ` Robert Haas <[email protected]>
2023-04-03 13:15 ` Daniel Gustafsson <[email protected]>
2023-04-03 14:09 ` Robert Haas <[email protected]>
2023-04-03 21:35 ` Daniel Gustafsson <[email protected]>
2023-04-05 08:10 ` Michael Paquier <[email protected]>
2023-04-05 08:57 ` Daniel Gustafsson <[email protected]>
2023-04-05 14:30 ` Robert Haas <[email protected]>
2023-04-05 14:43 ` Tom Lane <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v12 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[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