public inbox for [email protected]
help / color / mirror / Atom feedRe: Add 64-bit XIDs into PostgreSQL 15
25+ messages / 6 participants
[nested] [flat]
* Re: Add 64-bit XIDs into PostgreSQL 15
@ 2022-12-09 12:49 Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 25+ messages in thread
From: Aleksander Alekseev @ 2022-12-09 12:49 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: adherent postgres <[email protected]>; Chris Travers <[email protected]>; Chris Travers <[email protected]>; Bruce Momjian <[email protected]>
Hi adherent,
> Robertmhaas said that the project Zheap is dead（https://twitter.com/andy_pavlo/status/1590703943176589312）, which means that we cannot use Zheap to deal with the issue of xid wraparound and dead tuples in tables. The dead tuple issue is not a big deal because I can still use pg_repack to handle, although pg_repack will cause wal log to increase dramatically and may take one or two days to handle a large table. During this time the database can be accessed by external users, but the xid wraparound will cause PostgreSQL to be down, which is a disaster for DBAs. Maybe you are not a DBA, or your are from a small country, Database system tps is very low, so xid32 is enough for your database system , Oracle's scn was also 32bits, however, Oracle realized the issue and changed scn to 64 bits. The transaction id in mysql is 48 bits. MySQL didn't fix the transaction id wraparound problem because they think that 48 bits is enough for the transaction id. This project has been running for almost 1 year and now it is coming to an end. I strongly disagree with your idea of stopping this patch, and I suspect you are a saboteur. I strongly disagree with your viewpoint, as it is not a fundamental way to solve the xid wraparound problem. The PostgreSQL community urgently needs developers who solve problems like this, not bury one' head in the sand
This is not uncommon for people on the mailing list to have
disagreements. This is part of the process, we all are looking for
consensus. It's true that different people have different use cases in
mind and different backgrounds as well. It doesn't mean these use
cases are wrong and/or the experience is irrelevant and/or the
received feedback should be just discarded.
Although I also expressed my disagreement with Chris before, let's not
assume any bad intent and especially sabotage as you put it. (Unless
you have a strong proof of this of course which I doubt you have.) We
want all kinds of feedback to be welcomed here. I'm sure our goal here
is mutual, to make PostgreSQL even better than it is now. The only
problem is that the definition of "better" varies sometimes.
I see you believe that 64-bit XIDs are going to be useful. That's
great! Tell us more about your case and how the patch is going to help
with it. Also, maybe you could test your load with the applied
patchset and tell us whether it makes things better or worse?
Personally I would love hearing this from you.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add 64-bit XIDs into PostgreSQL 15
@ 2022-12-09 13:54 adherent postgres <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 2 replies; 25+ messages in thread
From: adherent postgres @ 2022-12-09 13:54 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; [email protected] <[email protected]>; +Cc: Chris Travers <[email protected]>; Chris Travers <[email protected]>; Bruce Momjian <[email protected]>
Hi Aleksander Alekseev
I think the xids 32bit transformation project has been dragged on for too long. Huawei's openGauss referenced this patch to implement xids 64bit, and Postgrespro also implemented xids 64bit, which is enough to prove that their worries are redundant.I think postgresql has no reason not to implement xid 64 bit. What about your opinion?
Best whish
________________________________
发件人: Aleksander Alekseev <[email protected]>
发送时间: 2022年12月9日 20:49
收件人: [email protected] <[email protected]>
抄送: adherent postgres <[email protected]>; Chris Travers <[email protected]>; Chris Travers <[email protected]>; Bruce Momjian <[email protected]>
主题: Re: Add 64-bit XIDs into PostgreSQL 15
Hi adherent,
> Robertmhaas said that the project Zheap is dead（https://twitter.com/andy_pavlo/status/1590703943176589312）, which means that we cannot use Zheap to deal with the issue of xid wraparound and dead tuples in tables. The dead tuple issue is not a big deal because I can still use pg_repack to handle, although pg_repack will cause wal log to increase dramatically and may take one or two days to handle a large table. During this time the database can be accessed by external users, but the xid wraparound will cause PostgreSQL to be down, which is a disaster for DBAs. Maybe you are not a DBA, or your are from a small country, Database system tps is very low, so xid32 is enough for your database system , Oracle's scn was also 32bits, however, Oracle realized the issue and changed scn to 64 bits. The transaction id in mysql is 48 bits. MySQL didn't fix the transaction id wraparound problem because they think that 48 bits is enough for the transaction id. This project has been running for almost 1 year and now it is coming to an end. I strongly disagree with your idea of stopping this patch, and I suspect you are a saboteur. I strongly disagree with your viewpoint, as it is not a fundamental way to solve the xid wraparound problem. The PostgreSQL community urgently needs developers who solve problems like this, not bury one' head in the sand
This is not uncommon for people on the mailing list to have
disagreements. This is part of the process, we all are looking for
consensus. It's true that different people have different use cases in
mind and different backgrounds as well. It doesn't mean these use
cases are wrong and/or the experience is irrelevant and/or the
received feedback should be just discarded.
Although I also expressed my disagreement with Chris before, let's not
assume any bad intent and especially sabotage as you put it. (Unless
you have a strong proof of this of course which I doubt you have.) We
want all kinds of feedback to be welcomed here. I'm sure our goal here
is mutual, to make PostgreSQL even better than it is now. The only
problem is that the definition of "better" varies sometimes.
I see you believe that 64-bit XIDs are going to be useful. That's
great! Tell us more about your case and how the patch is going to help
with it. Also, maybe you could test your load with the applied
patchset and tell us whether it makes things better or worse?
Personally I would love hearing this from you.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add 64-bit XIDs into PostgreSQL 15
@ 2022-12-09 14:13 Pavel Borisov <[email protected]>
parent: adherent postgres <[email protected]>
1 sibling, 0 replies; 25+ messages in thread
From: Pavel Borisov @ 2022-12-09 14:13 UTC (permalink / raw)
To: adherent postgres <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; [email protected] <[email protected]>; Chris Travers <[email protected]>; Chris Travers <[email protected]>; Bruce Momjian <[email protected]>
Hi, Adherent!
On Fri, 9 Dec 2022 at 17:54, adherent postgres
<[email protected]> wrote:
>
> Hi Aleksander Alekseev
> I think the xids 32bit transformation project has been dragged on for too long. Huawei's openGauss referenced this patch to implement xids 64bit, and Postgrespro also implemented xids 64bit, which is enough to prove that their worries are redundant.I think postgresql has no reason not to implement xid 64 bit. What about your opinion?
I agree it's high time to commit 64xids into PostgreSQL.
If you can do your review of the whole proposed patchset or only the
part that is likely to be committed earlier [1] it would help a lot!
I'd recommend beginning with the last version of the patch in thread
[1]. First, it is easier. Also, this review is going to be useful
sooner and will help a committer on January commitfest a lot.
[1]: https://www.postgresql.org/message-id/CAFiTN-uudj2PY8GsUzFtLYFpBoq_rKegW3On_8ZHdxB1mVv3-A%40mail.gma...
Regards,
Pavel Borisov,
Supabase
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Add 64-bit XIDs into PostgreSQL 15
@ 2022-12-09 14:29 Maxim Orlov <[email protected]>
parent: adherent postgres <[email protected]>
1 sibling, 0 replies; 25+ messages in thread
From: Maxim Orlov @ 2022-12-09 14:29 UTC (permalink / raw)
To: adherent postgres <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; [email protected] <[email protected]>; Chris Travers <[email protected]>; Chris Travers <[email protected]>; Bruce Momjian <[email protected]>
On Fri, 9 Dec 2022 at 16:54, adherent postgres <
[email protected]> wrote:
> Hi Aleksander Alekseev
> I think the xids 32bit transformation project has been dragged on for too
> long. Huawei's openGauss referenced this patch to implement xids 64bit, and
> Postgrespro also implemented xids 64bit, which is enough to prove that
> their worries are redundant.I think postgresql has no reason not to
> implement xid 64 bit. What about your opinion?
>
Yeah, I totally agree, the time has come. With a high transaction load,
Postgres become more and more difficult to maintain.
The problem is in the overall complexity of the patch set. We need more
reviewers.
Since committing such a big patch is not viable once at a time, from the
start of the work we did split it into several logical parts.
The evolution concept is more preferable in this case. As far as I can see,
there is overall consensus to commit SLRU related
changes first.
--
Best regards,
Maxim Orlov.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
@ 2023-07-05 20:53 Tristan Partin <[email protected]>
0 siblings, 1 reply; 25+ messages in thread
From: Tristan Partin @ 2023-07-05 20:53 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; joe@cd; Heikki Linnakangas <[email protected]>; Tom Lane <[email protected]>; [email protected]; +Cc: pgsql-hackers
On Mon Jul 3, 2023 at 9:42 AM CDT, Tristan Partin wrote:
> Thanks for your patience. Attached is a patch that should cover all the
> problematic use cases of setlocale(). There are some setlocale() calls in
> tests, initdb, and ecpg left. I plan to get to ecpglib before the final
> version of this patch after I abstract over Windows not having
> uselocale(). I think leaving initdb and tests as is would be fine, but I
> am also happy to just permanently purge setlocale() from the codebase
> if people see value in that. We could also poison[0] setlocale() at that
> point.
>
> [0]: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
Here is a v2 with best effort Windows support. My patch currently
assumes that you either have uselocale() or are Windows. I dropped the
environment variable hacks, but could bring them back if we didn't like
this requirement.
I tried to add an email[0] to discuss this with hackers, but failed to add
the CC. Let's discuss here instead given my complete inability to manage
mailing lists :).
[0]: https://www.postgresql.org/message-id/CTUJ604ZWHI1.3PFZK152XCWLX%40gonk
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
@ 2023-07-05 20:55 Tristan Partin <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 25+ messages in thread
From: Tristan Partin @ 2023-07-05 20:55 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; joe@cd; Heikki Linnakangas <[email protected]>; Tom Lane <[email protected]>; [email protected]; +Cc: pgsql-hackers
Someday I will learn...
Attached is the v2.
--
Tristan Partin
Neon (https://neon.tech)
Attachments:
[text/x-patch] v2-0001-Skip-checking-for-uselocale-on-Windows.patch (677B, ../../CTUJD214VCXS.1SD6HDEQMICD0@gonk/2-v2-0001-Skip-checking-for-uselocale-on-Windows.patch)
download | inline diff:
From 5688bc2b2c6331f437a72b6a429199c5416ccd76 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Fri, 30 Jun 2023 09:31:04 -0500
Subject: [PATCH v2 1/3] Skip checking for uselocale on Windows
Windows doesn't have uselocale, so skip it.
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index fbec997947..df76f10822 100644
--- a/meson.build
+++ b/meson.build
@@ -2439,7 +2439,7 @@ func_checks = [
['strsignal'],
['sync_file_range'],
['syncfs'],
- ['uselocale'],
+ ['uselocale', {'skip': host_system == 'windows'}],
['wcstombs_l'],
]
--
Tristan Partin
Neon (https://neon.tech)
[text/x-patch] v2-0002-Add-locale_is_c-function.patch (4.5K, ../../CTUJD214VCXS.1SD6HDEQMICD0@gonk/3-v2-0002-Add-locale_is_c-function.patch)
download | inline diff:
From d52ab7851e9638e36cd99859014d7aa31154e600 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Fri, 30 Jun 2023 11:13:29 -0500
Subject: [PATCH v2 2/3] Add locale_is_c function
In some places throughout the codebase, there are string comparisons for
locales matching C or POSIX. Encapsulate this logic into a single
function and use it.
---
src/backend/utils/adt/pg_locale.c | 37 ++++++++++++++++++-------------
src/backend/utils/init/postinit.c | 4 +---
src/backend/utils/mb/mbutils.c | 5 ++---
src/include/utils/pg_locale.h | 1 +
4 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 514d4a9eeb..b455ef3aff 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -154,6 +154,21 @@ static void icu_set_collation_attributes(UCollator *collator, const char *loc,
UErrorCode *status);
#endif
+/*
+ * Check if a locale is the C locale
+ *
+ * POSIX is an alias for C. Passing ingore_case as true will use
+ * pg_strcasecmp() instead of strcmp().
+ */
+bool
+locale_is_c(const char *locale, bool ignore_case)
+{
+ if (!ignore_case)
+ return strcmp(locale, "C") == 0 || strcmp(locale, "POSIX") == 0;
+
+ return pg_strcasecmp(locale, "C") == 0 || pg_strcasecmp(locale, "POSIX") == 0;
+}
+
/*
* pg_perm_setlocale
*
@@ -1239,10 +1254,8 @@ lookup_collation_cache(Oid collation, bool set_flags)
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
collctype = TextDatumGetCString(datum);
- cache_entry->collate_is_c = ((strcmp(collcollate, "C") == 0) ||
- (strcmp(collcollate, "POSIX") == 0));
- cache_entry->ctype_is_c = ((strcmp(collctype, "C") == 0) ||
- (strcmp(collctype, "POSIX") == 0));
+ cache_entry->collate_is_c = locale_is_c(collcollate, false);
+ cache_entry->ctype_is_c = locale_is_c(collctype, false);
}
else
{
@@ -1290,12 +1303,8 @@ lc_collate_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_COLLATE setting");
- if (strcmp(localeptr, "C") == 0)
- result = true;
- else if (strcmp(localeptr, "POSIX") == 0)
- result = true;
- else
- result = false;
+ result = locale_is_c(localeptr, false);
+
return (bool) result;
}
@@ -1343,12 +1352,8 @@ lc_ctype_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_CTYPE setting");
- if (strcmp(localeptr, "C") == 0)
- result = true;
- else if (strcmp(localeptr, "POSIX") == 0)
- result = true;
- else
- result = false;
+ result = locale_is_c(localeptr, false);
+
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 0f9b92b32e..a92a0c438f 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -419,9 +419,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
" which is not recognized by setlocale().", ctype),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (strcmp(ctype, "C") == 0 ||
- strcmp(ctype, "POSIX") == 0)
- database_ctype_is_c = true;
+ database_ctype_is_c = locale_is_c(ctype, false);
if (dbform->datlocprovider == COLLPROVIDER_ICU)
{
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 67a1ab2ab2..997156515c 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -39,6 +39,7 @@
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
+#include "utils/pg_locale.h"
#include "utils/syscache.h"
#include "varatt.h"
@@ -1237,9 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- const char *ctype = setlocale(LC_CTYPE, NULL);
-
- if (pg_strcasecmp(ctype, "C") == 0 || pg_strcasecmp(ctype, "POSIX") == 0)
+ if (locale_is_c(locale_ctype, true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index e2a7243542..872bef798a 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -54,6 +54,7 @@ extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
extern char *pg_perm_setlocale(int category, const char *locale);
+extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
extern bool lc_ctype_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
[text/x-patch] v2-0003-Use-thread-safe-locale-APIs.patch (27.7K, ../../CTUJD214VCXS.1SD6HDEQMICD0@gonk/4-v2-0003-Use-thread-safe-locale-APIs.patch)
download | inline diff:
From 777a30e5c5f04fa433ea7e52cce8a0516223ace3 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Mon, 3 Jul 2023 08:51:59 -0500
Subject: [PATCH v2 3/3] Use thread-safe locale APIs
setlocale() has been a thorn in Postgres's side for a while. Most
recently this has manifested in bug #17946 where loading libperl would
cause the localization to change out from under Postgres. On Windows,
use _ENABLE_PER_THREAD_LOCALE.
---
src/backend/main/main.c | 55 +++-
src/backend/utils/adt/cash.c | 29 +++
src/backend/utils/adt/formatting.c | 11 +-
src/backend/utils/adt/pg_locale.c | 389 ++++++++++++++++++-----------
src/backend/utils/init/postinit.c | 4 +-
src/backend/utils/mb/mbutils.c | 2 +-
src/common/exec.c | 16 --
src/include/utils/pg_locale.h | 2 +-
8 files changed, 326 insertions(+), 182 deletions(-)
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index ed11e8be7f..093df88f14 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -59,6 +59,11 @@ int
main(int argc, char *argv[])
{
bool do_check_root = true;
+ char *collate_locale;
+ char *ctype_locale;
+#ifdef LC_MESSAGES
+ char *messages_locale;
+#endif
reached_main = true;
@@ -111,15 +116,39 @@ main(int argc, char *argv[])
* these set to "C" then message localization might not work well in the
* postmaster.
*/
- init_locale("LC_COLLATE", LC_COLLATE, "");
- init_locale("LC_CTYPE", LC_CTYPE, "");
+
+ /*
+ * Save off the original locale values prior to the first call of
+ * uselocale(). This ensures that we don't call pg_setlocale() with an empty
+ * string. See the function comment for pg_setlocale() for further
+ * explanation. Note that this restriction doesn't exist on Windows, but we
+ * can use the same code path anyway.
+ */
+ collate_locale = setlocale(LC_COLLATE, "");
+ ctype_locale = setlocale(LC_CTYPE, "");
+
+#ifdef LC_MESSAGES
+ messages_locale = setlocale(LC_MESSAGES, "");
+#endif
+
+
+#ifdef HAVE__CONFIGTHREADLOCALE
+ /*
+ * This call could most likely happen sooner, but just to err on the side of
+ * caution, call it after absorbing locales from the environment.
+ */
+ _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
+#endif
+
+ init_locale("LC_COLLATE", LC_COLLATE, collate_locale);
+ init_locale("LC_CTYPE", LC_CTYPE, ctype_locale);
/*
* LC_MESSAGES will get set later during GUC option processing, but we set
* it here to allow startup error messages to be localized.
*/
#ifdef LC_MESSAGES
- init_locale("LC_MESSAGES", LC_MESSAGES, "");
+ init_locale("LC_MESSAGES", LC_MESSAGES, messages_locale);
#endif
/*
@@ -130,13 +159,6 @@ main(int argc, char *argv[])
init_locale("LC_NUMERIC", LC_NUMERIC, "C");
init_locale("LC_TIME", LC_TIME, "C");
- /*
- * Now that we have absorbed as much as we wish to from the locale
- * environment, remove any LC_ALL setting, so that the environment
- * variables installed by pg_perm_setlocale have force.
- */
- unsetenv("LC_ALL");
-
/*
* Catch standard options before doing much else, in particular before we
* insist on not being root.
@@ -307,8 +329,17 @@ startup_hacks(const char *progname)
static void
init_locale(const char *categoryname, int category, const char *locale)
{
- if (pg_perm_setlocale(category, locale) == NULL &&
- pg_perm_setlocale(category, "C") == NULL)
+ /*
+ * Since we are initializing global locales, NULL and empty string are
+ * invalid arguments. See the pg_setlocale() function description for more
+ * explanation on the empty string. NULL just doesn't make sense in this
+ * context of initializing locales.
+ */
+ Assert(locale);
+ Assert(locale[0] != '\0');
+
+ if (pg_setlocale(category, locale) == NULL &&
+ pg_setlocale(category, "C") == NULL)
elog(FATAL, "could not adopt \"%s\" locale nor C locale for %s",
locale, categoryname);
}
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 32fbad2f57..5c254d912f 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -111,6 +111,11 @@ cash_in(PG_FUNCTION_ARGS)
*csymbol;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(escontext, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/*
* frac_digits will be CHAR_MAX in some locales, notably C. However, just
* testing for == CHAR_MAX is risky, because of compilers like gcc that
@@ -325,6 +330,11 @@ cash_out(PG_FUNCTION_ARGS)
sep_by_space;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
points = lconvert->frac_digits;
if (points < 0 || points > 10)
@@ -1036,6 +1046,11 @@ cash_numeric(PG_FUNCTION_ARGS)
int fpoint;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1095,6 +1110,11 @@ numeric_cash(PG_FUNCTION_ARGS)
Datum numeric_scale;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1128,6 +1148,11 @@ int4_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1158,6 +1183,10 @@ int8_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index e6246dc44b..b4229d075e 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -5067,12 +5067,12 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Positive / Negative number sign
*/
- if (lconv->negative_sign && *lconv->negative_sign)
+ if (lconv && lconv->negative_sign && *lconv->negative_sign)
Np->L_negative_sign = lconv->negative_sign;
else
Np->L_negative_sign = "-";
- if (lconv->positive_sign && *lconv->positive_sign)
+ if (lconv && lconv->positive_sign && *lconv->positive_sign)
Np->L_positive_sign = lconv->positive_sign;
else
Np->L_positive_sign = "+";
@@ -5080,9 +5080,8 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Number decimal point
*/
- if (lconv->decimal_point && *lconv->decimal_point)
+ if (lconv && lconv->decimal_point && *lconv->decimal_point)
Np->decimal = lconv->decimal_point;
-
else
Np->decimal = ".";
@@ -5096,7 +5095,7 @@ NUM_prepare_locale(NUMProc *Np)
* but "" for thousands_sep, so we set the thousands_sep too.
* http://archives.postgresql.org/pgsql-hackers/2007-11/msg00772.php
*/
- if (lconv->thousands_sep && *lconv->thousands_sep)
+ if (lconv && lconv->thousands_sep && *lconv->thousands_sep)
Np->L_thousands_sep = lconv->thousands_sep;
/* Make sure thousands separator doesn't match decimal point symbol. */
else if (strcmp(Np->decimal, ",") != 0)
@@ -5107,7 +5106,7 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Currency symbol
*/
- if (lconv->currency_symbol && *lconv->currency_symbol)
+ if (lconv && lconv->currency_symbol && *lconv->currency_symbol)
Np->L_currency_symbol = lconv->currency_symbol;
else
Np->L_currency_symbol = " ";
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index b455ef3aff..d3ca8c4b2d 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -98,6 +98,23 @@ char *locale_time;
int icu_validation_level = WARNING;
+#ifdef HAVE_USELOCALE
+/*
+ * Current locale settings
+ *
+ * uselocale() and friends don't have an equivalent to setlocale(cat, NULL), so
+ * as we set locales for various categories, stash them for retrieval as needed.
+ */
+static struct {
+ char *collate;
+ char *ctype;
+ char *messages;
+ char *monetary;
+ char *numeric;
+ char *time;
+} curr_locales = { 0 };
+#endif /* HAVE_USELOCALE */
+
/*
* lc_time localization cache.
*
@@ -169,28 +186,146 @@ locale_is_c(const char *locale, bool ignore_case)
return pg_strcasecmp(locale, "C") == 0 || pg_strcasecmp(locale, "POSIX") == 0;
}
+
+#ifdef HAVE_USELOCALE
/*
- * pg_perm_setlocale
- *
- * This wraps the libc function setlocale(), with two additions. First, when
- * changing LC_CTYPE, update gettext's encoding for the current message
- * domain. GNU gettext automatically tracks LC_CTYPE on most platforms, but
- * not on Windows. Second, if the operation is successful, the corresponding
- * LC_XXX environment variable is set to match. By setting the environment
- * variable, we ensure that any subsequent use of setlocale(..., "") will
- * preserve the settings made through this routine. Of course, LC_ALL must
- * also be unset to fully ensure that, but that has to be done elsewhere after
- * all the individual LC_XXX variables have been set correctly. (Thank you
- * Perl for making this kluge necessary.)
+ * Turns locale categories into their mask equivalents for use in newlocale(3)
+ *
+ * GCC has the masks defined as (1 << category), but this isn't guaranteed to be
+ * the same in other libcs.
+ */
+static int
+category_to_mask(int category)
+{
+ switch (category) {
+ case LC_ALL:
+ return LC_ALL_MASK;
+ case LC_COLLATE:
+ return LC_COLLATE_MASK;
+ case LC_CTYPE:
+ return LC_CTYPE_MASK;
+#ifdef LC_MESSAGES
+ case LC_MESSAGES:
+ return LC_MESSAGES_MASK;
+#endif
+ case LC_MONETARY:
+ return LC_MONETARY_MASK;
+ case LC_NUMERIC:
+ return LC_NUMERIC_MASK;
+ case LC_TIME:
+ return LC_TIME_MASK;
+ }
+
+ pg_unreachable();
+}
+
+
+static char *
+pg_getlocale(int category)
+{
+ switch (category) {
+ case LC_COLLATE:
+ return curr_locales.collate;
+ case LC_CTYPE:
+ return curr_locales.ctype;
+ case LC_MESSAGES:
+ return curr_locales.messages;
+ case LC_MONETARY:
+ return curr_locales.monetary;
+ case LC_NUMERIC:
+ return curr_locales.numeric;
+ case LC_TIME:
+ return curr_locales.time;
+ default:
+ pg_unreachable();
+ }
+}
+#endif /* HAVE_USELOCALE */
+
+
+/*
+ * pg_setlocale
+ *
+ * This wraps the libc functions uselocale() or setlocale() (depending on the
+ * platform), with a single addition. When changing LC_CTYPE, update gettext's
+ * encoding for the current message domain. GNU gettext automatically tracks
+ * LC_CTYPE on most platforms, but not on Windows. This function will abort if
+ * locale is the empty string. A workaround is to pass the output of
+ * setlocale(category, "") to the locale argument.
*/
char *
-pg_perm_setlocale(int category, const char *locale)
+pg_setlocale(int category, const char *locale)
{
char *result;
- const char *envvar;
+#ifdef HAVE_USELOCALE
+ char **save;
+ int category_mask;
+ locale_t prev_locale;
+ locale_t temp_locale;
+ locale_t work_locale;
+
+ if (locale == NULL)
+ return pg_getlocale(category);
+
+ Assert(locale[0] != '\0');
+
+ category_mask = category_to_mask(category);
+
+ prev_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
+
+ result = strdup(locale);
+ work_locale = duplocale(prev_locale);
+ if (!result || work_locale == (locale_t) 0) {
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ return NULL;
+ }
+
+ temp_locale = newlocale(category_mask, locale, work_locale);
+ if (temp_locale == (locale_t) 0) {
+ freelocale(work_locale);
+
+ return NULL; /* fall out immediately on failure */
+ }
+ work_locale = temp_locale;
+
+ uselocale(work_locale);
+ if (prev_locale != LC_GLOBAL_LOCALE)
+ freelocale(prev_locale);
+
+ /*
+ * Copying the locale must occur before we potentially bind the text domain.
+ */
+ switch (category) {
+ case LC_COLLATE:
+ save = &curr_locales.collate;
+ break;
+ case LC_CTYPE:
+ save = &curr_locales.ctype;
+ break;
+ case LC_MESSAGES:
+ save = &curr_locales.messages;
+ break;
+ case LC_MONETARY:
+ save = &curr_locales.monetary;
+ break;
+ case LC_NUMERIC:
+ save = &curr_locales.numeric;
+ break;
+ case LC_TIME:
+ save = &curr_locales.time;
+ break;
+ default:
+ pg_unreachable();
+ }
+
+ /* Free old locale string, and stash the new one */
+ free(*save);
+ *save = result;
-#ifndef WIN32
- result = setlocale(category, locale);
#else
/*
@@ -209,11 +344,12 @@ pg_perm_setlocale(int category, const char *locale)
else
#endif
result = setlocale(category, locale);
-#endif /* WIN32 */
if (result == NULL)
return result; /* fall out immediately on failure */
+#endif /* HAVE_USELOCALE */
+
/*
* Use the right encoding in translated messages. Under ENABLE_NLS, let
* pg_bind_textdomain_codeset() figure it out. Under !ENABLE_NLS, message
@@ -223,11 +359,13 @@ pg_perm_setlocale(int category, const char *locale)
*/
if (category == LC_CTYPE)
{
+#ifndef HAVE_USELOCALE
static char save_lc_ctype[LOCALE_NAME_BUFLEN];
/* copy setlocale() return value before callee invokes it again */
strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));
result = save_lc_ctype;
+#endif
#ifdef ENABLE_NLS
SetMessageEncoding(pg_bind_textdomain_codeset(textdomain(NULL)));
@@ -236,42 +374,6 @@ pg_perm_setlocale(int category, const char *locale)
#endif
}
- switch (category)
- {
- case LC_COLLATE:
- envvar = "LC_COLLATE";
- break;
- case LC_CTYPE:
- envvar = "LC_CTYPE";
- break;
-#ifdef LC_MESSAGES
- case LC_MESSAGES:
- envvar = "LC_MESSAGES";
-#ifdef WIN32
- result = IsoLocaleName(locale);
- if (result == NULL)
- result = (char *) locale;
- elog(DEBUG3, "IsoLocaleName() executed; locale: \"%s\"", result);
-#endif /* WIN32 */
- break;
-#endif /* LC_MESSAGES */
- case LC_MONETARY:
- envvar = "LC_MONETARY";
- break;
- case LC_NUMERIC:
- envvar = "LC_NUMERIC";
- break;
- case LC_TIME:
- envvar = "LC_TIME";
- break;
- default:
- elog(FATAL, "unrecognized LC category: %d", category);
- return NULL; /* keep compiler quiet */
- }
-
- if (setenv(envvar, result, 1) != 0)
- return NULL;
-
return result;
}
@@ -289,6 +391,47 @@ pg_perm_setlocale(int category, const char *locale)
bool
check_locale(int category, const char *locale, char **canonname)
{
+#ifdef HAVE_USELOCALE
+ locale_t saved_locale;
+ locale_t temp_locale;
+ locale_t work_locale;
+ int category_mask;
+
+ category_mask = category_to_mask(category);
+
+ if (canonname)
+ *canonname = NULL; /* in case of failure */
+
+ saved_locale = uselocale((locale_t) 0);
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0) {
+ if (errno == ENOMEM)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ return false;
+ }
+
+ temp_locale = newlocale(category_mask, locale, work_locale);
+ if (temp_locale == (locale_t) 0) {
+ freelocale(work_locale);
+
+ return false;
+ }
+ work_locale = temp_locale;
+
+ uselocale(work_locale);
+ if (canonname)
+ *canonname = pstrdup(locale); /* TODO: This does not handle "" as the locale */
+
+ uselocale(saved_locale);
+ freelocale(work_locale);
+
+ return true;
+
+#else
+
char *save;
char *res;
@@ -315,6 +458,7 @@ check_locale(int category, const char *locale, char **canonname)
pfree(save);
return (res != NULL);
+#endif /* HAVE_USELOCALE */
}
@@ -327,7 +471,7 @@ check_locale(int category, const char *locale, char **canonname)
*
* Note: we accept value = "" as selecting the postmaster's environment
* value, whatever it was (so long as the environment setting is legal).
- * This will have been locked down by an earlier call to pg_perm_setlocale.
+ * This will have been locked down by an earlier call to pg_setlocale.
*/
bool
check_locale_monetary(char **newval, void **extra, GucSource source)
@@ -406,7 +550,7 @@ assign_locale_messages(const char *newval, void *extra)
* We ignore failure, as per comment above.
*/
#ifdef LC_MESSAGES
- (void) pg_perm_setlocale(LC_MESSAGES, newval);
+ (void) pg_setlocale(LC_MESSAGES, newval);
#endif
}
@@ -502,11 +646,8 @@ PGLC_localeconv(void)
static bool CurrentLocaleConvAllocated = false;
struct lconv *extlconv;
struct lconv worklconv;
- char *save_lc_monetary;
- char *save_lc_numeric;
-#ifdef WIN32
- char *save_lc_ctype;
-#endif
+ locale_t saved_locale;
+ locale_t work_locale;
/* Did we do it already? */
if (CurrentLocaleConvValid)
@@ -533,16 +674,15 @@ PGLC_localeconv(void)
*/
memset(&worklconv, 0, sizeof(worklconv));
- /* Save prevailing values of monetary and numeric locales */
- save_lc_monetary = setlocale(LC_MONETARY, NULL);
- if (!save_lc_monetary)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_monetary = pstrdup(save_lc_monetary);
+ /* Save prevailing locale */
+ saved_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
- save_lc_numeric = setlocale(LC_NUMERIC, NULL);
- if (!save_lc_numeric)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_numeric = pstrdup(save_lc_numeric);
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
#ifdef WIN32
@@ -560,37 +700,37 @@ PGLC_localeconv(void)
* results. Hence, we must temporarily set that category as well.
*/
- /* Save prevailing value of ctype locale */
- save_lc_ctype = setlocale(LC_CTYPE, NULL);
- if (!save_lc_ctype)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_ctype = pstrdup(save_lc_ctype);
-
/* Here begins the critical section where we must not throw error */
/* use numeric to set the ctype */
- setlocale(LC_CTYPE, locale_numeric);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_numeric, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for numeric */
- setlocale(LC_NUMERIC, locale_numeric);
+ work_locale = newlocale(LC_NUMERIC_MASK, locale_numeric, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */
+ /* Must copy data now in case updating the locale overwrites it */
worklconv.decimal_point = strdup(extlconv->decimal_point);
worklconv.thousands_sep = strdup(extlconv->thousands_sep);
worklconv.grouping = strdup(extlconv->grouping);
#ifdef WIN32
/* use monetary to set the ctype */
- setlocale(LC_CTYPE, locale_monetary);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_monetary, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for monetary */
- setlocale(LC_MONETARY, locale_monetary);
+ work_locale = newlocale(LC_MONETARY_MASK, locale_monetary, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */
+ /* Must copy data now in case updating the locale overwrites it */
worklconv.int_curr_symbol = strdup(extlconv->int_curr_symbol);
worklconv.currency_symbol = strdup(extlconv->currency_symbol);
worklconv.mon_decimal_point = strdup(extlconv->mon_decimal_point);
@@ -608,22 +748,9 @@ PGLC_localeconv(void)
worklconv.p_sign_posn = extlconv->p_sign_posn;
worklconv.n_sign_posn = extlconv->n_sign_posn;
- /*
- * Restore the prevailing locale settings; failure to do so is fatal.
- * Possibly we could limp along with nondefault LC_MONETARY or LC_NUMERIC,
- * but proceeding with the wrong value of LC_CTYPE would certainly be bad
- * news; and considering that the prevailing LC_MONETARY and LC_NUMERIC
- * are almost certainly "C", there's really no reason that restoring those
- * should fail.
- */
-#ifdef WIN32
- if (!setlocale(LC_CTYPE, save_lc_ctype))
- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);
-#endif
- if (!setlocale(LC_MONETARY, save_lc_monetary))
- elog(FATAL, "failed to restore LC_MONETARY to \"%s\"", save_lc_monetary);
- if (!setlocale(LC_NUMERIC, save_lc_numeric))
- elog(FATAL, "failed to restore LC_NUMERIC to \"%s\"", save_lc_numeric);
+ /* Restore the prevailing locale settings; failure to do so is fatal. */
+ uselocale(saved_locale);
+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can call functions
@@ -634,13 +761,6 @@ PGLC_localeconv(void)
{
int encoding;
- /* Release the pstrdup'd locale names */
- pfree(save_lc_monetary);
- pfree(save_lc_numeric);
-#ifdef WIN32
- pfree(save_lc_ctype);
-#endif
-
/* If any of the preceding strdup calls failed, complain now. */
if (!struct_lconv_is_valid(&worklconv))
ereport(ERROR,
@@ -787,10 +907,8 @@ cache_locale_time(void)
bool strftimefail = false;
int encoding;
int i;
- char *save_lc_time;
-#ifdef WIN32
- char *save_lc_ctype;
-#endif
+ locale_t saved_locale;
+ locale_t work_locale;
/* did we do this already? */
if (CurrentLCTimeValid)
@@ -805,11 +923,15 @@ cache_locale_time(void)
* results afterwards.
*/
- /* Save prevailing value of time locale */
- save_lc_time = setlocale(LC_TIME, NULL);
- if (!save_lc_time)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_time = pstrdup(save_lc_time);
+ /* Save prevailing locale */
+ saved_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
+
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
#ifdef WIN32
@@ -820,17 +942,14 @@ cache_locale_time(void)
* the encoding we'll get back from strftime_win32().
*/
- /* Save prevailing value of ctype locale */
- save_lc_ctype = setlocale(LC_CTYPE, NULL);
- if (!save_lc_ctype)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_ctype = pstrdup(save_lc_ctype);
-
/* use lc_time to set the ctype */
- setlocale(LC_CTYPE, locale_time);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_time, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
- setlocale(LC_TIME, locale_time);
+ work_locale = newlocale(LC_TIME_MASK, locale_time, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
/* We use times close to current time as data for strftime(). */
timenow = time(NULL);
@@ -878,12 +997,8 @@ cache_locale_time(void)
* Restore the prevailing locale settings; as in PGLC_localeconv(),
* failure to do so is fatal.
*/
-#ifdef WIN32
- if (!setlocale(LC_CTYPE, save_lc_ctype))
- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);
-#endif
- if (!setlocale(LC_TIME, save_lc_time))
- elog(FATAL, "failed to restore LC_TIME to \"%s\"", save_lc_time);
+ uselocale(saved_locale);
+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can throw errors, or
@@ -892,12 +1007,6 @@ cache_locale_time(void)
if (strftimefail)
elog(ERROR, "strftime() failed: %m");
- /* Release the pstrdup'd locale names */
- pfree(save_lc_time);
-#ifdef WIN32
- pfree(save_lc_ctype);
-#endif
-
#ifndef WIN32
/*
@@ -1292,18 +1401,14 @@ lc_collate_is_c(Oid collation)
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_COLLATE, NULL);
- if (!localeptr)
- elog(ERROR, "invalid LC_COLLATE setting");
- result = locale_is_c(localeptr, false);
+ result = locale_is_c(curr_locales.collate, false);
return (bool) result;
}
@@ -1336,23 +1441,19 @@ lc_ctype_is_c(Oid collation)
/*
* If we're asked about the default collation, we have to inquire of the C
- * library. Cache the result so we only have to compute it once.
+ * library.
*/
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_CTYPE, NULL);
- if (!localeptr)
- elog(ERROR, "invalid LC_CTYPE setting");
- result = locale_is_c(localeptr, false);
+ result = locale_is_c(curr_locales.ctype, false);
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index a92a0c438f..0741f5666b 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -405,14 +405,14 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datctype);
ctype = TextDatumGetCString(datum);
- if (pg_perm_setlocale(LC_COLLATE, collate) == NULL)
+ if (pg_setlocale(LC_COLLATE, collate) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_COLLATE \"%s\", "
" which is not recognized by setlocale().", collate),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (pg_perm_setlocale(LC_CTYPE, ctype) == NULL)
+ if (pg_setlocale(LC_CTYPE, ctype) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_CTYPE \"%s\", "
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 997156515c..8dc7da47bd 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1238,7 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- if (locale_is_c(locale_ctype, true))
+ if (locale_is_c(pg_setlocale(LC_CTYPE, NULL), true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..f7a9643be4 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -438,22 +438,6 @@ set_pglocale_pgservice(const char *argv0, const char *app)
char path[MAXPGPATH];
char my_exec_path[MAXPGPATH];
- /* don't set LC_ALL in the backend */
- if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0)
- {
- setlocale(LC_ALL, "");
-
- /*
- * One could make a case for reproducing here PostmasterMain()'s test
- * for whether the process is multithreaded. Unlike the postmaster,
- * no frontend program calls sigprocmask() or otherwise provides for
- * mutual exclusion between signal handlers. While frontends using
- * fork(), if multithreaded, are formally exposed to undefined
- * behavior, we have not witnessed a concrete bug. Therefore,
- * complaining about multithreading here may be mere pedantry.
- */
- }
-
if (find_my_exec(argv0, my_exec_path) < 0)
return;
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index 872bef798a..60682051cb 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -52,7 +52,7 @@ extern PGDLLIMPORT char *localized_full_months[];
extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
-extern char *pg_perm_setlocale(int category, const char *locale);
+extern char *pg_setlocale(int category, const char *locale);
extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
@ 2023-07-11 00:52 Tristan Partin <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Tristan Partin @ 2023-07-11 00:52 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; joe@cd; Heikki Linnakangas <[email protected]>; Tom Lane <[email protected]>; [email protected]; +Cc: pgsql-hackers
Here is an up to date patch given some churn on the master branch.
--
Tristan Partin
Neon (https://neon.tech)
Attachments:
[text/x-patch] v3-0001-Skip-checking-for-uselocale-on-Windows.patch (677B, ../../CTYXENTV385K.18K3T1Q7IVAB7@gonk/2-v3-0001-Skip-checking-for-uselocale-on-Windows.patch)
download | inline diff:
From b68cec481768c7c635ec48329b4764eced264572 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Fri, 30 Jun 2023 09:31:04 -0500
Subject: [PATCH v3 1/3] Skip checking for uselocale on Windows
Windows doesn't have uselocale, so skip it.
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 0c44f19cb9..3356f72bf0 100644
--- a/meson.build
+++ b/meson.build
@@ -2438,7 +2438,7 @@ func_checks = [
['strsignal'],
['sync_file_range'],
['syncfs'],
- ['uselocale'],
+ ['uselocale', {'skip': host_system == 'windows'}],
['wcstombs_l'],
]
--
Tristan Partin
Neon (https://neon.tech)
[text/x-patch] v3-0002-Add-locale_is_c-function.patch (4.5K, ../../CTYXENTV385K.18K3T1Q7IVAB7@gonk/3-v3-0002-Add-locale_is_c-function.patch)
download | inline diff:
From 3207694a1d214a7d5b844f3f6dfd8378408172af Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Fri, 30 Jun 2023 11:13:29 -0500
Subject: [PATCH v3 2/3] Add locale_is_c function
In some places throughout the codebase, there are string comparisons for
locales matching C or POSIX. Encapsulate this logic into a single
function and use it.
---
src/backend/utils/adt/pg_locale.c | 39 ++++++++++++++++++-------------
src/backend/utils/init/postinit.c | 4 +---
src/backend/utils/mb/mbutils.c | 5 ++--
src/include/utils/pg_locale.h | 1 +
4 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index aa9da99308..047c02dbab 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -191,6 +191,23 @@ wcstombs_l(char *dest, const wchar_t *src, size_t n, locale_t loc)
}
#endif
+
+/*
+ * Check if a locale is the C locale
+ *
+ * POSIX is an alias for C. Passing ingore_case as true will use
+ * pg_strcasecmp() instead of strcmp().
+ */
+bool
+locale_is_c(const char *locale, bool ignore_case)
+{
+ if (!ignore_case)
+ return strcmp(locale, "C") == 0 || strcmp(locale, "POSIX") == 0;
+
+ return pg_strcasecmp(locale, "C") == 0 || pg_strcasecmp(locale, "POSIX") == 0;
+}
+
+
/*
* pg_perm_setlocale
*
@@ -1276,10 +1293,8 @@ lookup_collation_cache(Oid collation, bool set_flags)
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
collctype = TextDatumGetCString(datum);
- cache_entry->collate_is_c = ((strcmp(collcollate, "C") == 0) ||
- (strcmp(collcollate, "POSIX") == 0));
- cache_entry->ctype_is_c = ((strcmp(collctype, "C") == 0) ||
- (strcmp(collctype, "POSIX") == 0));
+ cache_entry->collate_is_c = locale_is_c(collcollate, false);
+ cache_entry->ctype_is_c = locale_is_c(collctype, false);
}
else
{
@@ -1327,12 +1342,8 @@ lc_collate_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_COLLATE setting");
- if (strcmp(localeptr, "C") == 0)
- result = true;
- else if (strcmp(localeptr, "POSIX") == 0)
- result = true;
- else
- result = false;
+ result = locale_is_c(localeptr, false);
+
return (bool) result;
}
@@ -1380,12 +1391,8 @@ lc_ctype_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_CTYPE setting");
- if (strcmp(localeptr, "C") == 0)
- result = true;
- else if (strcmp(localeptr, "POSIX") == 0)
- result = true;
- else
- result = false;
+ result = locale_is_c(localeptr, false);
+
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 0f9b92b32e..a92a0c438f 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -419,9 +419,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
" which is not recognized by setlocale().", ctype),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (strcmp(ctype, "C") == 0 ||
- strcmp(ctype, "POSIX") == 0)
- database_ctype_is_c = true;
+ database_ctype_is_c = locale_is_c(ctype, false);
if (dbform->datlocprovider == COLLPROVIDER_ICU)
{
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 67a1ab2ab2..997156515c 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -39,6 +39,7 @@
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
+#include "utils/pg_locale.h"
#include "utils/syscache.h"
#include "varatt.h"
@@ -1237,9 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- const char *ctype = setlocale(LC_CTYPE, NULL);
-
- if (pg_strcasecmp(ctype, "C") == 0 || pg_strcasecmp(ctype, "POSIX") == 0)
+ if (locale_is_c(locale_ctype, true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index 6447bea8e0..e08d96e099 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -54,6 +54,7 @@ extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
extern char *pg_perm_setlocale(int category, const char *locale);
+extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
extern bool lc_ctype_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
[text/x-patch] v3-0003-Use-thread-safe-locale-APIs.patch (27.6K, ../../CTYXENTV385K.18K3T1Q7IVAB7@gonk/4-v3-0003-Use-thread-safe-locale-APIs.patch)
download | inline diff:
From bc0a79fd0b1cf5f45c93dfd40fc00c054646fb00 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email protected]>
Date: Mon, 3 Jul 2023 08:51:59 -0500
Subject: [PATCH v3 3/3] Use thread-safe locale APIs
setlocale() has been a thorn in Postgres's side for a while. Most
recently this has manifested in bug #17946 where loading libperl would
cause the localization to change out from under Postgres. On Windows,
use _ENABLE_PER_THREAD_LOCALE.
---
src/backend/main/main.c | 55 +++-
src/backend/utils/adt/cash.c | 29 +++
src/backend/utils/adt/formatting.c | 11 +-
src/backend/utils/adt/pg_locale.c | 388 ++++++++++++++++++-----------
src/backend/utils/init/postinit.c | 4 +-
src/backend/utils/mb/mbutils.c | 2 +-
src/common/exec.c | 16 --
src/include/utils/pg_locale.h | 2 +-
8 files changed, 325 insertions(+), 182 deletions(-)
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index ed11e8be7f..093df88f14 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -59,6 +59,11 @@ int
main(int argc, char *argv[])
{
bool do_check_root = true;
+ char *collate_locale;
+ char *ctype_locale;
+#ifdef LC_MESSAGES
+ char *messages_locale;
+#endif
reached_main = true;
@@ -111,15 +116,39 @@ main(int argc, char *argv[])
* these set to "C" then message localization might not work well in the
* postmaster.
*/
- init_locale("LC_COLLATE", LC_COLLATE, "");
- init_locale("LC_CTYPE", LC_CTYPE, "");
+
+ /*
+ * Save off the original locale values prior to the first call of
+ * uselocale(). This ensures that we don't call pg_setlocale() with an empty
+ * string. See the function comment for pg_setlocale() for further
+ * explanation. Note that this restriction doesn't exist on Windows, but we
+ * can use the same code path anyway.
+ */
+ collate_locale = setlocale(LC_COLLATE, "");
+ ctype_locale = setlocale(LC_CTYPE, "");
+
+#ifdef LC_MESSAGES
+ messages_locale = setlocale(LC_MESSAGES, "");
+#endif
+
+
+#ifdef HAVE__CONFIGTHREADLOCALE
+ /*
+ * This call could most likely happen sooner, but just to err on the side of
+ * caution, call it after absorbing locales from the environment.
+ */
+ _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
+#endif
+
+ init_locale("LC_COLLATE", LC_COLLATE, collate_locale);
+ init_locale("LC_CTYPE", LC_CTYPE, ctype_locale);
/*
* LC_MESSAGES will get set later during GUC option processing, but we set
* it here to allow startup error messages to be localized.
*/
#ifdef LC_MESSAGES
- init_locale("LC_MESSAGES", LC_MESSAGES, "");
+ init_locale("LC_MESSAGES", LC_MESSAGES, messages_locale);
#endif
/*
@@ -130,13 +159,6 @@ main(int argc, char *argv[])
init_locale("LC_NUMERIC", LC_NUMERIC, "C");
init_locale("LC_TIME", LC_TIME, "C");
- /*
- * Now that we have absorbed as much as we wish to from the locale
- * environment, remove any LC_ALL setting, so that the environment
- * variables installed by pg_perm_setlocale have force.
- */
- unsetenv("LC_ALL");
-
/*
* Catch standard options before doing much else, in particular before we
* insist on not being root.
@@ -307,8 +329,17 @@ startup_hacks(const char *progname)
static void
init_locale(const char *categoryname, int category, const char *locale)
{
- if (pg_perm_setlocale(category, locale) == NULL &&
- pg_perm_setlocale(category, "C") == NULL)
+ /*
+ * Since we are initializing global locales, NULL and empty string are
+ * invalid arguments. See the pg_setlocale() function description for more
+ * explanation on the empty string. NULL just doesn't make sense in this
+ * context of initializing locales.
+ */
+ Assert(locale);
+ Assert(locale[0] != '\0');
+
+ if (pg_setlocale(category, locale) == NULL &&
+ pg_setlocale(category, "C") == NULL)
elog(FATAL, "could not adopt \"%s\" locale nor C locale for %s",
locale, categoryname);
}
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 32fbad2f57..5c254d912f 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -111,6 +111,11 @@ cash_in(PG_FUNCTION_ARGS)
*csymbol;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(escontext, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/*
* frac_digits will be CHAR_MAX in some locales, notably C. However, just
* testing for == CHAR_MAX is risky, because of compilers like gcc that
@@ -325,6 +330,11 @@ cash_out(PG_FUNCTION_ARGS)
sep_by_space;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
points = lconvert->frac_digits;
if (points < 0 || points > 10)
@@ -1036,6 +1046,11 @@ cash_numeric(PG_FUNCTION_ARGS)
int fpoint;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1095,6 +1110,11 @@ numeric_cash(PG_FUNCTION_ARGS)
Datum numeric_scale;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1128,6 +1148,11 @@ int4_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1158,6 +1183,10 @@ int8_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ ereturn(fcinfo->context, (Datum) 0,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("could not allocate memory for locale information")));
+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index e27ea8ef97..d8dbcbd56b 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -5049,12 +5049,12 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Positive / Negative number sign
*/
- if (lconv->negative_sign && *lconv->negative_sign)
+ if (lconv && lconv->negative_sign && *lconv->negative_sign)
Np->L_negative_sign = lconv->negative_sign;
else
Np->L_negative_sign = "-";
- if (lconv->positive_sign && *lconv->positive_sign)
+ if (lconv && lconv->positive_sign && *lconv->positive_sign)
Np->L_positive_sign = lconv->positive_sign;
else
Np->L_positive_sign = "+";
@@ -5062,9 +5062,8 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Number decimal point
*/
- if (lconv->decimal_point && *lconv->decimal_point)
+ if (lconv && lconv->decimal_point && *lconv->decimal_point)
Np->decimal = lconv->decimal_point;
-
else
Np->decimal = ".";
@@ -5078,7 +5077,7 @@ NUM_prepare_locale(NUMProc *Np)
* but "" for thousands_sep, so we set the thousands_sep too.
* http://archives.postgresql.org/pgsql-hackers/2007-11/msg00772.php
*/
- if (lconv->thousands_sep && *lconv->thousands_sep)
+ if (lconv && lconv->thousands_sep && *lconv->thousands_sep)
Np->L_thousands_sep = lconv->thousands_sep;
/* Make sure thousands separator doesn't match decimal point symbol. */
else if (strcmp(Np->decimal, ",") != 0)
@@ -5089,7 +5088,7 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Currency symbol
*/
- if (lconv->currency_symbol && *lconv->currency_symbol)
+ if (lconv && lconv->currency_symbol && *lconv->currency_symbol)
Np->L_currency_symbol = lconv->currency_symbol;
else
Np->L_currency_symbol = " ";
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 047c02dbab..178f387630 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -98,6 +98,23 @@ char *locale_time;
int icu_validation_level = WARNING;
+#ifdef HAVE_USELOCALE
+/*
+ * Current locale settings
+ *
+ * uselocale() and friends don't have an equivalent to setlocale(cat, NULL), so
+ * as we set locales for various categories, stash them for retrieval as needed.
+ */
+static struct {
+ char *collate;
+ char *ctype;
+ char *messages;
+ char *monetary;
+ char *numeric;
+ char *time;
+} curr_locales = { 0 };
+#endif /* HAVE_USELOCALE */
+
/*
* lc_time localization cache.
*
@@ -208,28 +225,145 @@ locale_is_c(const char *locale, bool ignore_case)
}
+#ifdef HAVE_USELOCALE
+/*
+ * Turns locale categories into their mask equivalents for use in newlocale(3)
+ *
+ * GCC has the masks defined as (1 << category), but this isn't guaranteed to be
+ * the same in other libcs.
+ */
+static int
+category_to_mask(int category)
+{
+ switch (category) {
+ case LC_ALL:
+ return LC_ALL_MASK;
+ case LC_COLLATE:
+ return LC_COLLATE_MASK;
+ case LC_CTYPE:
+ return LC_CTYPE_MASK;
+#ifdef LC_MESSAGES
+ case LC_MESSAGES:
+ return LC_MESSAGES_MASK;
+#endif
+ case LC_MONETARY:
+ return LC_MONETARY_MASK;
+ case LC_NUMERIC:
+ return LC_NUMERIC_MASK;
+ case LC_TIME:
+ return LC_TIME_MASK;
+ }
+
+ pg_unreachable();
+}
+
+
+static char *
+pg_getlocale(int category)
+{
+ switch (category) {
+ case LC_COLLATE:
+ return curr_locales.collate;
+ case LC_CTYPE:
+ return curr_locales.ctype;
+ case LC_MESSAGES:
+ return curr_locales.messages;
+ case LC_MONETARY:
+ return curr_locales.monetary;
+ case LC_NUMERIC:
+ return curr_locales.numeric;
+ case LC_TIME:
+ return curr_locales.time;
+ default:
+ pg_unreachable();
+ }
+}
+#endif /* HAVE_USELOCALE */
+
+
/*
- * pg_perm_setlocale
- *
- * This wraps the libc function setlocale(), with two additions. First, when
- * changing LC_CTYPE, update gettext's encoding for the current message
- * domain. GNU gettext automatically tracks LC_CTYPE on most platforms, but
- * not on Windows. Second, if the operation is successful, the corresponding
- * LC_XXX environment variable is set to match. By setting the environment
- * variable, we ensure that any subsequent use of setlocale(..., "") will
- * preserve the settings made through this routine. Of course, LC_ALL must
- * also be unset to fully ensure that, but that has to be done elsewhere after
- * all the individual LC_XXX variables have been set correctly. (Thank you
- * Perl for making this kluge necessary.)
+ * pg_setlocale
+ *
+ * This wraps the libc functions uselocale() or setlocale() (depending on the
+ * platform), with a single addition. When changing LC_CTYPE, update gettext's
+ * encoding for the current message domain. GNU gettext automatically tracks
+ * LC_CTYPE on most platforms, but not on Windows. This function will abort if
+ * locale is the empty string. A workaround is to pass the output of
+ * setlocale(category, "") to the locale argument.
*/
char *
-pg_perm_setlocale(int category, const char *locale)
+pg_setlocale(int category, const char *locale)
{
char *result;
- const char *envvar;
+#ifdef HAVE_USELOCALE
+ char **save;
+ int category_mask;
+ locale_t prev_locale;
+ locale_t temp_locale;
+ locale_t work_locale;
+
+ if (locale == NULL)
+ return pg_getlocale(category);
+
+ Assert(locale[0] != '\0');
+
+ category_mask = category_to_mask(category);
+
+ prev_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
+
+ result = strdup(locale);
+ work_locale = duplocale(prev_locale);
+ if (!result || work_locale == (locale_t) 0) {
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ return NULL;
+ }
+
+ temp_locale = newlocale(category_mask, locale, work_locale);
+ if (temp_locale == (locale_t) 0) {
+ freelocale(work_locale);
+
+ return NULL; /* fall out immediately on failure */
+ }
+ work_locale = temp_locale;
+
+ uselocale(work_locale);
+ if (prev_locale != LC_GLOBAL_LOCALE)
+ freelocale(prev_locale);
+
+ /*
+ * Copying the locale must occur before we potentially bind the text domain.
+ */
+ switch (category) {
+ case LC_COLLATE:
+ save = &curr_locales.collate;
+ break;
+ case LC_CTYPE:
+ save = &curr_locales.ctype;
+ break;
+ case LC_MESSAGES:
+ save = &curr_locales.messages;
+ break;
+ case LC_MONETARY:
+ save = &curr_locales.monetary;
+ break;
+ case LC_NUMERIC:
+ save = &curr_locales.numeric;
+ break;
+ case LC_TIME:
+ save = &curr_locales.time;
+ break;
+ default:
+ pg_unreachable();
+ }
+
+ /* Free old locale string, and stash the new one */
+ free(*save);
+ *save = result;
-#ifndef WIN32
- result = setlocale(category, locale);
#else
/*
@@ -248,11 +382,12 @@ pg_perm_setlocale(int category, const char *locale)
else
#endif
result = setlocale(category, locale);
-#endif /* WIN32 */
if (result == NULL)
return result; /* fall out immediately on failure */
+#endif /* HAVE_USELOCALE */
+
/*
* Use the right encoding in translated messages. Under ENABLE_NLS, let
* pg_bind_textdomain_codeset() figure it out. Under !ENABLE_NLS, message
@@ -262,11 +397,13 @@ pg_perm_setlocale(int category, const char *locale)
*/
if (category == LC_CTYPE)
{
+#ifndef HAVE_USELOCALE
static char save_lc_ctype[LOCALE_NAME_BUFLEN];
/* copy setlocale() return value before callee invokes it again */
strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));
result = save_lc_ctype;
+#endif
#ifdef ENABLE_NLS
SetMessageEncoding(pg_bind_textdomain_codeset(textdomain(NULL)));
@@ -275,42 +412,6 @@ pg_perm_setlocale(int category, const char *locale)
#endif
}
- switch (category)
- {
- case LC_COLLATE:
- envvar = "LC_COLLATE";
- break;
- case LC_CTYPE:
- envvar = "LC_CTYPE";
- break;
-#ifdef LC_MESSAGES
- case LC_MESSAGES:
- envvar = "LC_MESSAGES";
-#ifdef WIN32
- result = IsoLocaleName(locale);
- if (result == NULL)
- result = (char *) locale;
- elog(DEBUG3, "IsoLocaleName() executed; locale: \"%s\"", result);
-#endif /* WIN32 */
- break;
-#endif /* LC_MESSAGES */
- case LC_MONETARY:
- envvar = "LC_MONETARY";
- break;
- case LC_NUMERIC:
- envvar = "LC_NUMERIC";
- break;
- case LC_TIME:
- envvar = "LC_TIME";
- break;
- default:
- elog(FATAL, "unrecognized LC category: %d", category);
- return NULL; /* keep compiler quiet */
- }
-
- if (setenv(envvar, result, 1) != 0)
- return NULL;
-
return result;
}
@@ -328,6 +429,47 @@ pg_perm_setlocale(int category, const char *locale)
bool
check_locale(int category, const char *locale, char **canonname)
{
+#ifdef HAVE_USELOCALE
+ locale_t saved_locale;
+ locale_t temp_locale;
+ locale_t work_locale;
+ int category_mask;
+
+ category_mask = category_to_mask(category);
+
+ if (canonname)
+ *canonname = NULL; /* in case of failure */
+
+ saved_locale = uselocale((locale_t) 0);
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0) {
+ if (errno == ENOMEM)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ return false;
+ }
+
+ temp_locale = newlocale(category_mask, locale, work_locale);
+ if (temp_locale == (locale_t) 0) {
+ freelocale(work_locale);
+
+ return false;
+ }
+ work_locale = temp_locale;
+
+ uselocale(work_locale);
+ if (canonname)
+ *canonname = pstrdup(locale); /* TODO: This does not handle "" as the locale */
+
+ uselocale(saved_locale);
+ freelocale(work_locale);
+
+ return true;
+
+#else
+
char *save;
char *res;
@@ -354,6 +496,7 @@ check_locale(int category, const char *locale, char **canonname)
pfree(save);
return (res != NULL);
+#endif /* HAVE_USELOCALE */
}
@@ -366,7 +509,7 @@ check_locale(int category, const char *locale, char **canonname)
*
* Note: we accept value = "" as selecting the postmaster's environment
* value, whatever it was (so long as the environment setting is legal).
- * This will have been locked down by an earlier call to pg_perm_setlocale.
+ * This will have been locked down by an earlier call to pg_setlocale.
*/
bool
check_locale_monetary(char **newval, void **extra, GucSource source)
@@ -445,7 +588,7 @@ assign_locale_messages(const char *newval, void *extra)
* We ignore failure, as per comment above.
*/
#ifdef LC_MESSAGES
- (void) pg_perm_setlocale(LC_MESSAGES, newval);
+ (void) pg_setlocale(LC_MESSAGES, newval);
#endif
}
@@ -541,11 +684,8 @@ PGLC_localeconv(void)
static bool CurrentLocaleConvAllocated = false;
struct lconv *extlconv;
struct lconv worklconv;
- char *save_lc_monetary;
- char *save_lc_numeric;
-#ifdef WIN32
- char *save_lc_ctype;
-#endif
+ locale_t saved_locale;
+ locale_t work_locale;
/* Did we do it already? */
if (CurrentLocaleConvValid)
@@ -572,16 +712,15 @@ PGLC_localeconv(void)
*/
memset(&worklconv, 0, sizeof(worklconv));
- /* Save prevailing values of monetary and numeric locales */
- save_lc_monetary = setlocale(LC_MONETARY, NULL);
- if (!save_lc_monetary)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_monetary = pstrdup(save_lc_monetary);
+ /* Save prevailing locale */
+ saved_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
- save_lc_numeric = setlocale(LC_NUMERIC, NULL);
- if (!save_lc_numeric)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_numeric = pstrdup(save_lc_numeric);
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
#ifdef WIN32
@@ -599,37 +738,37 @@ PGLC_localeconv(void)
* results. Hence, we must temporarily set that category as well.
*/
- /* Save prevailing value of ctype locale */
- save_lc_ctype = setlocale(LC_CTYPE, NULL);
- if (!save_lc_ctype)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_ctype = pstrdup(save_lc_ctype);
-
/* Here begins the critical section where we must not throw error */
/* use numeric to set the ctype */
- setlocale(LC_CTYPE, locale_numeric);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_numeric, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for numeric */
- setlocale(LC_NUMERIC, locale_numeric);
+ work_locale = newlocale(LC_NUMERIC_MASK, locale_numeric, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */
+ /* Must copy data now in case updating the locale overwrites it */
worklconv.decimal_point = strdup(extlconv->decimal_point);
worklconv.thousands_sep = strdup(extlconv->thousands_sep);
worklconv.grouping = strdup(extlconv->grouping);
#ifdef WIN32
/* use monetary to set the ctype */
- setlocale(LC_CTYPE, locale_monetary);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_monetary, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for monetary */
- setlocale(LC_MONETARY, locale_monetary);
+ work_locale = newlocale(LC_MONETARY_MASK, locale_monetary, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */
+ /* Must copy data now in case updating the locale overwrites it */
worklconv.int_curr_symbol = strdup(extlconv->int_curr_symbol);
worklconv.currency_symbol = strdup(extlconv->currency_symbol);
worklconv.mon_decimal_point = strdup(extlconv->mon_decimal_point);
@@ -647,22 +786,9 @@ PGLC_localeconv(void)
worklconv.p_sign_posn = extlconv->p_sign_posn;
worklconv.n_sign_posn = extlconv->n_sign_posn;
- /*
- * Restore the prevailing locale settings; failure to do so is fatal.
- * Possibly we could limp along with nondefault LC_MONETARY or LC_NUMERIC,
- * but proceeding with the wrong value of LC_CTYPE would certainly be bad
- * news; and considering that the prevailing LC_MONETARY and LC_NUMERIC
- * are almost certainly "C", there's really no reason that restoring those
- * should fail.
- */
-#ifdef WIN32
- if (!setlocale(LC_CTYPE, save_lc_ctype))
- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);
-#endif
- if (!setlocale(LC_MONETARY, save_lc_monetary))
- elog(FATAL, "failed to restore LC_MONETARY to \"%s\"", save_lc_monetary);
- if (!setlocale(LC_NUMERIC, save_lc_numeric))
- elog(FATAL, "failed to restore LC_NUMERIC to \"%s\"", save_lc_numeric);
+ /* Restore the prevailing locale settings; failure to do so is fatal. */
+ uselocale(saved_locale);
+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can call functions
@@ -673,13 +799,6 @@ PGLC_localeconv(void)
{
int encoding;
- /* Release the pstrdup'd locale names */
- pfree(save_lc_monetary);
- pfree(save_lc_numeric);
-#ifdef WIN32
- pfree(save_lc_ctype);
-#endif
-
/* If any of the preceding strdup calls failed, complain now. */
if (!struct_lconv_is_valid(&worklconv))
ereport(ERROR,
@@ -826,10 +945,8 @@ cache_locale_time(void)
bool strftimefail = false;
int encoding;
int i;
- char *save_lc_time;
-#ifdef WIN32
- char *save_lc_ctype;
-#endif
+ locale_t saved_locale;
+ locale_t work_locale;
/* did we do this already? */
if (CurrentLCTimeValid)
@@ -844,11 +961,15 @@ cache_locale_time(void)
* results afterwards.
*/
- /* Save prevailing value of time locale */
- save_lc_time = setlocale(LC_TIME, NULL);
- if (!save_lc_time)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_time = pstrdup(save_lc_time);
+ /* Save prevailing locale */
+ saved_locale = uselocale((locale_t) 0);
+ Assert(saved_locale != (locale_t) 0);
+
+ work_locale = duplocale(saved_locale);
+ if (work_locale == (locale_t) 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
#ifdef WIN32
@@ -859,17 +980,14 @@ cache_locale_time(void)
* the encoding we'll get back from strftime_win32().
*/
- /* Save prevailing value of ctype locale */
- save_lc_ctype = setlocale(LC_CTYPE, NULL);
- if (!save_lc_ctype)
- elog(ERROR, "setlocale(NULL) failed");
- save_lc_ctype = pstrdup(save_lc_ctype);
-
/* use lc_time to set the ctype */
- setlocale(LC_CTYPE, locale_time);
+ work_locale = newlocale(LC_CTYPE_MASK, locale_time, work_locale);
+ Assert(work_locale != (locale_t) 0);
#endif
- setlocale(LC_TIME, locale_time);
+ work_locale = newlocale(LC_TIME_MASK, locale_time, work_locale);
+ Assert(work_locale != (locale_t) 0);
+ uselocale(work_locale);
/* We use times close to current time as data for strftime(). */
timenow = time(NULL);
@@ -917,12 +1035,8 @@ cache_locale_time(void)
* Restore the prevailing locale settings; as in PGLC_localeconv(),
* failure to do so is fatal.
*/
-#ifdef WIN32
- if (!setlocale(LC_CTYPE, save_lc_ctype))
- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);
-#endif
- if (!setlocale(LC_TIME, save_lc_time))
- elog(FATAL, "failed to restore LC_TIME to \"%s\"", save_lc_time);
+ uselocale(saved_locale);
+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can throw errors, or
@@ -931,12 +1045,6 @@ cache_locale_time(void)
if (strftimefail)
elog(ERROR, "strftime() failed: %m");
- /* Release the pstrdup'd locale names */
- pfree(save_lc_time);
-#ifdef WIN32
- pfree(save_lc_ctype);
-#endif
-
#ifndef WIN32
/*
@@ -1331,18 +1439,14 @@ lc_collate_is_c(Oid collation)
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_COLLATE, NULL);
- if (!localeptr)
- elog(ERROR, "invalid LC_COLLATE setting");
- result = locale_is_c(localeptr, false);
+ result = locale_is_c(curr_locales.collate, false);
return (bool) result;
}
@@ -1375,23 +1479,19 @@ lc_ctype_is_c(Oid collation)
/*
* If we're asked about the default collation, we have to inquire of the C
- * library. Cache the result so we only have to compute it once.
+ * library.
*/
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_CTYPE, NULL);
- if (!localeptr)
- elog(ERROR, "invalid LC_CTYPE setting");
- result = locale_is_c(localeptr, false);
+ result = locale_is_c(curr_locales.ctype, false);
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index a92a0c438f..0741f5666b 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -405,14 +405,14 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datctype);
ctype = TextDatumGetCString(datum);
- if (pg_perm_setlocale(LC_COLLATE, collate) == NULL)
+ if (pg_setlocale(LC_COLLATE, collate) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_COLLATE \"%s\", "
" which is not recognized by setlocale().", collate),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (pg_perm_setlocale(LC_CTYPE, ctype) == NULL)
+ if (pg_setlocale(LC_CTYPE, ctype) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_CTYPE \"%s\", "
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 997156515c..8dc7da47bd 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1238,7 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- if (locale_is_c(locale_ctype, true))
+ if (locale_is_c(pg_setlocale(LC_CTYPE, NULL), true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..f7a9643be4 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -438,22 +438,6 @@ set_pglocale_pgservice(const char *argv0, const char *app)
char path[MAXPGPATH];
char my_exec_path[MAXPGPATH];
- /* don't set LC_ALL in the backend */
- if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0)
- {
- setlocale(LC_ALL, "");
-
- /*
- * One could make a case for reproducing here PostmasterMain()'s test
- * for whether the process is multithreaded. Unlike the postmaster,
- * no frontend program calls sigprocmask() or otherwise provides for
- * mutual exclusion between signal handlers. While frontends using
- * fork(), if multithreaded, are formally exposed to undefined
- * behavior, we have not witnessed a concrete bug. Therefore,
- * complaining about multithreading here may be mere pedantry.
- */
- }
-
if (find_my_exec(argv0, my_exec_path) < 0)
return;
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index e08d96e099..585cdd3931 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -52,7 +52,7 @@ extern PGDLLIMPORT char *localized_full_months[];
extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
-extern char *pg_perm_setlocale(int category, const char *locale);
+extern char *pg_setlocale(int category, const char *locale);
extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 38b4596a775..08e346ae7b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0008-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a6b98fa12a1..b2397fe2054 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 38b4596a775..08e346ae7b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0008-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a6b98fa12a1..b2397fe2054 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v11 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 5ba8cb3657..ce47a158ae 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2172,7 +2172,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2300,6 +2301,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index ae758857bd..5c2a7b7422 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -815,6 +815,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -830,7 +833,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -2023,14 +2027,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2041,7 +2047,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v12 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 4c5fe8f090d..973734e9ffa 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2199,7 +2199,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2327,6 +2328,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595ec..c95e3412da8 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index e8d04b4dec6..19e99c81092 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -811,6 +811,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -826,7 +829,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -2013,14 +2017,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2031,7 +2037,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v12-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 38b4596a775..08e346ae7b3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0008-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c9b9b4c00f1..10c1c3b616b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2112,7 +2112,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2240,6 +2241,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a6b98fa12a1..b2397fe2054 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c43a8b3dea5..f1d0d4b78e3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -796,6 +796,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -811,7 +814,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1951,14 +1955,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1969,7 +1975,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local-.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 7fdccaf613..849cac3947 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2114,7 +2114,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2242,6 +2243,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 1bc5f7c057..b9ba4f9fb3 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -804,6 +804,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -819,7 +822,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1988,14 +1992,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2006,7 +2012,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
* [PATCH v11 06/17] table_scan_bitmap_next_block() returns lossy or exact
@ 2024-02-27 01:34 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Melanie Plageman @ 2024-02-27 01:34 UTC (permalink / raw)
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 5ba8cb3657..ce47a158ae 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2172,7 +2172,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2300,6 +2301,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 404de0595e..c95e3412da 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index ae758857bd..5c2a7b7422 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -815,6 +815,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -830,7 +833,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -2023,14 +2027,16 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
* Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
* a bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -2041,7 +2047,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres, lossy);
}
/*
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch"
^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2024-02-27 01:34 UTC | newest]
Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 12:49 Re: Add 64-bit XIDs into PostgreSQL 15 Aleksander Alekseev <[email protected]>
2022-12-09 13:54 ` adherent postgres <[email protected]>
2022-12-09 14:13 ` Pavel Borisov <[email protected]>
2022-12-09 14:29 ` Maxim Orlov <[email protected]>
2023-07-05 20:53 Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG Tristan Partin <[email protected]>
2023-07-05 20:55 ` Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG Tristan Partin <[email protected]>
2023-07-11 00:52 ` Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG Tristan Partin <[email protected]>
2024-02-27 01:34 [PATCH v12 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v11 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v11 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v8 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v6 07/14] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v7 06/13] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v10 06/17] table_scan_bitmap_next_block() returns lossy or exact Melanie Plageman <[email protected]>
2024-02-27 01:34 [PATCH v9 06/17] table_scan_bitmap_next_block() returns lossy or exact 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