Re: Generate pg_stat_get_xact*() functions with Macros
Hi, On 3/27/23 3:20 AM, Michael Paquier wrote: On Sat, Mar 25, 2023 at 11:50:50AM +0900, Michael Paquier wrote: On Fri, Mar 24, 2023 at 06:58:30AM +0100, Drouvot, Bertrand wrote: - Does not include the refactoring for pg_stat_get_xact_function_total_time(), pg_stat_get_xact_function_self_time(), pg_stat_get_function_total_time() and pg_stat_get_function_self_time(). I think they can be done in a dedicated commit once we agree on the renaming for PG_STAT_GET_DBENTRY_FLOAT8 (see Andres's comment up-thread) so that the new macros can match the future agreement. Thanks for the reminder. I have completely missed that this is mentioned in [1], and that it is all about 8018ffb. The suggestion to prefix the macro names with a "_MS" to outline the conversion sounds like a good one seen from here. So please find attached a patch to do this adjustment, completed with a similar switch for the two counters of the function entries. Thanks! LGTM, but what about also taking care of pg_stat_get_xact_function_total_time() and pg_stat_get_xact_function_self_time() while at it? - Does include the refactoring of the new - pg_stat_get_xact_tuples_newpage_updated() function (added in - ae4fdde135) Fine by me. One step is better than no steps, and this helps: 1 file changed, 29 insertions(+), 97 deletions(-) I'll go apply that if there are no objections. Just did this part to shave a bit more code. Thanks! Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)
On Fri, Mar 24, 2023 at 3:05 PM Alvaro Herrera wrote:
>
> On 2023-Mar-23, Greg Stark wrote:
>
> > On Thu, 23 Mar 2023 at 23:30, Bharath Rupireddy
> > wrote:
> > >
> > > > + ereport(log_replication_commands ? LOG : DEBUG3,
> > > > + (errmsg("acquired physical replication slot \"%s\"",
> > > > + slotname)));
> >
> > So this is just a bit of bike-shedding but I don't feel like these log
> > messages really meet the standard we set for our logging. Like what
> > did the acquiring? What does "acquired" actually mean for a
> > replication slot? Is there not any meta information about the
> > acquisition that can give more context to the reader to make this
> > message more meaningful?
> >
> > I would expect a log message like this to say, I dunno, something like
> > "physical replication slot \"%s\" acquired by streaming TCP connection
> > to 192.168.0.1:999 at LSN ... with xxxMB of logs to read"
>
> Hmm, I don't disagree with your argument in principle, but I think this
> proposal is going too far. I think stating the PID is more than
> sufficient.
Do you mean to have something like "physical/logical replication slot
\"%s\" is released/acquired by PID %d", MyProcPid? If yes, the
log_line_prefix already contains PID right? Or do we want to cover the
cases when someone changes log_line_prefix to not contain PID?
> And I don't think we need this patch to go great lengths to
> explain what acquisition is, either; I mean, maybe that's a good thing
> to have, but then that's a different patch.
>
> > I even would be wondering if the other end shouldn't also be logging a
> > corresponding log and we shouldn't be going out of our way to ensure
> > there's enough information to match them up and presenting them in a
> > way that makes that easy.
>
> Hmm, you should be able to match things using the connection
> information. I don't think the slot acquisition operation in itself is
> that important.
Yeah, the intention of the patch is to track the patterns of slot
acquisitions and releases to aid analysis. Of course, this information
alone may not help but when matched with others in the logs, it will.
The v9 patch was failing because I was using MyReplicationSlot after
it got reset by slot release, attached v10 patch fixes it.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 626dc27e81142cddee656c0140f4570e952e728a Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy
Date: Fri, 24 Mar 2023 06:11:52 +
Subject: [PATCH v10] Add LOG messages when replication slots become active and
inactive
These logs will be extremely useful on production servers to debug
and analyze inactive replication slot issues.
---
doc/src/sgml/config.sgml| 7 ++--
src/backend/replication/slot.c | 51 +
src/backend/replication/walsender.c | 17 ++
src/backend/utils/misc/guc_tables.c | 2 +-
src/include/replication/slot.h | 2 ++
5 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 481f93cea1..f9b0d3c9a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7593,9 +7593,10 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
-Causes each replication command to be logged in the server log.
-See for more information about
-replication command. The default value is off.
+Causes each replication command and related activity to be logged in
+the server log. See for more
+information about replication command. The default value is
+off.
Only superusers and users with the appropriate SET
privilege can change this setting.
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2293c0c6fc..2caa8a3532 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -180,7 +180,16 @@ ReplicationSlotShmemExit(int code, Datum arg)
{
/* Make sure active replication slots are released */
if (MyReplicationSlot != NULL)
+ {
+ bool is_physical;
+ char *slotname;
+
+ is_physical = SlotIsPhysical(MyReplicationSlot);
+ slotname = pstrdup(NameStr(MyReplicationSlot->data.name));
ReplicationSlotRelease();
+ LogReplicationSlotRelease(is_physical, slotname);
+ pfree(slotname);
+ }
/* Also cleanup all the temporary slots. */
ReplicationSlotCleanup();
@@ -445,6 +454,9 @@ ReplicationSlotName(int index, Name name)
*
* An error is raised if nowait is true and the slot is currently in use. If
* nowait is false, we sleep until the slot is released by the owning process.
+ *
+ * Note: use LogReplicationSlotAcquire() if needed, to log a message after
+ * acquiring the replication slot.
*/
void
ReplicationSlotAcquire(const char *name, bool nowait)
@@ -542,6 +554,9 @@ retry:
*
* This or another backend can re-acquire the slot later.
* Resources this slot requires will be
RE: Data is copied twice when specifying both child and parent table in publication
On Mon, Mar 27, 2023 at 11:32 AM Amit Kapila wrote:
> On Mon, Mar 27, 2023 at 7:03 AM Peter Smith
> wrote:
> >
> >
> > 1.
> > +# two publications, one publishing through ancestor and another one
> > directly
> > +# publsihing the partition, with different row filters
> > +$node_publisher->safe_psql('postgres',
> > + "CREATE PUBLICATION tap_pub_viaroot_sync_1 FOR TABLE
> > tab_rowfilter_viaroot_part_sync WHERE (a > 15) WITH
> > (publish_via_partition_root)"
> > +);
> > +$node_publisher->safe_psql('postgres',
> > + "CREATE PUBLICATION tap_pub_viaroot_sync_2 FOR TABLE
> > tab_rowfilter_viaroot_part_sync_1 WHERE (a < 15)"
> > +);
> > +
> >
> > 1a.
> > Typo "publsihing"
Changed.
> > ~
> >
> > 1b.
> > IMO these table and publication names could be better.
> >
> > I thought it was confusing to have the word "sync" in these table
> > names and publication names. To the casual reader, it looks like these
> > are synchronous replication tests but they are not.
> >
>
> Hmm, sync here is for initial sync, so I don't think it is too much of
> a problem to understand if one is aware that these are logical
> replication related tests.
>
> > Similarly, I thought it was confusing that 2nd publication and table
> > have names with the word "viaroot" when the option
> > publish_via_partition_root is not even true.
> >
>
> I think the better names for tables could be
> "tab_rowfilter_parent_sync, tab_rowfilter_child_sync" and for
> publications "tap_pub_parent_sync_1,
> tap_pub_child_sync_1"
Changed. And removed "_1" in the suggested publication names.
Previously, I added "_1" and "_2" to distinguish between two publications with
the same name. However, the publication names are now different, so I think we
could remove "_1".
Attach the new patch.
Regards,
Wang Wei
v24-0001-Avoid-syncing-data-twice-for-the-publish_via_par.patch
Description: v24-0001-Avoid-syncing-data-twice-for-the-publish_via_par.patch
Re: Add pg_walinspect function with block info columns
On Sat, Mar 25, 2023 at 12:12:50PM +0900, Michael Paquier wrote: > I don't see any need to move this block of code? This leads to > unnecessary diffs, potentially making backpatch a bit harder. Either > way is not a big deal, still.. Except for this bit, 0001 looks fine > by me. FYI, I have gone through 0001 and applied it, after tweaking a bit the part about block references so as we have only one XLogRecHasAnyBlockRefs, with its StringInfoData used only locally. -- Michael signature.asc Description: PGP signature
Re: Add pg_walinspect function with block info columns
On Mon, Mar 27, 2023 at 9:11 AM Kyotaro Horiguchi
wrote:
>
> At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier
> wrote in
> > On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > OUT reltablespace oid,
> > OUT reldatabase oid,
> > OUT relfilenode oid,
> > OUT relblocknumber int8,
> > + OUT blockid int2,
> > +OUT start_lsn pg_lsn,
> > +OUT end_lsn pg_lsn,
> > +OUT prev_lsn pg_lsn,
> >
> > I'd still put the LSN data before the three OIDs for consistency with
> > the structures, though my opinion does not seem to count much..
>
> I agree with Michael on this point. Also, although it may not be
> significant for SQL, the rows are output in lsn order from the
> function.
Done that way.
On Sat, Mar 25, 2023 at 8:42 AM Michael Paquier wrote:
>
> On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > Please see the attached v4 patch set addressing all the review comments.
>
> - desc = GetRmgr(XLogRecGetRmid(record));
> - id = desc.rm_identify(XLogRecGetInfo(record));
> -
> - if (id == NULL)
> - id = psprintf("UNKNOWN (%x)", XLogRecGetInfo(record) &
> ~XLR_INFO_MASK);
> -
> - initStringInfo(_desc);
> - desc.rm_desc(_desc, record);
> -
> - /* Block references. */
> - initStringInfo(_blk_ref);
> - XLogRecGetBlockRefInfo(record, false, true, _blk_ref, _len);
> -
> - main_data_len = XLogRecGetDataLen(record);
>
> I don't see any need to move this block of code? This leads to
> unnecessary diffs, potentially making backpatch a bit harder. Either
> way is not a big deal, still.. Except for this bit, 0001 looks fine
> by me.
It's a cosmetic change - I wanted to keep the calculation of column
values closer to where they're assigned to Datum values. I agree to
not cause too much diff and removed them.
Please see the attached v5 patch set.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 601c47f0e56e721da7a5fb7e0427faa926ad982c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy
Date: Mon, 27 Mar 2023 03:50:44 +
Subject: [PATCH v5] Few optimizations in pg_walinspect
1. Emits null values in output columns (description and block_ref)
if WAL records have no data to present.
2. Function to get block references is skipped for the records
that don't have block references.
---
contrib/pg_walinspect/pg_walinspect.c | 39 ++-
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index 3b3215daf5..90fb8eaed1 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -187,7 +187,6 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
uint32 fpi_len = 0;
StringInfoData rec_desc;
StringInfoData rec_blk_ref;
- uint32 main_data_len;
int i = 0;
desc = GetRmgr(XLogRecGetRmid(record));
@@ -199,11 +198,11 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
initStringInfo(_desc);
desc.rm_desc(_desc, record);
- /* Block references. */
- initStringInfo(_blk_ref);
- XLogRecGetBlockRefInfo(record, false, true, _blk_ref, _len);
-
- main_data_len = XLogRecGetDataLen(record);
+ if (XLogRecHasAnyBlockRefs(record))
+ {
+ initStringInfo(_blk_ref);
+ XLogRecGetBlockRefInfo(record, false, true, _blk_ref, _len);
+ }
values[i++] = LSNGetDatum(record->ReadRecPtr);
values[i++] = LSNGetDatum(record->EndRecPtr);
@@ -212,10 +211,18 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
values[i++] = CStringGetTextDatum(desc.rm_name);
values[i++] = CStringGetTextDatum(id);
values[i++] = UInt32GetDatum(XLogRecGetTotalLen(record));
- values[i++] = UInt32GetDatum(main_data_len);
+ values[i++] = UInt32GetDatum(XLogRecGetDataLen(record));
values[i++] = UInt32GetDatum(fpi_len);
- values[i++] = CStringGetTextDatum(rec_desc.data);
- values[i++] = CStringGetTextDatum(rec_blk_ref.data);
+
+ if (rec_desc.len > 0)
+ values[i++] = CStringGetTextDatum(rec_desc.data);
+ else
+ nulls[i++] = true;
+
+ if (XLogRecHasAnyBlockRefs(record))
+ values[i++] = CStringGetTextDatum(rec_blk_ref.data);
+ else
+ nulls[i++] = true;
Assert(i == ncols);
}
@@ -232,6 +239,8 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
int block_id;
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ Assert(XLogRecHasAnyBlockRefs(record));
+
for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++)
{
DecodedBkpBlock *blk;
@@ -377,6 +386,11 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
while (ReadNextXLogRecord(xlogreader) &&
xlogreader->EndRecPtr <= end_lsn)
{
+ CHECK_FOR_INTERRUPTS();
+
+ if (!XLogRecHasAnyBlockRefs(xlogreader))
+ continue;
+
/* Use the tmp context so we can clean up after each tuple is done */
old_cxt = MemoryContextSwitchTo(tmp_cxt);
@@ -385,8 +399,6 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
/*
Re: Support logical replication of DDLs
On Sun, 26 Mar 2023 at 18:08, vignesh C wrote: > > On Thu, 23 Mar 2023 at 09:22, Ajin Cherian wrote: > > > > On Mon, Mar 20, 2023 at 8:17 PM houzj.f...@fujitsu.com > > wrote: > > > > > > Attach the new patch set which addressed above comments. > > > 0002,0003,0004 patch has been updated in this version. > > > > > > Best Regards, > > > Hou zj > > > > Attached a patch-set which adds support for ONLY token in ALTER TABLE.. > > Changes are in patches 0003 and 0004. > > Few comments: > 1) This file should not be included: > typedef struct > diff --git a/src/test/modules/test_ddl_deparse_regress/regression.diffs > b/src/test/modules/test_ddl_deparse_regress/regression.diffs > deleted file mode 100644 > index 3be15de..000 > --- a/src/test/modules/test_ddl_deparse_regress/regression.diffs > +++ /dev/null > @@ -1,847 +0,0 @@ > -diff -U3 > /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/expected/create_table.out > /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/results/create_table.out > > /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/expected/create_table.out > 2023-03-22 23:08:34.915184709 -0400 > -+++ > /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/results/create_table.out > 2023-03-22 23:09:46.810424685 -0400 Removed > 2) The patch does not apply neatly: > git am v82-0004-Introduce-the-test_ddl_deparse_regress-test-modu.patch > Applying: Introduce the test_ddl_deparse_regress test module. > .git/rebase-apply/patch:2489: trailing whitespace. > NOTICE: re-formed command: CREATE TABLE public.ctlt1_like (a > pg_catalog.text STORAGE main COLLATE pg_catalog."default" , b > pg_catalog.text STORAGE external COLLATE pg_catalog."default" , > CONSTRAINT ctlt1_a_check CHECK ((pg_catalog.length(a) > OPERATOR(pg_catalog.>) 2)), CONSTRAINT ctlt1_like_pkey PRIMARY KEY > (a)) > .git/rebase-apply/patch:2502: trailing whitespace. > NOTICE: re-formed command: ALTER TABLE public.test_alter_type ALTER > COLUMN quantity SET DATA TYPE pg_catalog.float4 > .git/rebase-apply/patch:2511: trailing whitespace. > -NOTICE: re-formed command: CREATE TABLE > public.test_alter_set_default (id pg_catalog.int4 STORAGE plain , > name pg_catalog."varchar" STORAGE extended COLLATE > pg_catalog."default" , description pg_catalog.text STORAGE extended > COLLATE pg_catalog."default" , price pg_catalog.float4 STORAGE plain > , quantity pg_catalog.int4 STORAGE plain , purchase_date > pg_catalog.date STORAGE plain ) > .git/rebase-apply/patch:2525: trailing whitespace. > -NOTICE: re-formed command: CREATE TABLE public.test_drop_default > (id pg_catalog.int4 STORAGE plain , name pg_catalog."varchar" > STORAGE extended COLLATE pg_catalog."default" , description > pg_catalog.text STORAGE extended COLLATE pg_catalog."default" , > price pg_catalog.float4 STORAGE plain , quantity pg_catalog.int4 > STORAGE plain , purchase_date pg_catalog.date STORAGE plain , > default_price pg_catalog.float4 STORAGE plainDEFAULT 10.0 , > default_name pg_catalog."varchar" STORAGE extended COLLATE > pg_catalog."default" DEFAULT 'foo'::character varying ) > .git/rebase-apply/patch:2538: trailing whitespace. > -NOTICE: re-formed command: CREATE TABLE public.test_set_not_null > (id pg_catalog.int4 STORAGE plain , name pg_catalog."varchar" > STORAGE extended COLLATE pg_catalog."default" , description > pg_catalog.text STORAGE extended COLLATE pg_catalog."default" , > price pg_catalog.float4 STORAGE plain , quantity pg_catalog.int4 > STORAGE plain , purchase_date pg_catalog.date STORAGE plain , > size pg_catalog.int4 STORAGE plain NOT NULL ) > warning: squelched 74 whitespace errors > warning: 79 lines add whitespace errors. fixed > 3) This file should not be included: > diff --git a/src/test/modules/test_ddl_deparse_regress/regression.out > b/src/test/modules/test_ddl_deparse_regress/regression.out > deleted file mode 100644 > index a44b91f..000 > --- a/src/test/modules/test_ddl_deparse_regress/regression.out > +++ /dev/null > @@ -1,7 +0,0 @@ > -test test_ddl_deparse ... ok 31 ms > -test create_extension ... ok 52 ms removed > 4) The test file should be included in meson.build also: > 't/027_nosuperuser.pl', > 't/028_row_filter.pl', > 't/029_on_error.pl', > 't/030_origin.pl', > 't/031_column_list.pl', > 't/032_subscribe_use_index.pl', > 't/100_bugs.pl', > ], Modified These issues are fixed in the patch attached at [1] [1] - https://www.postgresql.org/message-id/CALDaNm3XUKfD%2BnD1AVvSuZyUY_zRk_eyz%2BPt9t13N8WXViR6pw%40mail.gmail.com Regards, Vignesh
Re: Add pg_walinspect function with block info columns
At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier wrote in > On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote: > OUT reltablespace oid, > OUT reldatabase oid, > OUT relfilenode oid, > OUT relblocknumber int8, > + OUT blockid int2, > +OUT start_lsn pg_lsn, > +OUT end_lsn pg_lsn, > +OUT prev_lsn pg_lsn, > > I'd still put the LSN data before the three OIDs for consistency with > the structures, though my opinion does not seem to count much.. I agree with Michael on this point. Also, although it may not be significant for SQL, the rows are output in lsn order from the function. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID
On Sun, Mar 26, 2023 at 5:37 PM Kirk Wolak wrote:
> On Wed, Feb 8, 2023 at 10:56 AM Kirk Wolak wrote:
>
>> On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule
>> wrote:
>>
>>> hi
>>>
>>> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud
>>> napsal:
>>>
On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote:
>
> GET DIAGNOSTICS _oid = PG_ROUTINE_OID;
> RAISE NOTICE '... % ... %', _oid, _oid::regproc::text;
>
> Do you think it can be useful feature?
+1, it would have been quite handy in a few of my projects.
>>>
>>> it can looks like that
>>>
>>> create or replace function foo(a int)
>>> returns int as $$
>>> declare s text; n text; o oid;
>>> begin
>>> get diagnostics s = pg_current_routine_signature,
>>> n = pg_current_routine_name,
>>> o = pg_current_routine_oid;
>>> raise notice 'sign:%, name:%, oid:%', s, n, o;
>>> return a;
>>> end;
>>> $$ language plpgsql;
>>> CREATE FUNCTION
>>> (2023-02-08 09:04:03) postgres=# select foo(10);
>>> NOTICE: sign:foo(integer), name:foo, oid:16392
>>> ┌─┐
>>> │ foo │
>>> ╞═╡
>>> │ 10 │
>>> └─┘
>>> (1 row)
>>>
>>> The name - pg_routine_oid can be confusing, because there is not clean
>>> if it is oid of currently executed routine or routine from top of exception
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>
>> I agree that the name changed to pg_current_routine_... makes the most
>> sense, great call...
>>
>> +1
>>
>
> Okay, I reviewed this. I tested it (allocating too small of
> varchar's for values, various "signature types"),
> and also a performance test... Wow, on my VM, 10,000 Calls in a loop was
> 2-4ms...
>
> The names are clear. Again, I tested with various options, and including
> ROW_COUNT, or not.
>
> This functions PERFECTLY Except there are no documentation changes.
> Because of that, I set it to Waiting on Author.
> Which might be unfair, because I could take a stab at doing the
> documentation (but docs are not compiling on my setup yet).
>
> The documentation changes are simple enough.
> If I can get the docs compiled on my rig, I will see if I can make the
> changes, and post an updated patch,
> that contains both...
>
> But I don't want to be stepping on toes, or having it look like I am
> taking credit.
>
> Regards - Kirk
>
Okay, I have modified the documentation and made sure it compiles. They
were simple enough changes.
I am attaching this updated patch.
I have marked the item Ready for Commiter...
Thanks for your patience. I now have a workable hacking environment!
Regards - Kirk
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 7c8a49fe43..19dfe529cf 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1639,6 +1639,21 @@ GET DIAGNOSTICS integer_var = ROW_COUNT;
line(s) of text describing the current call stack
(see )
+
+ PG_CURRENT_ROUTINE_SIGNATURE
+ text
+ text describing the current routine with paramater
types
+
+
+ PG_CURRENT_ROUTINE_NAME
+ text
+ text name of the function without parenthesis
+
+
+ PG_CURRENT_ROUTINE_OID
+ oid
+ oid of the function currently running
+
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index b0a2cac227..bb2f3ff828 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2475,6 +2475,28 @@ exec_stmt_getdiag(PLpgSQL_execstate *estate,
PLpgSQL_stmt_getdiag *stmt)
}
break;
+ case PLPGSQL_GETDIAG_CURRENT_ROUTINE_NAME:
+ {
+ char *funcname;
+
+ funcname =
get_func_name(estate->func->fn_oid);
+ exec_assign_c_string(estate, var,
funcname);
+ if (funcname)
+ pfree(funcname);
+ }
+ break;
+
+ case PLPGSQL_GETDIAG_CURRENT_ROUTINE_OID:
+ exec_assign_value(estate, var,
+
ObjectIdGetDatum(estate->func->fn_oid),
+ false,
OIDOID, -1);
+ break;
+
+ case PLPGSQL_GETDIAG_CURRENT_ROUTINE_SIGNATURE:
+ exec_assign_c_string(estate, var,
+
estate->func->fn_signature);
+ break;
+
default:
elog(ERROR, "unrecognized diagnostic item kind:
%d",
Re: Data is copied twice when specifying both child and parent table in publication
On Mon, Mar 27, 2023 at 7:03 AM Peter Smith wrote:
>
>
> 1.
> +# two publications, one publishing through ancestor and another one directly
> +# publsihing the partition, with different row filters
> +$node_publisher->safe_psql('postgres',
> + "CREATE PUBLICATION tap_pub_viaroot_sync_1 FOR TABLE
> tab_rowfilter_viaroot_part_sync WHERE (a > 15) WITH
> (publish_via_partition_root)"
> +);
> +$node_publisher->safe_psql('postgres',
> + "CREATE PUBLICATION tap_pub_viaroot_sync_2 FOR TABLE
> tab_rowfilter_viaroot_part_sync_1 WHERE (a < 15)"
> +);
> +
>
> 1a.
> Typo "publsihing"
>
> ~
>
> 1b.
> IMO these table and publication names could be better.
>
> I thought it was confusing to have the word "sync" in these table
> names and publication names. To the casual reader, it looks like these
> are synchronous replication tests but they are not.
>
Hmm, sync here is for initial sync, so I don't think it is too much of
a problem to understand if one is aware that these are logical
replication related tests.
> Similarly, I thought it was confusing that 2nd publication and table
> have names with the word "viaroot" when the option
> publish_via_partition_root is not even true.
>
I think the better names for tables could be
"tab_rowfilter_parent_sync, tab_rowfilter_child_sync" and for
publications "tap_pub_parent_sync_1,
tap_pub_child_sync_1"
> ~~~
>
> 2.
>
> # The following commands are executed after CREATE SUBSCRIPTION, so these SQL
> # commands are for testing normal logical replication behavior.
> #
>
> ~
>
> I think you should add a couple of INSERTS for the newly added table/s
> also. IMO it is not only better for test completeness, but it causes
> readers to question why there are INSERTS for every other table except
> these ones.
>
The purpose of the test is to test the initial sync's interaction with
'publish_via_partition_root' option. So, adding Inserts after that for
replication doesn't serve any purpose and it also consumes test cycles
without any additional benefit. So, -1 for extending it further.
--
With Regards,
Amit Kapila.
Re: About the constant-TRUE clause in reconsider_outer_join_clauses
Richard Guo writes: > On Sat, Mar 25, 2023 at 11:41 PM Tom Lane wrote: >> Richard Guo writes: >>> Should we instead mark the constant-TRUE clause with required_relids >>> plus the OJ relid? >> I do not think it matters. > Yeah, I agree that it makes no difference currently. One day if we want > to replace the is_pushed_down flag with checking to see if a clause's > required_relids includes the OJ being formed in order to tell whether > it's a filter or join clause, I think we'd need to make this change. I did think about that ... but a constant-TRUE clause is going to be a no-op no matter which classification you give it. We do have some work to do in that area, but I think it's not an issue for this particular case. regards, tom lane
Re: About the constant-TRUE clause in reconsider_outer_join_clauses
On Sat, Mar 25, 2023 at 11:41 PM Tom Lane wrote: > Richard Guo writes: > > Should we instead mark the constant-TRUE clause with required_relids > > plus the OJ relid? > > I do not think it matters. Yeah, I agree that it makes no difference currently. One day if we want to replace the is_pushed_down flag with checking to see if a clause's required_relids includes the OJ being formed in order to tell whether it's a filter or join clause, I think we'd need to make this change. > > > Even if the join does become clauseless, it will end up being an > > unqualified nestloop. I think the join ordering algorithm will force > > this join to be formed when necessary. > > We would find *some* valid plan, but not necessarily a *good* plan. > The point of the dummy clause is to ensure that the join is considered > as soon as possible. That might not be the ideal join order of course, > but we'll consider it among other join orders and arrive at a cost-based > decision. With no dummy clause, the join order heuristics would always > delay this join as long as possible; so even if another ordering is > better, we'd not find it. I understand it now. Thanks for the explanation. Thanks Richard
Re: Initial Schema Sync for Logical Replication
On Fri, Mar 24, 2023 at 11:51 PM Kumar, Sachin wrote: > > > From: Amit Kapila > > > I think we won't be able to use same snapshot because the transaction will > > > be committed. > > > In CreateSubscription() we can use the transaction snapshot from > > > walrcv_create_slot() till walrcv_disconnect() is called.(I am not sure > > > about this part maybe walrcv_disconnect() calls the commits internally ?). > > > So somehow we need to keep this snapshot alive, even after transaction > > > is committed(or delay committing the transaction , but we can have > > > CREATE SUBSCRIPTION with ENABLED=FALSE, so we can have a restart > > > before tableSync is able to use the same snapshot.) > > > > > > > Can we think of getting the table data as well along with schema via > > pg_dump? Won't then both schema and initial data will correspond to the > > same snapshot? > > Right , that will work, Thanks! While it works, we cannot get the initial data in parallel, no? > > > > I think we can have same issues as you mentioned New table t1 is added > > > to the publication , User does a refresh publication. > > > pg_dump / pg_restore restores the table definition. But before > > > tableSync can start, steps from 2 to 5 happen on the publisher. > > > > 1. Create Table t1(c1, c2); --LSN: 90 2. Insert t1 (1, 1); --LSN 100 > > > > 3. Insert t1 (2, 2); --LSN 110 4. Alter t1 Add Column c3; --LSN 120 > > > > 5. Insert t1 (3, 3, 3); --LSN 130 > > > And table sync errors out > > > There can be one more issue , since we took the pg_dump without > > snapshot (wrt to replication slot). > > > > > > > To avoid both the problems mentioned for Refresh Publication, we can do > > one of the following: (a) create a new slot along with a snapshot for this > > operation and drop it afterward; or (b) using the existing slot, establish a > > new snapshot using a technique proposed in email [1]. > > > > Thanks, I think option (b) will be perfect, since we don’t have to create a > new slot. Regarding (b), does it mean that apply worker stops streaming, requests to create a snapshot, and then resumes the streaming? Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Amcheck verification of GiST and GIN
On Sun, Mar 19, 2023 at 4:00 PM Andrey Borodin wrote: > After several attempts to corrupt GiST with this 0.01 epsilon > adjustment tolerance I think GiST indexing of points is valid. > Because intersection for search purposes is determined with the same epsilon! > So it's kind of odd > postgres=# select point(0.001,0)~=point(0,0); > ?column? > -- > t > (1 row) > , yet the index works correctly. I think that it's okay, provided that we can assume deterministic behavior in the code that forms new index tuples. Within nbtree, operator classes like numeric_ops are supported by heapallindexed verification, without any requirement for special normalization code to make it work correctly as a special case. This is true even though operator classes such as numeric_ops have similar "equality is not equivalence" issues, which comes up in other areas (e.g., nbtree deduplication, which must call support routine 4 during a CREATE INDEX [1]). The important principle is that amcheck must always be able to produce a consistent fingerprintable binary output given the same input (the same heap tuple/Datum array). This must work across all operator classes that play by the rules for GiST operator classes. We *can* tolerate some variation here. Well, we really *have* to tolerate a little of this kind of variation in order to deal with the TOAST input state thing...but I hope that that's the only complicating factor here, for GiST (as it is for nbtree). Note that we already rely on the fact that index_form_tuple() uses palloc0() (not plain palloc) in verify_nbtree.c, for the obvious reason. I think that there is a decent chance that it just wouldn't make sense for an operator class author to ever do something that we need to worry about. I'm pretty sure that it's just the TOAST thing. But it's worth thinking about carefully. [1] https://www.postgresql.org/docs/devel/btree-support-funcs.html -- Peter Geoghegan
Re: Documentation Not Compiling (http://docbook... not https:.//...)
On Sun, Mar 26, 2023 at 9:12 PM Kirk Wolak wrote: > Andres, > Apologies to pick on you directly. > But it appears that sites are refusing HTTP requests, > and it's affecting compilation of docs in a new configuration. > > I was surprised to see NON-HTTPS references in 2023, tbh... > I cannot even curl these references. > > Maybe I am missing a simple flag... > > Or should I offer to search/replace to fix everything to HTTPS, > and submit a patch? > > Regards, Kirk > Okay, for future reference I had to install a few things (fop, dbtoepub, docbook-xsl) Not sure why the original ./configure did not bring those in... Regards, Kirk
Re: Raising the SCRAM iteration count
On Sun, Mar 26, 2023 at 11:14:37PM +0200, Daniel Gustafsson wrote: > > On 25 Mar 2023, at 01:56, Michael Paquier wrote: > > > > On Fri, Mar 24, 2023 at 09:56:29AM +0100, Daniel Gustafsson wrote: > >> I've actually ripped out the test in question in the attached v9 to have it > >> ready and building green in CFbot. > > > > While reading through v9, I have noticed a few things. > > The attached rebase fixes all of these comments, and features a slightly > reworded commit message. I plan to go ahead with this tomorrow to close the > CF > patch item. Looks OK by me. + "SELECT substr(rolpassword,1,19) I would have perhaps used a regexp_replace() for that. What you have here is of course fine, so feel free to ignore :p -- Michael signature.asc Description: PGP signature
Re: Data is copied twice when specifying both child and parent table in publication
Here are some review comments for v23-0001.
==
src/test/subscription/t/028_row_filter.pl
1.
+# two publications, one publishing through ancestor and another one directly
+# publsihing the partition, with different row filters
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_viaroot_sync_1 FOR TABLE
tab_rowfilter_viaroot_part_sync WHERE (a > 15) WITH
(publish_via_partition_root)"
+);
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_viaroot_sync_2 FOR TABLE
tab_rowfilter_viaroot_part_sync_1 WHERE (a < 15)"
+);
+
1a.
Typo "publsihing"
~
1b.
IMO these table and publication names could be better.
I thought it was confusing to have the word "sync" in these table
names and publication names. To the casual reader, it looks like these
are synchronous replication tests but they are not.
Similarly, I thought it was confusing that 2nd publication and table
have names with the word "viaroot" when the option
publish_via_partition_root is not even true.
~~~
2.
# The following commands are executed after CREATE SUBSCRIPTION, so these SQL
# commands are for testing normal logical replication behavior.
#
~
I think you should add a couple of INSERTS for the newly added table/s
also. IMO it is not only better for test completeness, but it causes
readers to question why there are INSERTS for every other table except
these ones.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Re: logical decoding and replication of sequences, take 2
Hi, On Fri, Mar 24, 2023 at 7:26 AM Tomas Vondra wrote: > > I merged the earlier "fixup" patches into the relevant parts, and left > two patches with new tweaks (deducing the corrent "WAL" state from the > current state read by copy_sequence), and the interlock discussed here. > Apart from that, how does the publication having sequences work with subscribers who are not able to handle sequence changes, e.g. in a case where PostgreSQL version of publication is newer than the subscriber? As far as I tested the latest patches, the subscriber (v15) errors out with the error 'invalid logical replication message type "Q"' when receiving a sequence change. I'm not sure it's sensible behavior. I think we should instead either (1) deny starting the replication if the subscriber isn't able to handle sequence changes and the publication includes that, or (2) not send sequence changes to such subscribers. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Request for comment on setting binary format output per session
Dave Cramer writes: > On Sun, 26 Mar 2023 at 18:12, Tom Lane wrote: >> I would not expect DISCARD ALL to reset a session-level property. > Well if we can't reset it with DISCARD ALL how would that work with > pgbouncer, or any pool for that matter since it doesn't know which client > asked for which (if any) OID's to be binary. Well, it'd need to know that, just like it already needs to know which clients asked for which database or which login role. Having DISCARD ALL reset those session properties is obviously silly. The way I'm imagining this working is that it fits into the framework for protocol options (cf commits ae65f6066 and bbf9c282c), whereby the client and server negotiate whether they can handle this feature. A non-updated pooler would act like a server that doesn't know the feature, and the client would have to fall back to not using it, just as it would with an older server. I doubt that this would crimp a pooler's freedom of action very much. In any given environment there will probably be only a few values of the set-of-okay-types in use. regards, tom lane
Re: Documentation Not Compiling (http://docbook... not https:.//...)
On Sun, Mar 26, 2023 at 09:12:35PM -0400, Kirk Wolak wrote: > Andres, > Apologies to pick on you directly. > But it appears that sites are refusing HTTP requests, > and it's affecting compilation of docs in a new configuration. > > I was surprised to see NON-HTTPS references in 2023, tbh... > I cannot even curl these references. > > Maybe I am missing a simple flag... > > Or should I offer to search/replace to fix everything to HTTPS, > and submit a patch? See 969509c3f2e3b4c32dcf264f9d642b5ef01319f3 Do you have the necessary packages installed for your platform (which platform?). -- Justin
Re: Generate pg_stat_get_xact*() functions with Macros
On Sat, Mar 25, 2023 at 11:50:50AM +0900, Michael Paquier wrote:
> On Fri, Mar 24, 2023 at 06:58:30AM +0100, Drouvot, Bertrand wrote:
>> - Does not include the refactoring for
>> pg_stat_get_xact_function_total_time(),
>> pg_stat_get_xact_function_self_time(),
>> pg_stat_get_function_total_time() and
>> pg_stat_get_function_self_time(). I think they can be done in a
>> dedicated commit once we agree on the renaming for
>> PG_STAT_GET_DBENTRY_FLOAT8 (see Andres's comment up-thread) so that
>> the new macros can match the future agreement.
Thanks for the reminder. I have completely missed that this is
mentioned in [1], and that it is all about 8018ffb. The suggestion to
prefix the macro names with a "_MS" to outline the conversion sounds
like a good one seen from here. So please find attached a patch to do
this adjustment, completed with a similar switch for the two counters
of the function entries.
>> - Does include the refactoring of the new
>> - pg_stat_get_xact_tuples_newpage_updated() function (added in
>> - ae4fdde135)
>
> Fine by me. One step is better than no steps, and this helps:
> 1 file changed, 29 insertions(+), 97 deletions(-)
>
> I'll go apply that if there are no objections.
Just did this part to shave a bit more code.
[1]:
https://www.postgresql.org/message-id/20230111225907.6el6c5j3hukiz...@awork3.anarazel.de
--
Michael
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index e1dd1e0ad3..ff155e003f 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -152,29 +152,26 @@ pg_stat_get_function_calls(PG_FUNCTION_ARGS)
PG_RETURN_INT64(funcentry->numcalls);
}
-Datum
-pg_stat_get_function_total_time(PG_FUNCTION_ARGS)
-{
- Oid funcid = PG_GETARG_OID(0);
- PgStat_StatFuncEntry *funcentry;
-
- if ((funcentry = pgstat_fetch_stat_funcentry(funcid)) == NULL)
- PG_RETURN_NULL();
- /* convert counter from microsec to millisec for display */
- PG_RETURN_FLOAT8(((double) funcentry->total_time) / 1000.0);
+/* convert counter from microsec to millisec for display */
+#define PG_STAT_GET_FUNCENTRY_FLOAT8_MS(stat) \
+Datum\
+CppConcat(pg_stat_get_function_,stat)(PG_FUNCTION_ARGS)\
+{ \
+ Oid funcid = PG_GETARG_OID(0); \
+ double result;\
+ PgStat_StatFuncEntry *funcentry;\
+ \
+ if ((funcentry = pgstat_fetch_stat_funcentry(funcid)) == NULL) \
+ PG_RETURN_NULL(); \
+ result = ((double) funcentry->stat) / 1000.0; \
+ PG_RETURN_FLOAT8(result); \
}
-Datum
-pg_stat_get_function_self_time(PG_FUNCTION_ARGS)
-{
- Oid funcid = PG_GETARG_OID(0);
- PgStat_StatFuncEntry *funcentry;
+/* pg_stat_get_function_total_time */
+PG_STAT_GET_FUNCENTRY_FLOAT8_MS(total_time)
- if ((funcentry = pgstat_fetch_stat_funcentry(funcid)) == NULL)
- PG_RETURN_NULL();
- /* convert counter from microsec to millisec for display */
- PG_RETURN_FLOAT8(((double) funcentry->self_time) / 1000.0);
-}
+/* pg_stat_get_function_self_time */
+PG_STAT_GET_FUNCENTRY_FLOAT8_MS(self_time)
Datum
pg_stat_get_backend_idset(PG_FUNCTION_ARGS)
@@ -1147,7 +1144,8 @@ pg_stat_get_db_checksum_last_failure(PG_FUNCTION_ARGS)
PG_RETURN_TIMESTAMPTZ(result);
}
-#define PG_STAT_GET_DBENTRY_FLOAT8(stat) \
+/* convert counter from microsec to millisec for display */
+#define PG_STAT_GET_DBENTRY_FLOAT8_MS(stat) \
Datum \
CppConcat(pg_stat_get_db_,stat)(PG_FUNCTION_ARGS)\
{\
@@ -1164,19 +1162,19 @@ CppConcat(pg_stat_get_db_,stat)(PG_FUNCTION_ARGS)\
}
/* pg_stat_get_db_active_time */
-PG_STAT_GET_DBENTRY_FLOAT8(active_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(active_time)
/* pg_stat_get_db_blk_read_time */
-PG_STAT_GET_DBENTRY_FLOAT8(blk_read_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(blk_read_time)
/* pg_stat_get_db_blk_write_time */
-PG_STAT_GET_DBENTRY_FLOAT8(blk_write_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(blk_write_time)
/* pg_stat_get_db_idle_in_transaction_time */
-PG_STAT_GET_DBENTRY_FLOAT8(idle_in_transaction_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(idle_in_transaction_time)
/* pg_stat_get_db_session_time */
-PG_STAT_GET_DBENTRY_FLOAT8(session_time)
+PG_STAT_GET_DBENTRY_FLOAT8_MS(session_time)
Datum
pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS)
signature.asc
Description: PGP signature
Re: meson/msys2 fails with plperl/Strawberry
> On Mar 26, 2023, at 5:28 PM, Andres Freund wrote: > > Hi, > >> On 2023-03-26 12:39:08 -0700, Andres Freund wrote: >> First: I am *not* arguing we shouldn't repair building against strawberry >> perl >> with mingw. > > Hm - can you describe the failure more - I just tried, and it worked to build > against strawberry perl on mingw, without any issues. All I did was set > -DPERL="c:/strawberrly/perl/bin/perl.exe". > > That might be the secret sauce I’m missing. I will be offline for a day or three, will test when I’m back. Cheers Andrew
Documentation Not Compiling (http://docbook... not https:.//...)
Andres, Apologies to pick on you directly. But it appears that sites are refusing HTTP requests, and it's affecting compilation of docs in a new configuration. I was surprised to see NON-HTTPS references in 2023, tbh... I cannot even curl these references. Maybe I am missing a simple flag... Or should I offer to search/replace to fix everything to HTTPS, and submit a patch? Regards, Kirk
Re: Request for comment on setting binary format output per session
Dave Cramer On Sun, 26 Mar 2023 at 18:12, Tom Lane wrote: > Dave Cramer writes: > > Well I was presuming that they would just pass the parameter on. If they > > didn't then binary_format won't work with them. In the case that they do > > pass it on, then DISCARD_ALL will reset it and future borrows of the > > connection will have no way to set it again; effectively making this a > one > > time setting. > > I would not expect DISCARD ALL to reset a session-level property. > Well if we can't reset it with DISCARD ALL how would that work with pgbouncer, or any pool for that matter since it doesn't know which client asked for which (if any) OID's to be binary. Dave
Re: Request for comment on setting binary format output per session
Dave Cramer writes: > Well I was presuming that they would just pass the parameter on. If they > didn't then binary_format won't work with them. In the case that they do > pass it on, then DISCARD_ALL will reset it and future borrows of the > connection will have no way to set it again; effectively making this a one > time setting. I would not expect DISCARD ALL to reset a session-level property. regards, tom lane
Re: Time to move pg_test_timing to measure in nanoseconds
Sure, will do. On Sun, Mar 26, 2023 at 11:40 PM Andres Freund wrote: > > Hi, > > On 2023-03-26 16:43:21 +0200, Hannu Krosing wrote: > > Currently pg_test_timing utility measures its timing overhead in > > microseconds, giving results like this > > I have a patch that does that and a bit more that's included in a larger > patchset by David Geier: > https://postgr.es/m/198ef658-a5b7-9862-2017-faf85d59e3a8%40gmail.com > > Could you review that part of the patchset? > > Greetings, > > Andres Freund
Re: Request for comment on setting binary format output per session
On Sun, 26 Mar 2023 at 14:00, Jeff Davis wrote: > On Sat, 2023-03-25 at 19:58 -0400, Dave Cramer wrote: > > Well that means that connection poolers have to all be fixed. There > > are more than just pgbouncer. > > Seems rather harsh that a new feature breaks a connection pooler or > > makes the pooler unusable. > > Would it actually break connection poolers as they are now? Or would, > for example, pgbouncer just not set the binary_format parameter on the > outbound connection, and therefore just return everything as text until > they add support to configure it? > Well I was presuming that they would just pass the parameter on. If they didn't then binary_format won't work with them. In the case that they do pass it on, then DISCARD_ALL will reset it and future borrows of the connection will have no way to set it again; effectively making this a one time setting. So while it may not break them it doesn't seem like it is a very useful solution. Dave
Re: refactoring relation extension and BufferAlloc(), faster COPY
Hi,
Below is my review of a slightly older version than you just posted --
much of it you may have already addressed.
>From 3a6c3f41000e057bae12ab4431e6bb1c5f3ec4b0 Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Mon, 20 Mar 2023 21:57:40 -0700
Subject: [PATCH v5 01/15] createdb-using-wal-fixes
This could use a more detailed commit message -- I don't really get what
it is doing
>From 6faba69c241fd5513022bb042c33af09d91e84a6 Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Wed, 1 Jul 2020 19:06:45 -0700
Subject: [PATCH v5 02/15] Add some error checking around pinning
---
src/backend/storage/buffer/bufmgr.c | 40 -
src/include/storage/bufmgr.h| 1 +
2 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
index 95212a3941..fa20fab5a2 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4283,6 +4287,25 @@ ConditionalLockBuffer(Buffer buffer)
LW_EXCLUSIVE);
}
+void
+BufferCheckOneLocalPin(Buffer buffer)
+{
+if (BufferIsLocal(buffer))
+{
+/* There should be exactly one pin */
+if (LocalRefCount[-buffer - 1] != 1)
+elog(ERROR, "incorrect local pin count: %d",
+LocalRefCount[-buffer - 1]);
+}
+else
+{
+/* There should be exactly one local pin */
+if (GetPrivateRefCount(buffer) != 1)
I'd rather this be an else if (was already like this, but, no reason not
to change it now)
+elog(ERROR, "incorrect local pin count: %d",
+GetPrivateRefCount(buffer));
+}
+}
>From 00d3044770478eea31e00fee8d1680f22ca6adde Mon Sep 17 00:00:00 2001
From: Andres Freund
Date: Mon, 27 Feb 2023 17:36:37 -0800
Subject: [PATCH v5 04/15] Add smgrzeroextend(), FileZero(), FileFallocate()
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 9fd8444ed4..c34ed41d52 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -2206,6 +2206,92 @@ FileSync(File file, uint32 wait_event_info)
return returnCode;
}
+/*
+ * Zero a region of the file.
+ *
+ * Returns 0 on success, -1 otherwise. In the latter case errno is set to the
+ * appropriate error.
+ */
+int
+FileZero(File file, off_t offset, off_t amount, uint32 wait_event_info)
+{
+intreturnCode;
+ssize_twritten;
+
+Assert(FileIsValid(file));
+returnCode = FileAccess(file);
+if (returnCode < 0)
+return returnCode;
+
+pgstat_report_wait_start(wait_event_info);
+written = pg_pwrite_zeros(VfdCache[file].fd, amount, offset);
+pgstat_report_wait_end();
+
+if (written < 0)
+return -1;
+else if (written != amount)
this doesn't need to be an else if
+{
+/* if errno is unset, assume problem is no disk space */
+if (errno == 0)
+errno = ENOSPC;
+return -1;
+}
+int
+FileFallocate(File file, off_t offset, off_t amount, uint32 wait_event_info)
+{
+#ifdef HAVE_POSIX_FALLOCATE
+intreturnCode;
+
+Assert(FileIsValid(file));
+returnCode = FileAccess(file);
+if (returnCode < 0)
+return returnCode;
+
+pgstat_report_wait_start(wait_event_info);
+returnCode = posix_fallocate(VfdCache[file].fd, offset, amount);
+pgstat_report_wait_end();
+
+if (returnCode == 0)
+return 0;
+
+/* for compatibility with %m printing etc */
+errno = returnCode;
+
+/*
+* Return in cases of a "real" failure, if fallocate is not supported,
+* fall through to the FileZero() backed implementation.
+*/
+if (returnCode != EINVAL && returnCode != EOPNOTSUPP)
+return returnCode;
I'm pretty sure you can just delete the below if statement
+if (returnCode == 0 ||
+(returnCode != EINVAL && returnCode != EINVAL))
+return returnCode;
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 352958e1fe..59a65a8305 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -28,6 +28,7 @@
#include "access/xlog.h"
#include "access/xlogutils.h"
#include "commands/tablespace.h"
+#include "common/file_utils.h"
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
@@ -500,6 +501,116 @@ mdextend(SMgrRelation reln, ForkNumber forknum,
BlockNumber blocknum,
Assert(_mdnblocks(reln, forknum, v) <= ((BlockNumber) RELSEG_SIZE));
}
+/*
+ *mdzeroextend() -- Add ew zeroed out blocks to the specified relation.
not sure what ew is
+ *
+ *Similar to mdrextend(), except the relation can be extended by
mdrextend->mdextend
+ *multiple blocks at once, and that the added blocks will be
filled with
I would lose the comma and just say "and the added blocks will be filled..."
+void
+mdzeroextend(SMgrRelation reln, ForkNumber forknum,
+BlockNumber blocknum, int
Re: Time to move pg_test_timing to measure in nanoseconds
Hi, On 2023-03-26 16:43:21 +0200, Hannu Krosing wrote: > Currently pg_test_timing utility measures its timing overhead in > microseconds, giving results like this I have a patch that does that and a bit more that's included in a larger patchset by David Geier: https://postgr.es/m/198ef658-a5b7-9862-2017-faf85d59e3a8%40gmail.com Could you review that part of the patchset? Greetings, Andres Freund
Re: possible proposal plpgsql GET DIAGNOSTICS oid = PG_ROUTINE_OID
On Wed, Feb 8, 2023 at 10:56 AM Kirk Wolak wrote: > On Wed, Feb 8, 2023 at 3:08 AM Pavel Stehule > wrote: > >> hi >> >> st 8. 2. 2023 v 7:33 odesílatel Julien Rouhaud >> napsal: >> >>> On Tue, Feb 07, 2023 at 08:48:22PM +0100, Pavel Stehule wrote: >>> > >>> > GET DIAGNOSTICS _oid = PG_ROUTINE_OID; >>> > RAISE NOTICE '... % ... %', _oid, _oid::regproc::text; >>> > >>> > Do you think it can be useful feature? >>> >>> +1, it would have been quite handy in a few of my projects. >>> >> >> it can looks like that >> >> create or replace function foo(a int) >> returns int as $$ >> declare s text; n text; o oid; >> begin >> get diagnostics s = pg_current_routine_signature, >> n = pg_current_routine_name, >> o = pg_current_routine_oid; >> raise notice 'sign:%, name:%, oid:%', s, n, o; >> return a; >> end; >> $$ language plpgsql; >> CREATE FUNCTION >> (2023-02-08 09:04:03) postgres=# select foo(10); >> NOTICE: sign:foo(integer), name:foo, oid:16392 >> ┌─┐ >> │ foo │ >> ╞═╡ >> │ 10 │ >> └─┘ >> (1 row) >> >> The name - pg_routine_oid can be confusing, because there is not clean if >> it is oid of currently executed routine or routine from top of exception >> >> Regards >> >> Pavel >> > > I agree that the name changed to pg_current_routine_... makes the most > sense, great call... > > +1 > Okay, I reviewed this. I tested it (allocating too small of varchar's for values, various "signature types"), and also a performance test... Wow, on my VM, 10,000 Calls in a loop was 2-4ms... The names are clear. Again, I tested with various options, and including ROW_COUNT, or not. This functions PERFECTLY Except there are no documentation changes. Because of that, I set it to Waiting on Author. Which might be unfair, because I could take a stab at doing the documentation (but docs are not compiling on my setup yet). The documentation changes are simple enough. If I can get the docs compiled on my rig, I will see if I can make the changes, and post an updated patch, that contains both... But I don't want to be stepping on toes, or having it look like I am taking credit. Regards - Kirk
Re: meson/msys2 fails with plperl/Strawberry
Hi, On 2023-03-26 12:39:08 -0700, Andres Freund wrote: > First: I am *not* arguing we shouldn't repair building against strawberry perl > with mingw. Hm - can you describe the failure more - I just tried, and it worked to build against strawberry perl on mingw, without any issues. All I did was set -DPERL="c:/strawberrly/perl/bin/perl.exe". Greetings, Andres Freund
Re: Support logical replication of DDLs
vignesh C writes: > [ YA patch set ] I spent some time looking through this thread to try to get a sense of the state of things, and I came away quite depressed. The patchset has ballooned to over 2MB, which is a couple orders of magnitude larger than anyone could hope to meaningfully review from scratch. Despite that, it seems that there are fundamental semantics issues remaining, not to mention clear-and-present security dangers, not to mention TODO comments all over the code. I'm also less than sold on the technical details, specifically the notion of "let's translate utility parse trees into JSON and send that down the wire". You can probably make that work for now, but I wonder if it will be any more robust against cross-version changes than just shipping the outfuncs.c representation. (Perhaps it can be made more robust than the raw parse trees, but I see no evidence that anyone's thought much about how.) And TBH, I don't think that I quite believe the premise in the first place. The whole point of using logical rather than physical replication is that the subscriber installation(s) aren't exactly like the publisher. Given that, how can we expect that automated DDL replication is going to do the right thing often enough to be a useful tool rather than a disastrous foot-gun? The more you expand the scope of what gets replicated, the worse that problem becomes --- for example, I don't buy for one second that "let's replicate roles" is a credible solution for the problems that come from the roles not being the same on publisher and subscriber. I'm not sure how we get from here to a committable and useful feature, but I don't think we're close to that yet, and I'm not sure that minor iterations on a 2MB patchset will accomplish much. I'm afraid that a whole lot of work is going to end up going down the drain, which would be a shame because surely there are use-cases here. I suggest taking a couple of steps back from the minutiae of the patch, and spending some hard effort thinking about how the thing would be controlled in a useful fashion (that is, a real design for the filtering that was mentioned at the very outset), and about the security issues, and about how we could get to a committable patch. If you ask me, a committable initial patch could be at most about a tenth the size of what's here, which means that the functionality goals for the first version have to be stripped way back. [ Now, where did I put my flameproof vest? ] regards, tom lane
Re: Raising the SCRAM iteration count
> On 25 Mar 2023, at 01:56, Michael Paquier wrote: > > On Fri, Mar 24, 2023 at 09:56:29AM +0100, Daniel Gustafsson wrote: >> I've actually ripped out the test in question in the attached v9 to have it >> ready and building green in CFbot. > > While reading through v9, I have noticed a few things. The attached rebase fixes all of these comments, and features a slightly reworded commit message. I plan to go ahead with this tomorrow to close the CF patch item. -- Daniel Gustafsson v10-0001-Make-SCRAM-iteration-count-configurable.patch Description: Binary data
Re: meson/msys2 fails with plperl/Strawberry
On 2023-03-26 07:57:59 -0400, Andrew Dunstan wrote:
>
> On 2023-03-25 Sa 12:38, Andres Freund wrote:
> > Hi,
> >
> > On 2023-03-25 08:46:42 -0400, Andrew Dunstan wrote:
> > > config/perl.m4 contains this:
> > >
> > >
> > > AC_MSG_CHECKING(for flags to link embedded Perl)
> > > if test "$PORTNAME" = "win32" ; then
> > > perl_lib=`basename $perl_archlibexp/CORE/perl[[5-9]]*.lib .lib`
> > > if test -e "$perl_archlibexp/CORE/$perl_lib.lib"; then
> > > perl_embed_ldflags="-L$perl_archlibexp/CORE -l$perl_lib"
> > > else
> > > perl_lib=`basename $perl_archlibexp/CORE/libperl[[5-9]]*.a
> > > .a | sed 's/^lib//'`
> > > if test -e "$perl_archlibexp/CORE/lib$perl_lib.a"; then
> > > perl_embed_ldflags="-L$perl_archlibexp/CORE -l$perl_lib"
> > > fi
> > > fi
> > > else
> > > pgac_tmp1=`$PERL -MExtUtils::Embed -e ldopts`
> > > pgac_tmp2=`$PERL -MConfig -e 'print "$Config{ccdlflags}
> > > $Config{ldflags}"'`
> > > perl_embed_ldflags=`echo X"$pgac_tmp1" | sed -e "s/^X//" -e
> > > "s%$pgac_tmp2%%"`
> > > fi
> > > AC_SUBST(perl_embed_ldflags)dnl
> > >
> > > I don't see any equivalent in meson.build of the win32 logic, and thus I
> > > am
> > > getting a setup failure on fairywren when trying to move it to meson,
> > > while
> > > it will happily build with autoconf.
> > I did not try to build with strawberry perl using mingw - it doesn't seem
> > like
> > a very interesting thing, given that mingw has a much more reasonable perl
> > than strawberry - but with the mingw perl it works well.
>
>
> Strawberry is a recommended perl installation for Windows
> () and is widely used AFAICT.
It also hasn't released anything in years, including security fixes, dumps
broken binaries alongside the directory containing perl.
> In general my approach has been to build as independently as possible from
> msys2 infrastructure, in particular a) not to rely on it at all for MSVC
> builds and b) to use independent third party installations for things like
> openssl and PLs.
Note that the msvc CI build *does* use strawberry perl.
First: I am *not* arguing we shouldn't repair building against strawberry perl
with mingw.
But I fail to see what we gain by using builds of openssl etc from random
places - all that achieves is making it very hard to reproduce problems. Given
how few users mingw built windows has, that's the opposite of what we should
do.
> In any case, I don't think we should be choosing gratuitously to break
> things that hitherto worked, however uninteresting you personally might find
> them.
I didn't gratuitously do so. I didn't even know it was broken - as I said
above, CI tests build with strawberry perl many times a day. I spent plenty
time figuring out why newer perl versions were broken on windows.
> > The above logic actually did *not* work well with mingw for me, because the
> > names are not actually what configure expects, and it seems like a seriously
> > bad idea to encode that much knowledge about library naming and locations.
>
>
> Didn't work well how? It just worked perfectly for me with ucrt perl (setup,
> built and tested) using configure:
>
> $ grep perl532 config.log
> configure:10482: result: -LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE
> -lperl532
> configure:18820: gcc -o conftest.exe -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
> -O2 -I./src/include/port/win32
> -IC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE
> -Wl,--allow-multiple-definition -Wl,--disable-auto-import conftest.c
> -LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE -lperl532 >&5
> perl_embed_ldflags='-LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE
> -lperl532'
I got mismatches around library names, because some of the win32 specific
pattern matches didn't apply or applied over broadly. I don't have a windows
system running right now, I'll try to reproduce in the next few days.
> > > I would expect the ld flags to be "-LC:/STRAWB~1/perl/lib/CORE -lperl532"
> > You didn't say what they ended up as?
>
>
> I think you misunderstand me. This is what they should end up as.
I know. Without knowing what they *did* end up as, it's hard to compare, no?
Greetings,
Andres Freund
Re: Request for comment on setting binary format output per session
On Sat, 2023-03-25 at 19:58 -0400, Dave Cramer wrote: > Well that means that connection poolers have to all be fixed. There > are more than just pgbouncer. > Seems rather harsh that a new feature breaks a connection pooler or > makes the pooler unusable. Would it actually break connection poolers as they are now? Or would, for example, pgbouncer just not set the binary_format parameter on the outbound connection, and therefore just return everything as text until they add support to configure it? I'll admit that GUCs wouldn't have this problem at all, but it would be nice to know how much of a problem it is before we decide between a protocol extension and a GUC. Regards, Jeff Davis
Re: Schema variables - new implementation for Postgres 15
> On Sun, Mar 26, 2023 at 07:32:05PM +0800, Julien Rouhaud wrote: > Hi, > > I just have a few minor wording improvements for the various comments / > documentation you quoted. Talking about documentation I've noticed that the implementation contains few limitations, that are not mentioned in the docs. Examples are WITH queries: WITH x AS (LET public.svar = 100) SELECT * FROM x; ERROR: LET not supported in WITH query and using with set-returning functions (haven't found any related tests). Another small note is about this change in the rowsecurity: /* -* For SELECT, UPDATE and DELETE, add security quals to enforce the USING -* policies. These security quals control access to existing table rows. -* Restrictive policies are combined together using AND, and permissive -* policies are combined together using OR. +* For SELECT, LET, UPDATE and DELETE, add security quals to enforce the +* USING policies. These security quals control access to existing table +* rows. Restrictive policies are combined together using AND, and +* permissive policies are combined together using OR. */ >From this commentary one may think that LET command supports row level security, but I don't see it being implemented. A wrong commentary?
Re: Schema variables - new implementation for Postgres 15
> On Fri, Mar 24, 2023 at 08:04:08AM +0100, Pavel Stehule wrote: > čt 23. 3. 2023 v 19:54 odesílatel Pavel Stehule > napsal: > > > čt 23. 3. 2023 v 16:33 odesílatel Peter Eisentraut < > > peter.eisentr...@enterprisedb.com> napsal: > > > >> The other issue is that by its nature this patch adds a lot of code in a > >> lot of places. Large patches are more likely to be successful if they > >> add a lot of code in one place or smaller amounts of code in a lot of > >> places. But this patch does both and it's just overwhelming. There is > >> so much new internal functionality and terminology. Variables can be > >> created, registered, initialized, stored, copied, prepared, set, freed, > >> removed, released, synced, dropped, and more. I don't know if anyone > >> has actually reviewed all that in detail. > >> > >> Has any effort been made to make this simpler, smaller, reduce scope, > >> refactoring, find commonalities with other features, try to manage the > >> complexity somehow? > >> > > I agree that this patch is large, but almost all code is simple. Complex > > code is "only" in 0002-session-variables.patch (113KB/438KB). > > > > Now, I have no idea how the functionality can be sensibly reduced or > > divided (no without significant performance loss). I see two difficult > > points in this code: > > > > 1. when to clean memory. The code implements cleaning very accurately - > > and this is unique in Postgres. Partially I implement some functionality of > > storage manager. Probably no code from Postgres can be reused, because > > there is not any support for global temporary objects. Cleaning based on > > sinval messages processing is difficult, but there is nothing else. The > > code is a little bit more complex, because there are three types of session > > variables: a) session variables, b) temp session variables, c) session > > variables with transaction scope. Maybe @c can be removed, and maybe we > > don't need to support not null default (this can simplify initialization). > > What do you think about it? > > > > 2. how to pass a variable's value to the executor. The implementation is > > based on extending the Param node, but it cannot reuse query params buffers > > and implements own. > > But it is hard to simplify code, because we want to support usage > > variables in queries, and usage in PL/pgSQL expressions too. And both are > > processed differently. > > > > Maybe I can divide the patch 0002-session-variables to three sections - > related to memory management, planning and execution? I agree, the patch scale is a bit overwhelming. It's worth noting that due to the nature of this change certain heavy lifting has to be done in any case, plus I've got an impression that some part of the patch are quite solid (although I haven't reviewed everything, did anyone achieve that milestone?). But still, it would be of great help to simplify the current implementation, and I'm afraid the only way of doing this is to make trades-off about functionality vs change size & complexity. Maybe instead splitting the patch into implementation components, it's possible to split it feature-by-feature, where every single patch would represent an independent (to a certain degree) functionality? I have in mind something like: catalog changes; base implementation; ACL support; xact actions implementation (on commit drop, etc); variables with default value; shadowing; etc. If such approach is possible, it will give us: flexibility to apply only a subset of the whole patch series; some understanding how much complexity is coming from each feature. What do you think about this idea? I also recall somewhere earlier in the thread Pavel has mentioned that a transactional version of session variables patch would be actually simpler, and he has plans to implement it later on. Is there another trade-off on the table we could think of, transactional vs non-transactional session variables?
Re: Disable vacuuming to provide data history
There is also another blocker - our timestamp resolution is 1 microsecond and we are dangerously close to speeds where one could update a row twice in the same microsecond . I have been thinking about this, and what is needed is 1. a nanosecond-resolution "abstime" type - not absolutely necessary, but would help with corner cases. 2. VACUUM should be able to "freeze" by replacing xmin/xmax values with commit timestamps, or adding tmin/tmax where necessary. 3. Optionally VACUUM could move historic rows to archive tables with explicit tmin/tmax columns (this also solves the pg_dump problem) Most of the above design - apart from the timestamp resolution and vacuum being the one doing stamping in commit timestamps - is not really new - up to version 6.2 PostgreSQL had tmin/tmax instead of xmin/xmax and you could specify the timestamp you want to query any table at. And the original Postgres design was Full History Database where you could say " SELECT name, population FROM cities['epoch' .. 'now'] " to get all historic population values. And historic data was meant to be moved to the WORM optical drives which had just arrived to the market --- Hannu On Sat, Feb 25, 2023 at 3:11 AM Vik Fearing wrote: > > On 2/24/23 22:06, Corey Huinker wrote: > > On Thu, Feb 23, 2023 at 6:04 AM wrote: > > > > [1] some implementations don't use null, they use an end-timestamp set to > > a date implausibly far in the future ( 3999-12-31 for example ), > > The specification is, "At any point in time, all rows that have their > system-time period end column set to the highest value supported by the > data type of that column are known as current system rows; all other > rows are known as historical system rows." > > I would like to see us use 'infinity' for this. > > The main design blocker for me is how to handle dump/restore. The > standard does not bother thinking about that. > -- > Vik Fearing > > >
awkward cancellation of parallel queries on standby.
When a parallel query gets cancelled on a standby due to max_standby_streaming_delay, it happens rather awkwardly. I get two errors stacked up, a query cancellation followed by a connection termination. I use `pgbench -R 1 -T3600 -P5` on the master to generate a light but steady stream of HOT pruning records, and then run `select sum(a.abalance*b.abalance) from pgbench_accounts a join pgbench_accounts b using (bid);` on the standby not in a transaction block to be a long-running parallel query (scale factor of 20) I also set max_standby_streaming_delay = 0. That isn't necessary, but it saves wear and tear on my patience. ERROR: canceling statement due to conflict with recovery DETAIL: User query might have needed to see row versions that must be removed. FATAL: terminating connection due to conflict with recovery DETAIL: User query might have needed to see row versions that must be removed. This happens quite reliably. In psql, these sometimes both show up immediately, and sometimes only the first one shows up immediately and then the second one appears upon the next communication to the backend. I don't know if this is actually a problem. It isn't for me as I don't do this kind of thing outside of testing, but it seems untidy and I can see it being frustrating from a catch-and-retry perspective and from a log-spam perspective. It looks like the backend gets signalled by the startup process, and then it signals the postmaster to signal the parallel workers, and then they ignore it for a quite long time (tens to hundreds of ms). By the time they get around responding, someone has decided to escalate things. Which doesn't seem to be useful, because no one can do anything until the workers respond anyway. This behavior seems to go back a long way, but the propensity for both messages to show up at the same time vs. in different round-trips changes from version to version. Is this something we should do something about? Cheers, Jeff
Re: CREATE INDEX CONCURRENTLY on partitioned index
On Sun, Dec 04, 2022 at 01:09:35PM -0600, Justin Pryzby wrote: > This currently handles partitions with a loop around the whole CIC > implementation, which means that things like WaitForLockers() happen > once for each index, the same as REINDEX CONCURRENTLY on a partitioned > table. Contrast that with ReindexRelationConcurrently(), which handles > all the indexes on a table in one pass by looping around indexes within > each phase. Rebased over the progress reporting fix (27f5c712b). I added a list of (intermediate) partitioned tables, rather than looping over the list of inheritors again, to save calling rel_get_relkind(). I think this patch is done. -- Justin >From 941f7f930fc18563e2da42143015b6573d5447b1 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 6 Jun 2020 17:42:23 -0500 Subject: [PATCH] Allow CREATE INDEX CONCURRENTLY on partitioned table https://www.postgresql.org/message-id/flat/20201031063117.gf3...@telsasoft.com --- doc/src/sgml/ddl.sgml | 4 +- doc/src/sgml/ref/create_index.sgml | 14 +- src/backend/commands/indexcmds.c | 200 ++--- src/test/regress/expected/indexing.out | 127 +++- src/test/regress/sql/indexing.sql | 26 +++- 5 files changed, 297 insertions(+), 74 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 91c036d1cbe..64efdf1e879 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -4178,9 +4178,7 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 so that they are applied automatically to the entire hierarchy. This is very convenient, as not only will the existing partitions become indexed, but - also any partitions that are created in the future will. One limitation is - that it's not possible to use the CONCURRENTLY - qualifier when creating such a partitioned index. To avoid long lock + also any partitions that are created in the future will. To avoid long lock times, it is possible to use CREATE INDEX ON ONLY the partitioned table; such an index is marked invalid, and the partitions do not get the index applied automatically. The indexes on partitions can diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml index 40986aa502f..b05102efdaf 100644 --- a/doc/src/sgml/ref/create_index.sgml +++ b/doc/src/sgml/ref/create_index.sgml @@ -645,7 +645,10 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] If a problem arises while scanning the table, such as a deadlock or a uniqueness violation in a unique index, the CREATE INDEX -command will fail but leave behind an invalid index. This index +command will fail but leave behind an invalid index. +If this happens while build an index concurrently on a partitioned +table, the command can also leave behind valid or +invalid indexes on table partitions. The invalid index will be ignored for querying purposes because it might be incomplete; however it will still consume update overhead. The psql \d command will report such an index as INVALID: @@ -692,15 +695,6 @@ Indexes: cannot. - -Concurrent builds for indexes on partitioned tables are currently not -supported. However, you may concurrently build the index on each -partition individually and then finally create the partitioned index -non-concurrently in order to reduce the time where writes to the -partitioned table will be locked out. In this case, building the -partitioned index is a metadata only operation. - - diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3ec8b5cca6c..daba8b67dbe 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -93,6 +93,11 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId, bool primary, bool isconstraint); static char *ChooseIndexNameAddition(List *colnames); static List *ChooseIndexColumnNames(List *indexElems); +static void DefineIndexConcurrentInternal(Oid relationId, + Oid indexRelationId, + IndexInfo *indexInfo, + LOCKTAG heaplocktag, + LockRelId heaprelid); static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel); static void RangeVarCallbackForReindexIndex(const RangeVar *relation, @@ -559,7 +564,6 @@ DefineIndex(Oid relationId, bool amissummarizing; amoptions_function amoptions; bool partitioned; - bool safe_index; Datum reloptions; int16 *coloptions; IndexInfo *indexInfo; @@ -567,12 +571,10 @@ DefineIndex(Oid relationId, bits16 constr_flags; int numberOfAttributes; int numberOfKeyAttributes; - TransactionId limitXmin; ObjectAddress address; LockRelId heaprelid; LOCKTAG heaplocktag; LOCKMODE lockmode; - Snapshot snapshot; Oid root_save_userid; int root_save_sec_context; int
Time to move pg_test_timing to measure in nanoseconds
Currently pg_test_timing utility measures its timing overhead in
microseconds, giving results like this
~$ /usr/lib/postgresql/15/bin/pg_test_timing
Testing timing overhead for 3 seconds.
Per loop time including overhead: 18.97 ns
Histogram of timing durations:
< us % of total count
1 98.11132 155154419
2 1.887562985010
4 0.00040630
8 0.00012184
16 0.00058919
32 0.3 40
64 0.0 6
I got curious and wanted to see how the 98.1% timings are distributed
(raw uncleaned patch attached)
And this is what I got when I increased the measuring resolution to nanoseconds
hannuk@hannuk1:~/work/postgres15_uni_dist_on/src/bin/pg_test_timing$
./pg_test_timing
Testing timing overhead for 3 seconds.
Per loop time including overhead: 17.34 ns, min: 15, same: 0
Histogram of timing durations:
< ns % of total count
1 0.0 0
2 0.0 0
4 0.0 0
8 0.0 0
16 1.143871979085
32 98.47924 170385392
64 0.21666 374859
128 0.15654 270843
256 0.00297 5139
512 0.00016272
1024 0.4 73
2048 0.00018306
4096 0.00022375
8192 0.6 99
16384 0.5 80
32768 0.1 20
65536 0.0 6
131072 0.0 2
As most of the samples seems to be in ranges 8..15 and 16..32
nanoseconds the current way of measuring at microsecond resolution is
clearly inadequate.
The attached patch is not meant to be applied as-is but is rather
there as a helper to easily verify the above numbers.
QUESTIONS
1. Do you think it is ok to just change pg_test_timing to return the
result in nanoseconds or should there be a flag that asks for
nanosecond resolution ?
2. Should the output be changed to give ranges instead of `*** pg_test_timing.c.orig 2023-01-16 01:09:51.839251695 +0100
--- pg_test_timing.c 2023-01-16 22:24:49.768037225 +0100
***
*** 11,16
--- 11,20
#include "getopt_long.h"
#include "portability/instr_time.h"
+ #define INSTR_TIME_GET_NANOSEC(t) \
+ (((uint64) (t).tv_sec * (uint64) 10) + (uint64) ((t).tv_nsec))
+
+
static const char *progname;
static unsigned int test_duration = 3;
***
*** 19,26
static uint64 test_timing(unsigned int duration);
static void output(uint64 loop_count);
! /* record duration in powers of 2 microseconds */
! long long int histogram[32];
int
main(int argc, char *argv[])
--- 23,30
static uint64 test_timing(unsigned int duration);
static void output(uint64 loop_count);
! /* record duration in powers of 2 nanoseconds */
! long long int histogram[64];
int
main(int argc, char *argv[])
***
*** 124,139
uint64 total_time;
int64 time_elapsed = 0;
uint64 loop_count = 0;
uint64 prev,
cur;
instr_time start_time,
end_time,
temp;
! total_time = duration > 0 ? duration * INT64CONST(100) : 0;
INSTR_TIME_SET_CURRENT(start_time);
! cur = INSTR_TIME_GET_MICROSEC(start_time);
while (time_elapsed < total_time)
{
--- 128,145
uint64 total_time;
int64 time_elapsed = 0;
uint64 loop_count = 0;
+ uint64 same_count = 0;
+ uint64 min_diff = UINT64_MAX;
uint64 prev,
cur;
instr_time start_time,
end_time,
temp;
! total_time = duration > 0 ? duration * INT64CONST(10) : 0;
INSTR_TIME_SET_CURRENT(start_time);
! cur = INSTR_TIME_GET_NANOSEC(start_time);
while (time_elapsed < total_time)
{
***
*** 141,157
bits = 0;
prev = cur;
INSTR_TIME_SET_CURRENT(temp);
! cur = INSTR_TIME_GET_MICROSEC(temp);
diff = cur - prev;
/* Did time go backwards? */
! if (diff < 0)
{
fprintf(stderr, _("Detected clock going backwards in time.\n"));
fprintf(stderr, _("Time warp: %d ms\n"), diff);
exit(1);
}
/* What is the highest bit in the time diff? */
while (diff)
--- 147,172
bits = 0;
prev = cur;
+ // INSTR_TIME_SET_CURRENT(temp);
+ // prev = INSTR_TIME_GET_NANOSEC(temp);
INSTR_TIME_SET_CURRENT(temp);
! cur = INSTR_TIME_GET_NANOSEC(temp);
diff = cur - prev;
/* Did time go backwards? */
! if (unlikely(diff <= 0))
{
+ if (diff == 0 )
+ same_count ++;
+ else
+ {
fprintf(stderr, _("Detected clock going backwards in time.\n"));
fprintf(stderr, _("Time warp: %d ms\n"), diff);
exit(1);
+ }
}
+ if (min_diff > diff)
+ min_diff = diff;
/* What is the highest bit in the time diff? */
while (diff)
***
*** 165,179
loop_count++;
INSTR_TIME_SUBTRACT(temp, start_time);
! time_elapsed = INSTR_TIME_GET_MICROSEC(temp);
}
Re: Progress report of CREATE INDEX for nested partitioned tables
On Sat, Mar 25, 2023 at 03:43:32PM -0400, Tom Lane wrote: > I pushed 0001 with some cosmetic changes (for instance, trying to > make the style of the doc entries for partitions_total/partitions_done > match the rest of their table). Thanks. > I'm not touching 0002 or 0003, because I think they're fundamentally > a bad idea. Progress reporting is inherently inexact, because it's Nobody could disagree that it's inexact. The assertions are for minimal sanity tests and consistency. Like if "total" is set multiple times (as in this patch), or if a progress value goes backwards. Anyway the assertions exposed two other issues that would need to be fixed before the assertions themselves could be proposed. -- Justin
Re: Support logical replication of DDLs
On Thu, 23 Mar 2023 at 09:22, Ajin Cherian wrote: > > On Mon, Mar 20, 2023 at 8:17 PM houzj.f...@fujitsu.com > wrote: > > > > Attach the new patch set which addressed above comments. > > 0002,0003,0004 patch has been updated in this version. > > > > Best Regards, > > Hou zj > > Attached a patch-set which adds support for ONLY token in ALTER TABLE.. > Changes are in patches 0003 and 0004. Few comments: 1) This file should not be included: typedef struct diff --git a/src/test/modules/test_ddl_deparse_regress/regression.diffs b/src/test/modules/test_ddl_deparse_regress/regression.diffs deleted file mode 100644 index 3be15de..000 --- a/src/test/modules/test_ddl_deparse_regress/regression.diffs +++ /dev/null @@ -1,847 +0,0 @@ -diff -U3 /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/expected/create_table.out /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/results/create_table.out /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/expected/create_table.out 2023-03-22 23:08:34.915184709 -0400 -+++ /home/ajin/postgresql/postgres/postgres2/postgres/src/test/modules/test_ddl_deparse_regress/results/create_table.out 2023-03-22 23:09:46.810424685 -0400 2) The patch does not apply neatly: git am v82-0004-Introduce-the-test_ddl_deparse_regress-test-modu.patch Applying: Introduce the test_ddl_deparse_regress test module. .git/rebase-apply/patch:2489: trailing whitespace. NOTICE: re-formed command: CREATE TABLE public.ctlt1_like (a pg_catalog.text STORAGE main COLLATE pg_catalog."default" , b pg_catalog.text STORAGE external COLLATE pg_catalog."default" , CONSTRAINT ctlt1_a_check CHECK ((pg_catalog.length(a) OPERATOR(pg_catalog.>) 2)), CONSTRAINT ctlt1_like_pkey PRIMARY KEY (a)) .git/rebase-apply/patch:2502: trailing whitespace. NOTICE: re-formed command: ALTER TABLE public.test_alter_type ALTER COLUMN quantity SET DATA TYPE pg_catalog.float4 .git/rebase-apply/patch:2511: trailing whitespace. -NOTICE: re-formed command: CREATE TABLE public.test_alter_set_default (id pg_catalog.int4 STORAGE plain , name pg_catalog."varchar" STORAGE extended COLLATE pg_catalog."default" , description pg_catalog.text STORAGE extended COLLATE pg_catalog."default" , price pg_catalog.float4 STORAGE plain , quantity pg_catalog.int4 STORAGE plain , purchase_date pg_catalog.date STORAGE plain ) .git/rebase-apply/patch:2525: trailing whitespace. -NOTICE: re-formed command: CREATE TABLE public.test_drop_default (id pg_catalog.int4 STORAGE plain , name pg_catalog."varchar" STORAGE extended COLLATE pg_catalog."default" , description pg_catalog.text STORAGE extended COLLATE pg_catalog."default" , price pg_catalog.float4 STORAGE plain , quantity pg_catalog.int4 STORAGE plain , purchase_date pg_catalog.date STORAGE plain , default_price pg_catalog.float4 STORAGE plainDEFAULT 10.0 , default_name pg_catalog."varchar" STORAGE extended COLLATE pg_catalog."default" DEFAULT 'foo'::character varying ) .git/rebase-apply/patch:2538: trailing whitespace. -NOTICE: re-formed command: CREATE TABLE public.test_set_not_null (id pg_catalog.int4 STORAGE plain , name pg_catalog."varchar" STORAGE extended COLLATE pg_catalog."default" , description pg_catalog.text STORAGE extended COLLATE pg_catalog."default" , price pg_catalog.float4 STORAGE plain , quantity pg_catalog.int4 STORAGE plain , purchase_date pg_catalog.date STORAGE plain , size pg_catalog.int4 STORAGE plain NOT NULL ) warning: squelched 74 whitespace errors warning: 79 lines add whitespace errors. 3) This file should not be included: diff --git a/src/test/modules/test_ddl_deparse_regress/regression.out b/src/test/modules/test_ddl_deparse_regress/regression.out deleted file mode 100644 index a44b91f..000 --- a/src/test/modules/test_ddl_deparse_regress/regression.out +++ /dev/null @@ -1,7 +0,0 @@ -test test_ddl_deparse ... ok 31 ms -test create_extension ... ok 52 ms 4) The test file should be included in meson.build also: 't/027_nosuperuser.pl', 't/028_row_filter.pl', 't/029_on_error.pl', 't/030_origin.pl', 't/031_column_list.pl', 't/032_subscribe_use_index.pl', 't/100_bugs.pl', ], Regards, Vignesh
Re: meson/msys2 fails with plperl/Strawberry
On 2023-03-25 Sa 12:38, Andres Freund wrote:
Hi,
On 2023-03-25 08:46:42 -0400, Andrew Dunstan wrote:
config/perl.m4 contains this:
AC_MSG_CHECKING(for flags to link embedded Perl)
if test "$PORTNAME" = "win32" ; then
perl_lib=`basename $perl_archlibexp/CORE/perl[[5-9]]*.lib .lib`
if test -e "$perl_archlibexp/CORE/$perl_lib.lib"; then
perl_embed_ldflags="-L$perl_archlibexp/CORE -l$perl_lib"
else
perl_lib=`basename $perl_archlibexp/CORE/libperl[[5-9]]*.a .a |
sed 's/^lib//'`
if test -e "$perl_archlibexp/CORE/lib$perl_lib.a"; then
perl_embed_ldflags="-L$perl_archlibexp/CORE -l$perl_lib"
fi
fi
else
pgac_tmp1=`$PERL -MExtUtils::Embed -e ldopts`
pgac_tmp2=`$PERL -MConfig -e 'print "$Config{ccdlflags}
$Config{ldflags}"'`
perl_embed_ldflags=`echo X"$pgac_tmp1" | sed -e "s/^X//" -e
"s%$pgac_tmp2%%"`
fi
AC_SUBST(perl_embed_ldflags)dnl
I don't see any equivalent in meson.build of the win32 logic, and thus I am
getting a setup failure on fairywren when trying to move it to meson, while
it will happily build with autoconf.
I did not try to build with strawberry perl using mingw - it doesn't seem like
a very interesting thing, given that mingw has a much more reasonable perl
than strawberry - but with the mingw perl it works well.
Strawberry is a recommended perl installation for Windows
() and is widely used AFAICT.
In general my approach has been to build as independently as possible
from msys2 infrastructure, in particular a) not to rely on it at all for
MSVC builds and b) to use independent third party installations for
things like openssl and PLs.
In any case, I don't think we should be choosing gratuitously to break
things that hitherto worked, however uninteresting you personally might
find them.
The above logic actually did *not* work well with mingw for me, because the
names are not actually what configure expects, and it seems like a seriously
bad idea to encode that much knowledge about library naming and locations.
Didn't work well how? It just worked perfectly for me with ucrt perl
(setup, built and tested) using configure:
$ grep perl532 config.log
configure:10482: result:
-LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE -lperl532
configure:18820: gcc -o conftest.exe -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 -O2 -I./src/include/port/win32
-IC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE
-Wl,--allow-multiple-definition -Wl,--disable-auto-import conftest.c
-LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE -lperl532 >&5
perl_embed_ldflags='-LC:/tools/nmsys64/ucrt64/lib/perl5/core_perl/CORE
-lperl532'
I would expect the ld flags to be "-LC:/STRAWB~1/perl/lib/CORE -lperl532"
You didn't say what they ended up as?
I think you misunderstand me. This is what they should end up as.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Greg Stark (st...@mit.edu) wrote:
> The CFBot says there's a function be_gssapi_get_proxy() which is
> undefined. Presumably this is a missing #ifdef or a definition that
> should be outside an #ifdef.
Yup, just a couple of missing #ifdef's.
Updated and rebased patch attached.
Thanks!
Stephen
From 450a8749d04af54e8214a2ab357fbec7849a485b Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Mon, 28 Feb 2022 20:17:55 -0500
Subject: [PATCH] Add support for Kerberos credential delegation
Support GSSAPI/Kerberos credentials being delegated to the server by a
client. With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.
Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
proxied to the server by the client and GSSAPI is used to authenticate
to the remote system.
Authors: Stephen Frost, Peifeng Qiu
Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com
---
contrib/dblink/dblink.c | 120 ---
contrib/dblink/expected/dblink.out| 4 +-
contrib/postgres_fdw/connection.c | 70 +++-
.../postgres_fdw/expected/postgres_fdw.out| 19 +-
contrib/postgres_fdw/option.c | 3 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +-
doc/src/sgml/config.sgml | 17 +
doc/src/sgml/dblink.sgml | 5 +-
doc/src/sgml/libpq.sgml | 41 +++
doc/src/sgml/monitoring.sgml | 9 +
doc/src/sgml/postgres-fdw.sgml| 5 +-
src/backend/catalog/system_views.sql | 3 +-
src/backend/foreign/foreign.c | 1 +
src/backend/libpq/auth.c | 13 +-
src/backend/libpq/be-gssapi-common.c | 51 +++
src/backend/libpq/be-secure-gssapi.c | 26 +-
src/backend/utils/activity/backend_status.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 20 +-
src/backend/utils/init/postinit.c | 8 +-
src/backend/utils/misc/guc_tables.c | 10 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/catalog/pg_proc.dat | 6 +-
src/include/libpq/auth.h | 1 +
src/include/libpq/be-gssapi-common.h | 3 +
src/include/libpq/libpq-be.h | 2 +
src/include/utils/backend_status.h| 1 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-auth.c| 15 +-
src/interfaces/libpq/fe-connect.c | 17 +
src/interfaces/libpq/fe-secure-gssapi.c | 23 +-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 2 +
src/test/kerberos/Makefile| 3 +
src/test/kerberos/t/001_auth.pl | 335 --
src/test/perl/PostgreSQL/Test/Utils.pm| 27 ++
src/test/regress/expected/rules.out | 11 +-
36 files changed, 743 insertions(+), 135 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e..292377566b 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -48,6 +48,7 @@
#include "funcapi.h"
#include "lib/stringinfo.h"
#include "libpq-fe.h"
+#include "libpq/libpq-be.h"
#include "libpq/libpq-be-fe-helpers.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -111,7 +112,8 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
static char *generate_relation_name(Relation rel);
static void dblink_connstr_check(const char *connstr);
-static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+static bool dblink_connstr_has_pw(const char *connstr);
+static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
static char *get_connect_string(const char *servername);
@@ -213,7 +215,7 @@ dblink_get_conn(char *conname_or_str,
errmsg("could not establish connection"),
errdetail_internal("%s", msg)));
}
- dblink_security_check(conn, rconn);
+ dblink_security_check(conn, rconn, connstr);
if (PQclientEncoding(conn) != GetDatabaseEncoding())
PQsetClientEncoding(conn, GetDatabaseEncodingName());
freeconn = true;
@@ -307,7 +309,7 @@ dblink_connect(PG_FUNCTION_ARGS)
}
/* check password
Re: Schema variables - new implementation for Postgres 15
Hi, I just have a few minor wording improvements for the various comments / documentation you quoted. On Sun, Mar 26, 2023 at 08:53:49AM +0200, Pavel Stehule wrote: > út 21. 3. 2023 v 17:18 odesílatel Peter Eisentraut < > peter.eisentr...@enterprisedb.com> napsal: > > > - What is the purpose of struct Variable? It seems very similar to > >FormData_pg_variable. At least a comment would be useful. > > > > I wrote comment there: > > > /* > * The Variable struct is based on FormData_pg_variable struct. Against > * FormData_pg_variable it can hold node of deserialized expression used > * for calculation of default value. > */ Did you mean "Unlike" rather than "Against"? > > 0002 > > > > expr_kind_allows_session_variables() should have some explanation > > about criteria for determining which expression kinds should allow > > variables. > > > > I wrote comment there: > > /* > * Returns true, when expression of kind allows using of > * session variables. > + * The session's variables can be used everywhere where > + * can be used external parameters. Session variables > + * are not allowed in DDL. Session's variables cannot be > + * used in constraints. > + * > + * The identifier can be parsed as an session variable > + * only in expression's kinds where session's variables > + * are allowed. This is the primary usage of this function. > + * > + * Second usage of this function is for decision if > + * an error message "column does not exist" or "column > + * or variable does not exist" should be printed. When > + * we are in expression, where session variables cannot > + * be used, we raise the first form or error message. > */ Maybe /* * Returns true if the given expression kind is valid for session variables * Session variables can be used everywhere where external parameters can be * used. Session variables are not allowed in DDL commands or in constraints. * * An identifier can be parsed as a session variable only for expression kinds * where session variables are allowed. This is the primary usage of this * function. * * Second usage of this function is to decide whether "column does not exist" or * "column or variable does not exist" error message should be printed. * When we are in an expression where session variables cannot be used, we raise * the first form or error message. */ > > session_variables_ambiguity_warning: There needs to be more > > information about this. The current explanation is basically just, > > "warn if your query is confusing". Why do I want that? Why would I > > not want that? What is the alternative? What are some examples? > > Shouldn't there be a standard behavior without a need to configure > > anything? > > > > I enhanced this entry: > > + > +The session variables can be shadowed by column references in a > query. This > +is an expected feature. The existing queries should not be broken > by creating > +any session variable, because session variables are shadowed > always if the > +identifier is ambiguous. The variables should be named without > possibility > +to collision with identifiers of other database objects (column > names or > +record field names). The warnings enabled by setting > session_variables_ambiguity_warning > +should help with finding identifier's collisions. Maybe Session variables can be shadowed by column references in a query, this is an expected behavior. Previously working queries shouldn't error out by creating any session variable, so session variables are always shadowed if an identifier is ambiguous. Variables should be referenced using an unambiguous identifier without any possibility for a collision with identifier of other database objects (column names or record fields names). The warning messages emitted when enabling session_variables_ambiguity_warning can help finding such identifier collision. > + > + > +This feature can significantly increase size of logs, and then it > is > +disabled by default, but for testing or development environments it > +should be enabled. Maybe This feature can significantly increase log size, so it's disabled by default. For testing or development environments it's recommended to enable it if you use session variables.
Re: doc: add missing "id" attributes to extension packaging page
On 24.03.2023 at 10:45, Alvaro Herrera wrote: But why are there no anchors next to items on that page? For example, how do I get the link for the "Meta Commands" subsection? I somehow knew that responding from a crappy mobile phone e-mail client will mess up the format and the thread... For those trying to follow the thread in the archives: my response (it's probably a refsect which isn't supported yet) ended up here: https://www.postgresql.org/message-id/1N1fn0-1qd4xd1MyG-011ype%40mail.gmx.net Regards, Brar
