($INBOX_DIR/description missing)  
help / color / mirror / Atom feed
psql tab completion for ALTER INDEX SET
14+ messages / 6 participants
[nested] [flat]

* psql tab completion for ALTER INDEX SET
@ 2018-03-02 06:52  Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Masahiko Sawada @ 2018-03-02 06:52 UTC (permalink / raw)
  To: pgsql-hackers

Hi,

I found that tab completion for ALTER INDEX SET [tab] doesn't support
the reloptions of brin and gist. Attached patch adds "buffering",
"pages_per_range" and "autosummarize" options.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Attachments:

  [application/octet-stream] tab_completion_for_alter_index_set.patch (1.1K, ../../CAD21AoDSGfB0G4egOy2UvBT=uihojuh-syxgSipj+XNkpWdVzQ@mail.gmail.com/2-tab_completion_for_alter_index_set.patch)
  download | inline diff:
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8bc4a19..28b6dd4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1711,11 +1711,17 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH_CONST("(");
 	/* ALTER INDEX <foo> SET|RESET ( */
 	else if (Matches5("ALTER", "INDEX", MatchAny, "RESET", "("))
-		COMPLETE_WITH_LIST3("fillfactor", "fastupdate",
-							"gin_pending_list_limit");
+		COMPLETE_WITH_LIST6("fillfactor", "fastupdate",
+							"gin_pending_list_limit",	/* GIN */
+							"buffering",	/* GiST */
+							"pages_per_range", "autosummarize"	/* BRIN */
+			);
 	else if (Matches5("ALTER", "INDEX", MatchAny, "SET", "("))
-		COMPLETE_WITH_LIST3("fillfactor =", "fastupdate =",
-							"gin_pending_list_limit =");
+		COMPLETE_WITH_LIST6("fillfactor =", "fastupdate =",
+							"gin_pending_list_limit =",	/* GIN */
+							"buffering =", /* GiST */
+							"pages_per_range =", "autosummarize ="	/* BRIN */
+			);
 
 	/* ALTER LANGUAGE <name> */
 	else if (Matches3("ALTER", "LANGUAGE", MatchAny))


^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: psql tab completion for ALTER INDEX SET
@ 2018-03-02 16:43  Fujii Masao <[email protected]>
  parent: Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Fujii Masao @ 2018-03-02 16:43 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: pgsql-hackers

On Fri, Mar 2, 2018 at 3:52 PM, Masahiko Sawada <[email protected]> wrote:
> Hi,
>
> I found that tab completion for ALTER INDEX SET [tab] doesn't support
> the reloptions of brin and gist. Attached patch adds "buffering",
> "pages_per_range" and "autosummarize" options.

Thanks for the patch! Committed.

Regards,

-- 
Fujii Masao




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: psql tab completion for ALTER INDEX SET
@ 2018-03-03 05:35  Masahiko Sawada <[email protected]>
  parent: Fujii Masao <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Masahiko Sawada @ 2018-03-03 05:35 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: pgsql-hackers

On Sat, Mar 3, 2018 at 1:43 AM, Fujii Masao <[email protected]> wrote:
> On Fri, Mar 2, 2018 at 3:52 PM, Masahiko Sawada <[email protected]> wrote:
>> Hi,
>>
>> I found that tab completion for ALTER INDEX SET [tab] doesn't support
>> the reloptions of brin and gist. Attached patch adds "buffering",
>> "pages_per_range" and "autosummarize" options.
>
> Thanks for the patch! Committed.
>

Thank you!

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center




^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: remaining sql/json patches
@ 2024-03-15 10:30  jian he <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: jian he @ 2024-03-15 10:30 UTC (permalink / raw)
  To: Amit Langote <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Erik Rijkers <[email protected]>; pgsql-hackers; Tomas Vondra <[email protected]>

On Mon, Mar 11, 2024 at 11:30 AM jian he <[email protected]> wrote:
>
> On Sun, Mar 10, 2024 at 10:57 PM jian he <[email protected]> wrote:
> >
> > one more issue.
>
> Hi
> one more documentation issue.
> after applied V42, 0001 to 0003,
> there are 11 appearance of `FORMAT JSON` in functions-json.html
> still not a single place explained what it is for.
>
> json_query ( context_item, path_expression [ PASSING { value AS
> varname } [, ...]] [ RETURNING data_type [ FORMAT JSON [ ENCODING UTF8
> ] ] ] [ { WITHOUT | WITH { CONDITIONAL | [UNCONDITIONAL] } } [ ARRAY ]
> WRAPPER ] [ { KEEP | OMIT } QUOTES [ ON SCALAR STRING ] ] [ { ERROR |
> NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression } ON EMPTY ]
> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> ON ERROR ])
>
> FORMAT JSON seems just a syntax sugar or for compatibility in json_query.
> but it returns an error when the returning type category is not
> TYPCATEGORY_STRING.
>
> for example, even the following will return an error.
> `
> CREATE TYPE regtest_comptype AS (b text);
> SELECT JSON_QUERY(jsonb '{"a":{"b":"c"}}', '$.a' RETURNING
> regtest_comptype format json);
> `
>
> seems only types in[0] will not generate an error, when specifying
> FORMAT JSON in JSON_QUERY.
>
> so it actually does something, not a syntax sugar?
>

SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 text format json PATH '$' omit quotes));
SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 text format json PATH '$' keep quotes));
SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 text PATH '$' keep quotes)); -- JSON_QUERY_OP
SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 text PATH '$' omit quotes)); -- JSON_QUERY_OP
SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 text PATH '$')); -- JSON_VALUE_OP
SELECT * FROM JSON_TABLE(jsonb'[{"aaa": 123}]', 'lax $[*]' COLUMNS
(js2 json PATH '$')); -- JSON_QUERY_OP
comparing these queries, I think 'FORMAT JSON' main usage is in json_table.

CREATE TYPE regtest_comptype AS (b text);
SELECT JSON_QUERY(jsonb '{"a":{"b":"c"}}', '$.a' RETURNING
regtest_comptype format json);
ERROR:  cannot use JSON format with non-string output types
LINE 1: ..."a":{"b":"c"}}', '$.a' RETURNING regtest_comptype format jso...
                                                             ^
the error message is not good, but that's a minor issue. we can pursue it later.
-----------------------------------------------------------------------------------------
SELECT JSON_QUERY(jsonb 'true', '$' RETURNING int KEEP QUOTES );
SELECT JSON_QUERY(jsonb 'true', '$' RETURNING int omit QUOTES );
SELECT JSON_VALUE(jsonb 'true', '$' RETURNING int);
the third query returns integer 1, not sure this is the desired behavior.
it obviously has an implication for json_table.
-----------------------------------------------------------------------------------------
in jsonb_get_element, we have something like:
if (jbvp->type == jbvBinary)
{
container = jbvp->val.binary.data;
have_object = JsonContainerIsObject(container);
have_array = JsonContainerIsArray(container);
Assert(!JsonContainerIsScalar(container));
}

+ res = JsonValueListHead(&found);
+ if (res->type == jbvBinary && JsonContainerIsScalar(res->val.binary.data))
+ JsonbExtractScalar(res->val.binary.data, res);
So in JsonPathValue, the above (res->type == jbvBinary) is unreachable?
also see the comment in jbvBinary.

maybe we can just simply do:
if (res->type == jbvBinary)
Assert(!JsonContainerIsScalar(res->val.binary.data));
-----------------------------------------------------------------------------------------
+<synopsis>
+JSON_TABLE (
+  <replaceable>context_item</replaceable>,
<replaceable>path_expression</replaceable> <optional> AS
<replaceable>json_path_name</replaceable> </optional> <optional>
PASSING { <replaceable>value</replaceable> AS
<replaceable>varname</replaceable> } <optional>, ...</optional>
</optional>
+  COLUMNS ( <replaceable
class="parameter">json_table_column</replaceable> <optional>,
...</optional> )
+  <optional> { <literal>ERROR</literal> | <literal>EMPTY</literal> }
<literal>ON ERROR</literal> </optional>
+  <optional>
+    PLAN ( <replaceable class="parameter">json_table_plan</replaceable> ) |
+    PLAN DEFAULT ( { INNER | OUTER } <optional> , { CROSS | UNION } </optional>
+                 | { CROSS | UNION } <optional> , { INNER | OUTER }
</optional> )
+  </optional>
+)

based on the synopsis
the following query should not be allowed?
SELECT *FROM (VALUES ('"11"'), ('"err"')) vals(js)
LEFT OUTER JOIN  JSON_TABLE(vals.js::jsonb, '$' COLUMNS (a int PATH
'$') default '11' ON ERROR) jt ON true;

aslo the synopsis need to reflect case like:
SELECT *FROM (VALUES ('"11"'), ('"err"')) vals(js)
LEFT OUTER JOIN  JSON_TABLE(vals.js::jsonb, '$' COLUMNS (a int PATH
'$') NULL ON ERROR) jt ON true;






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-03-28 15:30  Peter Eisentraut <[email protected]>
  1 sibling, 1 reply; 14+ messages in thread

From: Peter Eisentraut @ 2025-03-28 15:30 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Tristan Partin <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On 09.02.25 08:32, Peter Eisentraut wrote:
> Checking the status of this thread ...
> 
> The patches that removed the configure checks for _configthreadlocale(), 
> and related cleanup, have been committed.
> 
> The original patch to "Tidy up locale thread safety in ECPG library" is 
> still outstanding.
> 
> Attached is a rebased version, based on the posted v6, with a couple of 
> small fixups from me.
> 
> I haven't re-reviewed it yet, but from scanning the discussion, it looks 
> close to done.

After staring at this a few more times, I figured it was ready enough 
and I committed it.






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-03-28 16:14  Masahiko Sawada <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  0 siblings, 1 reply; 14+ messages in thread

From: Masahiko Sawada @ 2025-03-28 16:14 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Tristan Partin <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut <[email protected]> wrote:
>
> On 09.02.25 08:32, Peter Eisentraut wrote:
> > Checking the status of this thread ...
> >
> > The patches that removed the configure checks for _configthreadlocale(),
> > and related cleanup, have been committed.
> >
> > The original patch to "Tidy up locale thread safety in ECPG library" is
> > still outstanding.
> >
> > Attached is a rebased version, based on the posted v6, with a couple of
> > small fixups from me.
> >
> > I haven't re-reviewed it yet, but from scanning the discussion, it looks
> > close to done.
>
> After staring at this a few more times, I figured it was ready enough
> and I committed it.

It seems that some bf animals such as jackdaw are unhappy with this
commit[0][1]. I also got the same 'undefined reference to symbol
error' locally when building test_json_parser.

Regards,

[0] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snakefly&dt=2025-03-28%2015%3A29%3A04
[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jackdaw&dt=2025-03-28%2015%3A30%3A44

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-03-28 16:32  Peter Eisentraut <[email protected]>
  parent: Masahiko Sawada <[email protected]>
  0 siblings, 2 replies; 14+ messages in thread

From: Peter Eisentraut @ 2025-03-28 16:32 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Tristan Partin <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On 28.03.25 17:14, Masahiko Sawada wrote:
> On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut <[email protected]> wrote:
>>
>> On 09.02.25 08:32, Peter Eisentraut wrote:
>>> Checking the status of this thread ...
>>>
>>> The patches that removed the configure checks for _configthreadlocale(),
>>> and related cleanup, have been committed.
>>>
>>> The original patch to "Tidy up locale thread safety in ECPG library" is
>>> still outstanding.
>>>
>>> Attached is a rebased version, based on the posted v6, with a couple of
>>> small fixups from me.
>>>
>>> I haven't re-reviewed it yet, but from scanning the discussion, it looks
>>> close to done.
>>
>> After staring at this a few more times, I figured it was ready enough
>> and I committed it.
> 
> It seems that some bf animals such as jackdaw are unhappy with this
> commit[0][1]. I also got the same 'undefined reference to symbol
> error' locally when building test_json_parser.

Yeah, looks like we'll have to revert this for now.  But I'm confused, 
because I don't see any clear pattern for which platforms or 
configurations it's failing and for which not.






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-03-28 20:34  Peter Eisentraut <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 14+ messages in thread

From: Peter Eisentraut @ 2025-03-28 20:34 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Tristan Partin <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On 28.03.25 17:32, Peter Eisentraut wrote:
> On 28.03.25 17:14, Masahiko Sawada wrote:
>> On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut 
>> <[email protected]> wrote:
>>>
>>> On 09.02.25 08:32, Peter Eisentraut wrote:
>>>> Checking the status of this thread ...
>>>>
>>>> The patches that removed the configure checks for 
>>>> _configthreadlocale(),
>>>> and related cleanup, have been committed.
>>>>
>>>> The original patch to "Tidy up locale thread safety in ECPG library" is
>>>> still outstanding.
>>>>
>>>> Attached is a rebased version, based on the posted v6, with a couple of
>>>> small fixups from me.
>>>>
>>>> I haven't re-reviewed it yet, but from scanning the discussion, it 
>>>> looks
>>>> close to done.
>>>
>>> After staring at this a few more times, I figured it was ready enough
>>> and I committed it.
>>
>> It seems that some bf animals such as jackdaw are unhappy with this
>> commit[0][1]. I also got the same 'undefined reference to symbol
>> error' locally when building test_json_parser.
> 
> Yeah, looks like we'll have to revert this for now.  But I'm confused, 
> because I don't see any clear pattern for which platforms or 
> configurations it's failing and for which not.

reverted






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-03-29 00:01  Masahiko Sawada <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 14+ messages in thread

From: Masahiko Sawada @ 2025-03-29 00:01 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Tristan Partin <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

On Fri, Mar 28, 2025 at 9:32 AM Peter Eisentraut <[email protected]> wrote:
>
> On 28.03.25 17:14, Masahiko Sawada wrote:
> > On Fri, Mar 28, 2025 at 8:30 AM Peter Eisentraut <[email protected]> wrote:
> >>
> >> On 09.02.25 08:32, Peter Eisentraut wrote:
> >>> Checking the status of this thread ...
> >>>
> >>> The patches that removed the configure checks for _configthreadlocale(),
> >>> and related cleanup, have been committed.
> >>>
> >>> The original patch to "Tidy up locale thread safety in ECPG library" is
> >>> still outstanding.
> >>>
> >>> Attached is a rebased version, based on the posted v6, with a couple of
> >>> small fixups from me.
> >>>
> >>> I haven't re-reviewed it yet, but from scanning the discussion, it looks
> >>> close to done.
> >>
> >> After staring at this a few more times, I figured it was ready enough
> >> and I committed it.
> >
> > It seems that some bf animals such as jackdaw are unhappy with this
> > commit[0][1]. I also got the same 'undefined reference to symbol
> > error' locally when building test_json_parser.
>
> Yeah, looks like we'll have to revert this for now.  But I'm confused,
> because I don't see any clear pattern for which platforms or
> configurations it's failing and for which not.
>

Not sure it would help the investigation but I got the linker error
when building with 'make' but not with 'meson'. Looking at the build
logs, when building test_json_parser with meson, it adds -lpthread as
follows:

% /home/masahiko/work/gcc/12.2.0/bin/gcc -v -o
src/test/modules/test_json_parser/test_json_parser_incremental_shlib
src/test/modules/test_json_parser/test_json_parser_incremental_shlib.p/test_json_parser_incremental.c.o
-Wl,--as-needed -Wl,--no-undefined
'-Wl,-rpath,$ORIGIN/../../../interfaces/libpq:/lib/../lib64'
-Wl,-rpath-link,/lib/../lib64
-Wl,-rpath-link,/home/masahiko/pgsql/source/dev_master/build/src/interfaces/libpq
-Wl,--start-group src/common/libpgcommon_excluded_shlib.a
src/common/libpgcommon_shlib.a
src/common/libpgcommon_shlib_config_info.a
src/common/libpgcommon_shlib_ryu.a src/port/libpgport_shlib.a
src/interfaces/libpq/libpq.so.5.18 -lm -ldl -pthread -lrt
/usr/lib64/libz.so /lib/../lib64/libzstd.so /usr/lib64/liblz4.so
/usr/lib64/libssl.so /usr/lib64/libcrypto.so -Wl,--end-group

whereas with 'make' it doesn't:

% gcc -v -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3
-Wcast-function-type -Wshadow=compatible-local -Wformat-security
-fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -g -g -O0
test_json_parser_incremental.o -L../../../../src/port
-L../../../../src/common   -Wl,--as-needed
-Wl,-rpath,'/home/masahiko/pgsql/master/lib',--enable-new-dtags
-lpgcommon_excluded_shlib -L../../../../src/common -lpgcommon_shlib
-L../../../../src/port -lpgport_shlib
-L../../../../src/interfaces/libpq -lpq  -o
test_json_parser_incremental_shlib

FYI the following change fixed the issue in my local env:

--- a/src/test/modules/test_json_parser/Makefile
+++ b/src/test/modules/test_json_parser/Makefile
@@ -27,7 +27,7 @@ test_json_parser_incremental$(X):
test_json_parser_incremental.o $(WIN32RES)
    $(CC) $(CFLAGS) $^ $(PG_LIBS_INTERNAL) $(LDFLAGS) $(LDFLAGS_EX)
$(PG_LIBS) $(LIBS) -o $@

 test_json_parser_incremental_shlib$(X):
test_json_parser_incremental.o $(WIN32RES)
-   $(CC) $(CFLAGS) $^ $(LDFLAGS) -lpgcommon_excluded_shlib
$(libpq_pgport_shlib) $(filter -lintl, $(LIBS)) -o $@
+   $(CC) $(CFLAGS) $^ $(LDFLAGS) -lpgcommon_excluded_shlib
$(libpq_pgport_shlib) $(filter -lintl, $(LIBS)) $(LIBS) -o $@

 test_json_parser_perf$(X): test_json_parser_perf.o $(WIN32RES)
    $(CC) $(CFLAGS) $^ $(PG_LIBS_INTERNAL) $(LDFLAGS) $(LDFLAGS_EX)
$(PG_LIBS) $(LIBS) -o $@

It seems that no MacOS and NetBSD animals failed due to this error
because they have LC_C_LOCALE. But I'm not sure why some animals
successfully built test_json_parser() even without -lpthread or
-pthread[1][2].

Regards,

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prion&dt=2025-03-28%2015%3A33%3A03...
[2] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=bushmaster&dt=2025-03-28%2015%3A32...

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: On non-Windows, hard depend on uselocale(3)
@ 2025-12-12 18:07  Tom Lane <[email protected]>
  1 sibling, 0 replies; 14+ messages in thread

From: Tom Lane @ 2025-12-12 18:07 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Thomas Munro <[email protected]>; Tristan Partin <[email protected]>; pgsql-hackers

Peter Eisentraut <[email protected]> writes:
> On 28.08.24 20:50, Peter Eisentraut wrote:
>> I suggest that the simplification of the xlocale.h configure tests could 
>> be committed separately.  This would also be useful independent of this, 
>> and it's a sizeable chunk of this patch.

> To keep this moving along a bit, I have extracted this part and 
> committed it separately.  I had to make a few small tweaks, e.g., there 
> was no check for xlocale.h in configure.ac, and the old 
> xlocale.h-including stanza could be removed from chklocale.h.  Let's see 
> how this goes.

For the archives' sake --- I discovered today during a "git bisect"
session that commits between 35eeea623 ("Use thread-safe
nl_langinfo_l(), not nl_langinfo()") and 9c2a6c5a5 ("Simplify checking
for xlocale.h", the commit Peter refers to here) fail to build on
current macOS (26/Tahoe):

chklocale.c:330:8: error: call to undeclared function 'nl_langinfo_l'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  330 |         sys = nl_langinfo_l(CODESET, loc);
      |               ^
chklocale.c:330:8: note: did you mean 'nl_langinfo'?
/Library/Developer/CommandLineTools/SDKs/MacOSX26.1.sdk/usr/include/_langinfo.h:116:20: note: 'nl_langinfo' declared here
  116 | char    *_LIBC_CSTR   nl_langinfo(nl_item);
      |                       ^
chklocale.c:330:6: error: incompatible integer to pointer conversion assigning to 'char *' from 'int' [-Wint-conversion]
  330 |         sys = nl_langinfo_l(CODESET, loc);
      |             ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

This happens because nl_langinfo_l() is declared in xlocale.h,
but chklocale.c elects not to #include that, evidently because
it's no longer needed to obtain typedef locale_t.

AFAICS there's nothing we can do about this retroactively;
it'll be a more or less permanent landmine for bisecting on macOS.
Fortunately it's not too difficult to work around, you can just do

diff --git a/src/port/chklocale.c b/src/port/chklocale.c
index 9506cd87ed8..0e35e0cf55f 100644
--- a/src/port/chklocale.c
+++ b/src/port/chklocale.c
@@ -23,9 +23,7 @@
 #include <langinfo.h>
 #endif
 
-#ifdef LOCALE_T_IN_XLOCALE
 #include <xlocale.h>
-#endif
 
 #include "mb/pg_wchar.h"
 
when trying to build one of the affected commits.  Another option
that might fit into a bisecting workflow more easily is to inject
-DLOCALE_T_IN_XLOCALE into CPPFLAGS.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting
@ 2026-04-03 19:01  Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Álvaro Herrera @ 2026-04-03 19:01 UTC (permalink / raw)

---
 src/backend/commands/repack_worker.c           |  3 ++-
 src/backend/replication/logical/logical.c      |  8 +++++---
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slot.c                 | 18 ++++++++++++------
 src/backend/replication/slotfuncs.c            | 11 ++++++-----
 src/backend/replication/walsender.c            |  5 +++--
 src/include/replication/logical.h              |  3 ++-
 src/include/replication/slot.h                 |  2 +-
 8 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 610592a05b0..ca827223845 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -224,7 +224,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Make sure we can use logical decoding.
 	 */
 	CheckSlotPermissions();
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(true);
 
 	/*
 	 * A single backend should not execute multiple REPACK commands at a time,
@@ -252,6 +252,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
+									true,
 									InvalidXLogRecPtr,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index f20a0fe70ad..a08aece5731 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -108,9 +108,9 @@ static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugi
  * decoding.
  */
 void
-CheckLogicalDecodingRequirements(void)
+CheckLogicalDecodingRequirements(bool repack)
 {
-	CheckSlotRequirements();
+	CheckSlotRequirements(repack);
 
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
@@ -305,6 +305,7 @@ StartupDecodingContext(List *output_plugin_options,
  * output_plugin_options -- contains options passed to the output plugin
  * need_full_snapshot -- if true, must obtain a snapshot able to read all
  *		tables; if false, one that can read only catalogs is acceptable.
+ * for_repack -- if true, we're going to be decoding for REPACK.
  * restart_lsn -- if given as invalid, it's this routine's responsibility to
  *		mark WAL as reserved by setting a convenient restart_lsn for the slot.
  *		Otherwise, we set for decoding to start from the given LSN without
@@ -325,6 +326,7 @@ LogicalDecodingContext *
 CreateInitDecodingContext(const char *plugin,
 						  List *output_plugin_options,
 						  bool need_full_snapshot,
+						  bool for_repack,
 						  XLogRecPtr restart_lsn,
 						  XLogReaderRoutine *xl_routine,
 						  LogicalOutputPluginWriterPrepareWrite prepare_write,
@@ -341,7 +343,7 @@ CreateInitDecodingContext(const char *plugin,
 	 * On a standby, this check is also required while creating the slot.
 	 * Check the comments in the function.
 	 */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(for_repack);
 
 	/* shorter lines... */
 	slot = MyReplicationSlot;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 9760818941d..512013b0ef0 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,7 +115,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	if (PG_ARGISNULL(0))
 		ereport(ERROR,
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2c6c6773ad2..13004ed547a 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1669,19 +1669,25 @@ CheckLogicalSlotExists(void)
  * slots.
  */
 void
-CheckSlotRequirements(void)
+CheckSlotRequirements(bool repack)
 {
+	int		limit;
+
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
 	 * needs the same check.
 	 */
 
-	/* XXX we should be able to check exactly which type of slot we need */
-	if (max_replication_slots + max_repack_replication_slots == 0)
+	if (repack)
+		limit = max_repack_replication_slots;
+	else
+		limit = max_replication_slots;
+
+	if (limit == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if \"%s\" > 0 or \"%s\" > 0",
-						"max_replication_slots", "max_repack_replication_slots")));
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("replication slots can only be used if \"%s\" > 0",
+					   repack ? "max_repack_replication_slots" : "max_replication_slots"));
 
 	if (wal_level < WAL_LEVEL_REPLICA)
 		ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 78dd3c4ea66..16fbd383735 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -90,7 +90,7 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	create_physical_replication_slot(NameStr(*name),
 									 immediately_reserve,
@@ -164,6 +164,7 @@ create_logical_replication_slot(char *name, char *plugin,
 	 */
 	ctx = CreateInitDecodingContext(plugin, NIL,
 									false,	/* just catalogs is OK */
+									false,	/* not repack */
 									restart_lsn,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
@@ -203,7 +204,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	create_logical_replication_slot(NameStr(*name),
 									NameStr(*plugin),
@@ -240,7 +241,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	ReplicationSlotDrop(NameStr(*name), true);
 
@@ -648,9 +649,9 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 	CheckSlotPermissions();
 
 	if (logical_slot)
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 	else
-		CheckSlotRequirements();
+		CheckSlotRequirements(false);
 
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75ef3419a15..9d7d675fa96 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1240,7 +1240,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 
 		Assert(cmd->kind == REPLICATION_KIND_LOGICAL);
 
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 
 		/*
 		 * Initially create persistent slot as ephemeral - that allows us to
@@ -1309,6 +1309,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 		Assert(IsLogicalDecodingEnabled());
 
 		ctx = CreateInitDecodingContext(cmd->plugin, NIL, need_full_snapshot,
+										false,
 										InvalidXLogRecPtr,
 										XL_ROUTINE(.page_read = logical_read_xlog_page,
 												   .segment_open = WalSndSegmentOpen,
@@ -1466,7 +1467,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
 	QueryCompletion qc;
 
 	/* make sure that our requirements are still fulfilled */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	Assert(!MyReplicationSlot);
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index bc9d4ece672..bc075b16741 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -115,11 +115,12 @@ typedef struct LogicalDecodingContext
 } LogicalDecodingContext;
 
 
-extern void CheckLogicalDecodingRequirements(void);
+extern void CheckLogicalDecodingRequirements(bool repack);
 
 extern LogicalDecodingContext *CreateInitDecodingContext(const char *plugin,
 														 List *output_plugin_options,
 														 bool need_full_snapshot,
+														 bool for_repack,
 														 XLogRecPtr restart_lsn,
 														 XLogReaderRoutine *xl_routine,
 														 LogicalOutputPluginWriterPrepareWrite prepare_write,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c316a01a807..489af7d8d6c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -378,7 +378,7 @@ extern void ReplicationSlotDropAtPubNode(WalReceiverConn *wrconn, char *slotname
 extern void StartupReplicationSlots(void);
 extern void CheckPointReplicationSlots(bool is_shutdown);
 
-extern void CheckSlotRequirements(void);
+extern void CheckSlotRequirements(bool repack);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
 			GetSlotInvalidationCause(const char *cause_name);
-- 
2.47.3


--gp2pyozrd5pweboh--





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v51 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting
@ 2026-04-03 19:01  Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Álvaro Herrera @ 2026-04-03 19:01 UTC (permalink / raw)

---
 src/backend/commands/repack_worker.c           |  3 ++-
 src/backend/replication/logical/logical.c      |  8 +++++---
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slot.c                 | 18 ++++++++++++------
 src/backend/replication/slotfuncs.c            | 11 ++++++-----
 src/backend/replication/walsender.c            |  5 +++--
 src/include/replication/logical.h              |  3 ++-
 src/include/replication/slot.h                 |  2 +-
 8 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 610592a05b0..ca827223845 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -224,7 +224,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Make sure we can use logical decoding.
 	 */
 	CheckSlotPermissions();
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(true);
 
 	/*
 	 * A single backend should not execute multiple REPACK commands at a time,
@@ -252,6 +252,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
+									true,
 									InvalidXLogRecPtr,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index f20a0fe70ad..a08aece5731 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -108,9 +108,9 @@ static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugi
  * decoding.
  */
 void
-CheckLogicalDecodingRequirements(void)
+CheckLogicalDecodingRequirements(bool repack)
 {
-	CheckSlotRequirements();
+	CheckSlotRequirements(repack);
 
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
@@ -305,6 +305,7 @@ StartupDecodingContext(List *output_plugin_options,
  * output_plugin_options -- contains options passed to the output plugin
  * need_full_snapshot -- if true, must obtain a snapshot able to read all
  *		tables; if false, one that can read only catalogs is acceptable.
+ * for_repack -- if true, we're going to be decoding for REPACK.
  * restart_lsn -- if given as invalid, it's this routine's responsibility to
  *		mark WAL as reserved by setting a convenient restart_lsn for the slot.
  *		Otherwise, we set for decoding to start from the given LSN without
@@ -325,6 +326,7 @@ LogicalDecodingContext *
 CreateInitDecodingContext(const char *plugin,
 						  List *output_plugin_options,
 						  bool need_full_snapshot,
+						  bool for_repack,
 						  XLogRecPtr restart_lsn,
 						  XLogReaderRoutine *xl_routine,
 						  LogicalOutputPluginWriterPrepareWrite prepare_write,
@@ -341,7 +343,7 @@ CreateInitDecodingContext(const char *plugin,
 	 * On a standby, this check is also required while creating the slot.
 	 * Check the comments in the function.
 	 */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(for_repack);
 
 	/* shorter lines... */
 	slot = MyReplicationSlot;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 9760818941d..512013b0ef0 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,7 +115,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	if (PG_ARGISNULL(0))
 		ereport(ERROR,
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2c6c6773ad2..13004ed547a 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1669,19 +1669,25 @@ CheckLogicalSlotExists(void)
  * slots.
  */
 void
-CheckSlotRequirements(void)
+CheckSlotRequirements(bool repack)
 {
+	int		limit;
+
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
 	 * needs the same check.
 	 */
 
-	/* XXX we should be able to check exactly which type of slot we need */
-	if (max_replication_slots + max_repack_replication_slots == 0)
+	if (repack)
+		limit = max_repack_replication_slots;
+	else
+		limit = max_replication_slots;
+
+	if (limit == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if \"%s\" > 0 or \"%s\" > 0",
-						"max_replication_slots", "max_repack_replication_slots")));
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("replication slots can only be used if \"%s\" > 0",
+					   repack ? "max_repack_replication_slots" : "max_replication_slots"));
 
 	if (wal_level < WAL_LEVEL_REPLICA)
 		ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 78dd3c4ea66..16fbd383735 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -90,7 +90,7 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	create_physical_replication_slot(NameStr(*name),
 									 immediately_reserve,
@@ -164,6 +164,7 @@ create_logical_replication_slot(char *name, char *plugin,
 	 */
 	ctx = CreateInitDecodingContext(plugin, NIL,
 									false,	/* just catalogs is OK */
+									false,	/* not repack */
 									restart_lsn,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
@@ -203,7 +204,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	create_logical_replication_slot(NameStr(*name),
 									NameStr(*plugin),
@@ -240,7 +241,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	ReplicationSlotDrop(NameStr(*name), true);
 
@@ -648,9 +649,9 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 	CheckSlotPermissions();
 
 	if (logical_slot)
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 	else
-		CheckSlotRequirements();
+		CheckSlotRequirements(false);
 
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75ef3419a15..9d7d675fa96 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1240,7 +1240,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 
 		Assert(cmd->kind == REPLICATION_KIND_LOGICAL);
 
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 
 		/*
 		 * Initially create persistent slot as ephemeral - that allows us to
@@ -1309,6 +1309,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 		Assert(IsLogicalDecodingEnabled());
 
 		ctx = CreateInitDecodingContext(cmd->plugin, NIL, need_full_snapshot,
+										false,
 										InvalidXLogRecPtr,
 										XL_ROUTINE(.page_read = logical_read_xlog_page,
 												   .segment_open = WalSndSegmentOpen,
@@ -1466,7 +1467,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
 	QueryCompletion qc;
 
 	/* make sure that our requirements are still fulfilled */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	Assert(!MyReplicationSlot);
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index bc9d4ece672..bc075b16741 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -115,11 +115,12 @@ typedef struct LogicalDecodingContext
 } LogicalDecodingContext;
 
 
-extern void CheckLogicalDecodingRequirements(void);
+extern void CheckLogicalDecodingRequirements(bool repack);
 
 extern LogicalDecodingContext *CreateInitDecodingContext(const char *plugin,
 														 List *output_plugin_options,
 														 bool need_full_snapshot,
+														 bool for_repack,
 														 XLogRecPtr restart_lsn,
 														 XLogReaderRoutine *xl_routine,
 														 LogicalOutputPluginWriterPrepareWrite prepare_write,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c316a01a807..489af7d8d6c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -378,7 +378,7 @@ extern void ReplicationSlotDropAtPubNode(WalReceiverConn *wrconn, char *slotname
 extern void StartupReplicationSlots(void);
 extern void CheckPointReplicationSlots(bool is_shutdown);
 
-extern void CheckSlotRequirements(void);
+extern void CheckSlotRequirements(bool repack);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
 			GetSlotInvalidationCause(const char *cause_name);
-- 
2.47.3


--r2slln3zpmilwu22--





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting
@ 2026-04-03 19:01  Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Álvaro Herrera @ 2026-04-03 19:01 UTC (permalink / raw)

---
 src/backend/commands/repack_worker.c           |  3 ++-
 src/backend/replication/logical/logical.c      |  8 +++++---
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slot.c                 | 18 ++++++++++++------
 src/backend/replication/slotfuncs.c            | 11 ++++++-----
 src/backend/replication/walsender.c            |  5 +++--
 src/include/replication/logical.h              |  3 ++-
 src/include/replication/slot.h                 |  2 +-
 8 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 610592a05b0..ca827223845 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -224,7 +224,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Make sure we can use logical decoding.
 	 */
 	CheckSlotPermissions();
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(true);
 
 	/*
 	 * A single backend should not execute multiple REPACK commands at a time,
@@ -252,6 +252,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
+									true,
 									InvalidXLogRecPtr,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index f20a0fe70ad..a08aece5731 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -108,9 +108,9 @@ static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugi
  * decoding.
  */
 void
-CheckLogicalDecodingRequirements(void)
+CheckLogicalDecodingRequirements(bool repack)
 {
-	CheckSlotRequirements();
+	CheckSlotRequirements(repack);
 
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
@@ -305,6 +305,7 @@ StartupDecodingContext(List *output_plugin_options,
  * output_plugin_options -- contains options passed to the output plugin
  * need_full_snapshot -- if true, must obtain a snapshot able to read all
  *		tables; if false, one that can read only catalogs is acceptable.
+ * for_repack -- if true, we're going to be decoding for REPACK.
  * restart_lsn -- if given as invalid, it's this routine's responsibility to
  *		mark WAL as reserved by setting a convenient restart_lsn for the slot.
  *		Otherwise, we set for decoding to start from the given LSN without
@@ -325,6 +326,7 @@ LogicalDecodingContext *
 CreateInitDecodingContext(const char *plugin,
 						  List *output_plugin_options,
 						  bool need_full_snapshot,
+						  bool for_repack,
 						  XLogRecPtr restart_lsn,
 						  XLogReaderRoutine *xl_routine,
 						  LogicalOutputPluginWriterPrepareWrite prepare_write,
@@ -341,7 +343,7 @@ CreateInitDecodingContext(const char *plugin,
 	 * On a standby, this check is also required while creating the slot.
 	 * Check the comments in the function.
 	 */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(for_repack);
 
 	/* shorter lines... */
 	slot = MyReplicationSlot;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 9760818941d..512013b0ef0 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,7 +115,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	if (PG_ARGISNULL(0))
 		ereport(ERROR,
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2c6c6773ad2..13004ed547a 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1669,19 +1669,25 @@ CheckLogicalSlotExists(void)
  * slots.
  */
 void
-CheckSlotRequirements(void)
+CheckSlotRequirements(bool repack)
 {
+	int		limit;
+
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
 	 * needs the same check.
 	 */
 
-	/* XXX we should be able to check exactly which type of slot we need */
-	if (max_replication_slots + max_repack_replication_slots == 0)
+	if (repack)
+		limit = max_repack_replication_slots;
+	else
+		limit = max_replication_slots;
+
+	if (limit == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if \"%s\" > 0 or \"%s\" > 0",
-						"max_replication_slots", "max_repack_replication_slots")));
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("replication slots can only be used if \"%s\" > 0",
+					   repack ? "max_repack_replication_slots" : "max_replication_slots"));
 
 	if (wal_level < WAL_LEVEL_REPLICA)
 		ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 78dd3c4ea66..16fbd383735 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -90,7 +90,7 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	create_physical_replication_slot(NameStr(*name),
 									 immediately_reserve,
@@ -164,6 +164,7 @@ create_logical_replication_slot(char *name, char *plugin,
 	 */
 	ctx = CreateInitDecodingContext(plugin, NIL,
 									false,	/* just catalogs is OK */
+									false,	/* not repack */
 									restart_lsn,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
@@ -203,7 +204,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	create_logical_replication_slot(NameStr(*name),
 									NameStr(*plugin),
@@ -240,7 +241,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	ReplicationSlotDrop(NameStr(*name), true);
 
@@ -648,9 +649,9 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 	CheckSlotPermissions();
 
 	if (logical_slot)
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 	else
-		CheckSlotRequirements();
+		CheckSlotRequirements(false);
 
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75ef3419a15..9d7d675fa96 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1240,7 +1240,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 
 		Assert(cmd->kind == REPLICATION_KIND_LOGICAL);
 
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 
 		/*
 		 * Initially create persistent slot as ephemeral - that allows us to
@@ -1309,6 +1309,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 		Assert(IsLogicalDecodingEnabled());
 
 		ctx = CreateInitDecodingContext(cmd->plugin, NIL, need_full_snapshot,
+										false,
 										InvalidXLogRecPtr,
 										XL_ROUTINE(.page_read = logical_read_xlog_page,
 												   .segment_open = WalSndSegmentOpen,
@@ -1466,7 +1467,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
 	QueryCompletion qc;
 
 	/* make sure that our requirements are still fulfilled */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	Assert(!MyReplicationSlot);
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index bc9d4ece672..bc075b16741 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -115,11 +115,12 @@ typedef struct LogicalDecodingContext
 } LogicalDecodingContext;
 
 
-extern void CheckLogicalDecodingRequirements(void);
+extern void CheckLogicalDecodingRequirements(bool repack);
 
 extern LogicalDecodingContext *CreateInitDecodingContext(const char *plugin,
 														 List *output_plugin_options,
 														 bool need_full_snapshot,
+														 bool for_repack,
 														 XLogRecPtr restart_lsn,
 														 XLogReaderRoutine *xl_routine,
 														 LogicalOutputPluginWriterPrepareWrite prepare_write,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c316a01a807..489af7d8d6c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -378,7 +378,7 @@ extern void ReplicationSlotDropAtPubNode(WalReceiverConn *wrconn, char *slotname
 extern void StartupReplicationSlots(void);
 extern void CheckPointReplicationSlots(bool is_shutdown);
 
-extern void CheckSlotRequirements(void);
+extern void CheckSlotRequirements(bool repack);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
 			GetSlotInvalidationCause(const char *cause_name);
-- 
2.47.3


--gp2pyozrd5pweboh--





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* [PATCH v51 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting
@ 2026-04-03 19:01  Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Álvaro Herrera @ 2026-04-03 19:01 UTC (permalink / raw)

---
 src/backend/commands/repack_worker.c           |  3 ++-
 src/backend/replication/logical/logical.c      |  8 +++++---
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slot.c                 | 18 ++++++++++++------
 src/backend/replication/slotfuncs.c            | 11 ++++++-----
 src/backend/replication/walsender.c            |  5 +++--
 src/include/replication/logical.h              |  3 ++-
 src/include/replication/slot.h                 |  2 +-
 8 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 610592a05b0..ca827223845 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -224,7 +224,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Make sure we can use logical decoding.
 	 */
 	CheckSlotPermissions();
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(true);
 
 	/*
 	 * A single backend should not execute multiple REPACK commands at a time,
@@ -252,6 +252,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
+									true,
 									InvalidXLogRecPtr,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index f20a0fe70ad..a08aece5731 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -108,9 +108,9 @@ static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugi
  * decoding.
  */
 void
-CheckLogicalDecodingRequirements(void)
+CheckLogicalDecodingRequirements(bool repack)
 {
-	CheckSlotRequirements();
+	CheckSlotRequirements(repack);
 
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
@@ -305,6 +305,7 @@ StartupDecodingContext(List *output_plugin_options,
  * output_plugin_options -- contains options passed to the output plugin
  * need_full_snapshot -- if true, must obtain a snapshot able to read all
  *		tables; if false, one that can read only catalogs is acceptable.
+ * for_repack -- if true, we're going to be decoding for REPACK.
  * restart_lsn -- if given as invalid, it's this routine's responsibility to
  *		mark WAL as reserved by setting a convenient restart_lsn for the slot.
  *		Otherwise, we set for decoding to start from the given LSN without
@@ -325,6 +326,7 @@ LogicalDecodingContext *
 CreateInitDecodingContext(const char *plugin,
 						  List *output_plugin_options,
 						  bool need_full_snapshot,
+						  bool for_repack,
 						  XLogRecPtr restart_lsn,
 						  XLogReaderRoutine *xl_routine,
 						  LogicalOutputPluginWriterPrepareWrite prepare_write,
@@ -341,7 +343,7 @@ CreateInitDecodingContext(const char *plugin,
 	 * On a standby, this check is also required while creating the slot.
 	 * Check the comments in the function.
 	 */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(for_repack);
 
 	/* shorter lines... */
 	slot = MyReplicationSlot;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 9760818941d..512013b0ef0 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,7 +115,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	if (PG_ARGISNULL(0))
 		ereport(ERROR,
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2c6c6773ad2..13004ed547a 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1669,19 +1669,25 @@ CheckLogicalSlotExists(void)
  * slots.
  */
 void
-CheckSlotRequirements(void)
+CheckSlotRequirements(bool repack)
 {
+	int		limit;
+
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
 	 * needs the same check.
 	 */
 
-	/* XXX we should be able to check exactly which type of slot we need */
-	if (max_replication_slots + max_repack_replication_slots == 0)
+	if (repack)
+		limit = max_repack_replication_slots;
+	else
+		limit = max_replication_slots;
+
+	if (limit == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if \"%s\" > 0 or \"%s\" > 0",
-						"max_replication_slots", "max_repack_replication_slots")));
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("replication slots can only be used if \"%s\" > 0",
+					   repack ? "max_repack_replication_slots" : "max_replication_slots"));
 
 	if (wal_level < WAL_LEVEL_REPLICA)
 		ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 78dd3c4ea66..16fbd383735 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -90,7 +90,7 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	create_physical_replication_slot(NameStr(*name),
 									 immediately_reserve,
@@ -164,6 +164,7 @@ create_logical_replication_slot(char *name, char *plugin,
 	 */
 	ctx = CreateInitDecodingContext(plugin, NIL,
 									false,	/* just catalogs is OK */
+									false,	/* not repack */
 									restart_lsn,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
@@ -203,7 +204,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	create_logical_replication_slot(NameStr(*name),
 									NameStr(*plugin),
@@ -240,7 +241,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	ReplicationSlotDrop(NameStr(*name), true);
 
@@ -648,9 +649,9 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 	CheckSlotPermissions();
 
 	if (logical_slot)
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 	else
-		CheckSlotRequirements();
+		CheckSlotRequirements(false);
 
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75ef3419a15..9d7d675fa96 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1240,7 +1240,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 
 		Assert(cmd->kind == REPLICATION_KIND_LOGICAL);
 
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 
 		/*
 		 * Initially create persistent slot as ephemeral - that allows us to
@@ -1309,6 +1309,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 		Assert(IsLogicalDecodingEnabled());
 
 		ctx = CreateInitDecodingContext(cmd->plugin, NIL, need_full_snapshot,
+										false,
 										InvalidXLogRecPtr,
 										XL_ROUTINE(.page_read = logical_read_xlog_page,
 												   .segment_open = WalSndSegmentOpen,
@@ -1466,7 +1467,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
 	QueryCompletion qc;
 
 	/* make sure that our requirements are still fulfilled */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	Assert(!MyReplicationSlot);
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index bc9d4ece672..bc075b16741 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -115,11 +115,12 @@ typedef struct LogicalDecodingContext
 } LogicalDecodingContext;
 
 
-extern void CheckLogicalDecodingRequirements(void);
+extern void CheckLogicalDecodingRequirements(bool repack);
 
 extern LogicalDecodingContext *CreateInitDecodingContext(const char *plugin,
 														 List *output_plugin_options,
 														 bool need_full_snapshot,
+														 bool for_repack,
 														 XLogRecPtr restart_lsn,
 														 XLogReaderRoutine *xl_routine,
 														 LogicalOutputPluginWriterPrepareWrite prepare_write,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c316a01a807..489af7d8d6c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -378,7 +378,7 @@ extern void ReplicationSlotDropAtPubNode(WalReceiverConn *wrconn, char *slotname
 extern void StartupReplicationSlots(void);
 extern void CheckPointReplicationSlots(bool is_shutdown);
 
-extern void CheckSlotRequirements(void);
+extern void CheckSlotRequirements(bool repack);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
 			GetSlotInvalidationCause(const char *cause_name);
-- 
2.47.3


--r2slln3zpmilwu22--





^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2026-04-03 19:01 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 06:52 psql tab completion for ALTER INDEX SET Masahiko Sawada <[email protected]>
2018-03-02 16:43 ` Fujii Masao <[email protected]>
2018-03-03 05:35   ` Masahiko Sawada <[email protected]>
2024-03-15 10:30 Re: remaining sql/json patches jian he <[email protected]>
2025-03-28 15:30 ` Re: On non-Windows, hard depend on uselocale(3) Peter Eisentraut <[email protected]>
2025-03-28 16:14   ` Re: On non-Windows, hard depend on uselocale(3) Masahiko Sawada <[email protected]>
2025-03-28 16:32     ` Re: On non-Windows, hard depend on uselocale(3) Peter Eisentraut <[email protected]>
2025-03-28 20:34       ` Re: On non-Windows, hard depend on uselocale(3) Peter Eisentraut <[email protected]>
2025-03-29 00:01       ` Re: On non-Windows, hard depend on uselocale(3) Masahiko Sawada <[email protected]>
2025-12-12 18:07 ` Re: On non-Windows, hard depend on uselocale(3) Tom Lane <[email protected]>
2026-04-03 19:01 [PATCH v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting Álvaro Herrera <[email protected]>
2026-04-03 19:01 [PATCH v51 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting Álvaro Herrera <[email protected]>
2026-04-03 19:01 [PATCH v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting Álvaro Herrera <[email protected]>
2026-04-03 19:01 [PATCH v51 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting Álvaro Herrera <[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