Re: Remove dependence on integer wrapping

On Thu, Aug 15, 2024 at 2:16 AM Nathan Bossart wrote: > +static inline bool +pg_neg_u64_overflow(uint64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#elif defined(HAVE_INT128) + uint128 res = -((int128) a); + + if (unlikely(res < PG_

Re: Proposal to have INCLUDE/EXCLUDE options for altering option values

Hi PostgreSQL Community, I would like to provide an update on the patch I previously submitted, along with a clearer explanation of the issue it addresses and the improvements it introduces. Current Issue: PostgreSQL currently supports several options with actions like ADD, SET, and DROP for foreig

Re: Enable data checksums by default

On 08.08.24 19:42, Robert Haas wrote: I'm thinking pg_upgrade could have a mode where it adds the checksum during the upgrade as it copies the files (essentially a subset of pg_checksums). I think that would be useful for that middle tier of users who just want a good default experience. That w

Re: Logical Replication of sequences

Hi Vignesh, I have reviewed your latest patchset: v20240814-0001. No comments v20240814-0002. No comments v20240814-0003. No comments v20240814-0004. See below v20240814-0005. No comments // v20240814-0004. == src/backend/commands/subscriptioncmds.c CreateSubscription: nit - XXX commen

Re: Variable renaming in dbcommands.c

On 09.08.24 09:43, Daniel Gustafsson wrote: On 9 Aug 2024, at 09:21, Peter Eisentraut wrote: In dbcommands.c function createdb(), there are several sets of very similar local variable names, such as "downer" and "dbowner", which is very confusing and error-prone. The first set are the DefE

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024, at 19:25, Joel Jacobson wrote: > What do we want to happen if passing a numeric with decimal digits, > to decimal_to_bytes()? It must be an error, right? > > Example: SELECT decimal_to_bytes(1.23); Hmm, an error feels quite ugly on second thought. Would be nicer if all numeri

Re: Adding clarification to description of IPC wait events XactGroupUpdate and ProcArrayGroupUpdate

On Thu, Aug 15, 2024 at 4:00 AM Nathan Bossart wrote: > On Wed, Aug 14, 2024 at 10:38:49PM +0800, SAMEER KUMAR wrote: > > Yes, while reviewing the history of changes, I too noticed the same. The > > documentation of older versions (v12 [1]) still has old descriptions. > > After reading the relate

Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest

On Wed, Aug 14, 2024, at 8:19 PM, Tom Lane wrote: > Jelte Fennema-Nio writes: > > I'd like to send an automatic mail to a thread whenever it gets added > > to a commitfest. Since this would impact everyone that's subscribed to > > the mailinglist I'd like some feedback on this. This mail would > >

Re: Typo in unicode_assigned() document PG17

On Thu, 2024-08-15 at 09:34 +0900, Hironobu SUZUKI wrote: > Hi, > > The PG17 documentation for unicode_assigned() in > doc/src/sgml/func.sgml > incorrectly states that the function returns a 'text' value. It > actually > returns a boolean value. Thank you for the report. Fixed and backported to

Re: Improve error message for ICU libraries if pkg-config is absent

On Fri, 2024-08-09 at 11:44 +0200, Michael Banck wrote: > So maybe the better change is to just remove the explicit error > message > again and depend on PKG_CHECK_MODULES erroring out helpfully?  The > downside would be that the hint about specifying --without-icu to get > around this would disapp

Re: Skip adding row-marks for non target tables when result relation is foreign table.

On Fri, 2024-08-09 at 17:35 +0900, Etsuro Fujita wrote: > I might be missing something, but I think the extra round trip > happens > for each foreign row locked using the RefetchForeignRow() API in > ExecLockRows(), so I think the overhead is caused in the normal case. Is there any sample code tha

Re: ECPG cleanup and fix for clang compile-time problem

I wrote: > Here's a rebased but otherwise identical patchset. I also added > an 0007 that removes check_rules.pl as threatened. I've done some more work on this and hope to post an updated patchset tomorrow. Before that though, is there any objection to going ahead with pushing the 0001 patch (p

Typo in unicode_assigned() document PG17

Hi, The PG17 documentation for unicode_assigned() in doc/src/sgml/func.sgml incorrectly states that the function returns a 'text' value. It actually returns a boolean value. unicode_assigned(): https://github.com/postgres/postgres/blob/ef6e028f05b3e4ab23c5edfdfff457e0d2a649f6/src/backend/uti

Re: [18] Fix a few issues with the collation cache

On Thu, 2024-08-08 at 12:24 -0700, Jeff Davis wrote: > The collation cache, which maps collation oids to pg_locale_t > objects, > has a few longstanding issues: Here's a patch set v2. I changed it so that the pg_locale_t itself a resource kind, rather than having separate locale_t and UCollator r

Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest

Jelte Fennema-Nio writes: > I'd like to send an automatic mail to a thread whenever it gets added > to a commitfest. Since this would impact everyone that's subscribed to > the mailinglist I'd like some feedback on this. This mail would > include: > 1. A very short blurb like: "This thread was ad

Re: Remaining dependency on setlocale()

On Thu, 2024-08-15 at 10:43 +1200, Thomas Munro wrote: > So I think the solution could perhaps be something like: in some > early > startup phase before there are any threads, we nail down all the > locale categories to "C" (or whatever we decide on for the permanent > global locale), and also quer

Re: tiny step toward threading: reduce dependence on setlocale()

On Wed, 2024-08-14 at 01:31 +0200, Andreas Karlsson wrote: > 0001-Remove-lc_collate_is_c.patch > > Removes lc_collate_is_c(). This could use some performance testing, as the commit message says, otherwise it looks good. > 0002-Remove-lc_ctype_is_c.patch > > Removes lc_ctype_is_c() and POSIX_COL

Re: Remaining dependency on setlocale()

On Wed, Aug 7, 2024 at 7:07 PM Thomas Munro wrote: > On Wed, Aug 7, 2024 at 10:23 AM Tom Lane wrote: > > Jeff Davis writes: > > > 2. I don't see a good way to canonicalize a locale name, like in > > > check_locale(), which uses the result of setlocale(). > > > > What I can tell you about that is

Re: Proposal for implementing OCSP Stapling in PostgreSQL

On Wed, Aug 7, 2024 at 12:20 AM Daniel Gustafsson wrote: > > While I have only skimmed the patch so far and need more review before I can > comment on it, I do have a question on the expected use of OCSP support in > postgres. With OCSP becoming optional [0], and big providers like Let's > Encryp

Opinion poll: Sending an automated email to a thread when it gets added to the commitfest

I'd like to send an automatic mail to a thread whenever it gets added to a commitfest. Since this would impact everyone that's subscribed to the mailinglist I'd like some feedback on this. This mail would include: 1. A very short blurb like: "This thread was added to the commitfest with ID 1234" 2

Re: collect_corrupt_items_vacuum.patch

On Wed, Aug 14, 2024 at 10:20 AM Heikki Linnakangas wrote: > On 14/08/2024 04:51, Alexander Korotkov wrote: > > On Tue, Aug 13, 2024 at 10:15 PM Alexander Korotkov > > wrote: > >> On Tue, Aug 13, 2024 at 9:39 PM Heikki Linnakangas wrote: > >>> > >>> This causes an assertion failure when executed

Re: query_id, pg_stat_activity, extended query protocol

I think the testing discussion should be moved to a different thread. What do you think? See v4. 0001 deals with reporting queryId in exec_execute_message and exec_bind_message. 0002 deals with reporting queryId after a cache invalidation. There are no tests as this requires more discussion i

Re: AIX support

On 14/08/2024 18:22, Srirama Kucherlapati wrote: Hi Heikki, I have attached the merged patch with all the changes, the earlier patch was just only the changes specific to older review comments.     > I'm sorry, I don't understand what you're saying here. Do you mean that     > we don't need

Re: Adding clarification to description of IPC wait events XactGroupUpdate and ProcArrayGroupUpdate

On Wed, Aug 14, 2024 at 10:38:49PM +0800, SAMEER KUMAR wrote: > Yes, while reviewing the history of changes, I too noticed the same. The > documentation of older versions (v12 [1]) still has old descriptions. After reading the related threads and code, I'm inclined to agree that this is a mistake,

Re: Remove dependence on integer wrapping

On Wed, Aug 14, 2024 at 10:29:39PM +0300, Heikki Linnakangas wrote: > Hmm, that still doesn't say what operation it's referring to. They existing > comments say "a + b", "a - b" or "a * b", but this one isn't referring to > anything at all. IMHO the existing comments are not too clear on that eithe

Re: Remove dependence on integer wrapping

On 14/08/2024 20:20, Nathan Bossart wrote: On Wed, Aug 14, 2024 at 10:02:28AM +0300, Heikki Linnakangas wrote: On 14/08/2024 06:07, Nathan Bossart wrote: * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defin

Re: generic plans and "initial" pruning

On Mon, Aug 12, 2024 at 8:54 AM Amit Langote wrote: > 1. I went through many iterations of the changes to ExecInitNode() to > return a partially initialized PlanState tree when it detects that the > CachedPlan was invalidated after locking a child table and to > ExecEndNode() to account for the Pl

Re: Remaining dependency on setlocale()

On Wed, 2024-08-14 at 14:31 +1200, Thomas Munro wrote: > 1.  The process global locale is always "C".  If you ever call > uselocale(), it can only be for short stretches, and you have to > restore it straight after; perhaps it is only ever used in > replacement > _l() functions for systems that lac

Re: libpq: Fix lots of discrepancies in PQtrace

On Wed, 14 Aug 2024 at 19:37, Alvaro Herrera wrote: > - to 0005 I change your pqTraceOutputEncryptionRequestResponse() > function name to pqTraceOutputCharResponse and instead of attaching > the "Response" literal in the outpuer to the name given in the > function call, just pass the whole s

Re: Remove dependence on integer wrapping

On Wed, Aug 14, 2024 at 01:41:40PM -0400, Joseph Koshakow wrote: > Since we know that a does not equal PG_INT64_MIN, could we shorten the > last three lines and do the following? > > static inline uint64 > pg_abs_s64(int64 a) > { > if (unlikely(a == PG_INT64_MIN)) > r

Re: [PATCH] Add log_transaction setting

  10.08.2024, 16:40, "Kirill Reshke" :On Thu, 4 Jul 2024 at 21:46, Sergey Solovev wrote: Hi. We encountered a problem with excessive logging when transaction is sampled. When it is getting sampled every statement is logged, event SELECT. This can lead to performance d

Re: Remove dependence on integer wrapping

Thanks for the improvements Nathan. The current iteration LGTM, just a single comment on `pg_abs_s64` > +static inline uint64 > +pg_abs_s64(int64 a) > +{ > + if (unlikely(a == PG_INT64_MIN)) > + return (uint64) PG_INT64_MAX + 1; > + if (a < 0) > + return -a; > + return

Re: libpq: Fix lots of discrepancies in PQtrace

I gave another look to the remaining patches; here they are again. I propose some changes: - to 0005 I change your pqTraceOutputEncryptionRequestResponse() function name to pqTraceOutputCharResponse and instead of attaching the "Response" literal in the outpuer to the name given in the func

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024, at 18:31, David Fetter wrote: > On Wed, Aug 14, 2024 at 05:39:32PM +0200, Joel Jacobson wrote: >> On Wed, Aug 14, 2024, at 16:43, David Fetter wrote: >> >> I also think it would be nice to provide these convenience functions: >> >> to_bytes(bigint) -> bytea >> >> from_bytes(by

Re: Remove dependence on integer wrapping

On Wed, Aug 14, 2024 at 10:02:28AM +0300, Heikki Linnakangas wrote: > On 14/08/2024 06:07, Nathan Bossart wrote: >> And here's a new version of the patch in which I've attempted to fix the >> silly mistakes. > > LGTM, just a few small comments: Thanks for reviewing. >> * - If a * b overflows,

Re: pg_verifybackup: TAR format backup verification

On Wed, Aug 14, 2024 at 9:20 AM Amul Sul wrote: > I agree with keeping verify_backup_file() separate, but I'm hesitant > about doing the same for verify_backup_directory(). I don't have time today to go through your whole email or re-review the code, but I plan to circle back to that at a later t

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024 at 05:39:32PM +0200, Joel Jacobson wrote: > On Wed, Aug 14, 2024, at 16:43, David Fetter wrote: > >> I also think it would be nice to provide these convenience functions: > >> to_bytes(bigint) -> bytea > >> from_bytes(bytea) -> bigint > > > > Along with these, would it make sen

Re: Report search_path value back to the client.

On Wed, 14 Aug 2024 at 18:22, Tomas Vondra wrote: > Here's the patch with a somewhat expanded / improved commit message. > Jelte, can you take a look there's no silly mistake? Looks good to me.

Re: Report search_path value back to the client.

Here's the patch with a somewhat expanded / improved commit message. Jelte, can you take a look there's no silly mistake? As mentioned previously, I plan to push this, so that if the protocol improvements from [1] don't land in PG18 we have at least something. I did take a brief look at the other

Re: format_datum debugging function

On 8/14/24 02:16, Peter Eisentraut wrote: On 12.08.24 23:15, Paul Jungwirth wrote: On 8/12/24 04:32, Aleksander Alekseev wrote: [...] This function takes a Datum and the appropriate out function, and returns a char *. So you can do this: (gdb) call format_datum(range_out, $1) $2 = 0x59162692d

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024, at 16:43, David Fetter wrote: >> I also think it would be nice to provide these convenience functions: >> to_bytes(bigint) -> bytea >> from_bytes(bytea) -> bigint > > Along with these, would it make sense to have other forms of these > that won't choke at 63 bits, e.g. NUMERIC

Re: POC: make mxidoff 64 bits

Hi! Sorry for delay. I was a bit busy last month. Anyway, here is my proposal for making multioffsets 64 bit. The patch set consists of three parts: 0001 - making user output of offsets 64-bit ready; 0002 - making offsets 64-bit; 0003 - provide 32 to 64 bit conversion in pg_upgarde. I'm pretty su

RE: AIX support

Hi Heikki, I have attached the merged patch with all the changes, the earlier patch was just only the changes specific to older review comments. > I'm sorry, I don't understand what you're saying here. Do you mean that > we don't need to do anything here, and the code we have in s_lock.h

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024 at 02:34:06PM +0200, Joel Jacobson wrote: > On Wed, Aug 14, 2024, at 13:31, Aleksander Alekseev wrote: > >> I wonder if get_bytes() and set_bytes() will behave differently > >> on little-endian vs big-endian systems? > > No, the returned value will not depend on the CPU endines

Re: macOS prefetching support

On 14.08.24 14:36, Thomas Munro wrote: On Wed, Aug 14, 2024 at 7:04 PM Peter Eisentraut wrote: Attached is a patch to implement this. It seems to work, but of course it's kind of hard to tell whether it actually does anything useful. Header order problem: pg_config_os.h defines __darwin__, b

Re: Adding clarification to description of IPC wait events XactGroupUpdate and ProcArrayGroupUpdate

Thanks for responding. On Wed, Aug 14, 2024 at 10:29 PM Nathan Bossart wrote: > On Thu, Jul 25, 2024 at 11:13:39AM +0800, SAMEER KUMAR wrote: > > While preparing for my presentation on PostgreSQL Wait Events at > > PgConf India, I was trying to understand *IPC:XactGroupUpdate* in more > > detail

Re: Adding clarification to description of IPC wait events XactGroupUpdate and ProcArrayGroupUpdate

On Thu, Jul 25, 2024 at 11:13:39AM +0800, SAMEER KUMAR wrote: > While preparing for my presentation on PostgreSQL Wait Events at > PgConf India, I was trying to understand *IPC:XactGroupUpdate* in more > detail. PostgreSQL documentation [1] mentions: > >> A process is waiting for the group leader

Re: Conflict detection and logging in logical replication

Hello, Hou! > This is as expected, and we have documented this in the code comments. We don't > need to report a conflict if the conflicting tuple has been removed or updated > due to concurrent transaction. The same is true if the transaction that > inserted the conflicting tuple is rolled back b

[BUG FIX]: invalid meson version detection

Hi. I looked at meson.build file at found an incorrectly used function to determine postgres version. > if pg_version.endswith('devel') >   pg_version_arr = [pg_version.split('devel')[0], '0'] There should be `pg_version.contains('devel')`, not `endswith`. Like this: -if pg_version.endswit

Re: Taking into account syncrep position in flush_lsn reported by apply worker

On 8/13/24 06:35, Amit Kapila wrote: On Mon, Aug 12, 2024 at 3:43 PM Arseny Sher wrote: Sorry for the poor formatting of the message above, this should be better: Hey. Currently synchronous_commit is disabled for logical apply worker on the ground that reported flush_lsn includes only loca

Re: pg_verifybackup: TAR format backup verification

On Tue, Aug 13, 2024 at 10:49 PM Robert Haas wrote: > > On Mon, Aug 12, 2024 at 5:13 AM Amul Sul wrote: > > I tried this in the attached version and made a few additional changes > > based on Sravan's off-list comments regarding function names and > > descriptions. > > > > Now, verification happe

Re: PG_TEST_EXTRA and meson

On Tue, Aug 13, 2024 at 9:07 PM Ashutosh Bapat wrote: > > I'm not entirely sure what you mean? src/test should work fine, > > anything lower than that (say src/test/ssl) does not. > > I could run them from src/test/modules/xid_wraparound/. That's desirable. On my machine, storing xid_wraparound i

Re: macOS prefetching support

On Wed, Aug 14, 2024 at 7:04 PM Peter Eisentraut wrote: > Attached is a patch to implement this. It seems to work, but of course > it's kind of hard to tell whether it actually does anything useful. Header order problem: pg_config_os.h defines __darwin__, but pg_config_manual.h is included first

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024, at 13:31, Aleksander Alekseev wrote: >> I wonder if get_bytes() and set_bytes() will behave differently >> on little-endian vs big-endian systems? > No, the returned value will not depend on the CPU endiness. Current > implementation uses big-endian / network order which in my

Re: PG docs - Sequence CYCLE clause

On 13.08.24 07:46, Peter Smith wrote: While reviewing another thread I had cause to be looking more carefully at the SEQUENCE documentation. I found it curious that, unlike other clauses, the syntax of the CYCLE clause was not displayed on a line by itself. I have changed that, and at the same

Re: Thread-safe nl_langinfo() and localeconv()

On Tue, Aug 13, 2024 at 6:23 PM Heikki Linnakangas wrote: > Patch 3 makes sense too, some comments on the details: > The #ifdefs and the LCONV_MEMBER stuff makes it a bit hard to follow > what happens in each implementation strategy. I wonder if it would be > more clear to duplicate more code. I

define PG_REPLSLOT_DIR

Hi hackers, while working on a replication slot tool (idea is to put it in contrib, not shared yet), I realized that "pg_replslot" is being used > 25 times in .c files. I think it would make sense to replace those occurrences with a $SUBJECT, attached a patch doing so. There is 2 places where i

Re: [PATCH] Add get_bytes() and set_bytes() functions

Hi, > +1 > > I wanted this myself many times. > > I wonder if get_bytes() and set_bytes() will behave differently > on little-endian vs big-endian systems? > > If so, then I think it would be nice to enforce a consistent byte order > (e.g., big-endian), to ensure consistent behavior across platfor

Re: [PATCH] Add get_bytes() and set_bytes() functions

On Wed, Aug 14, 2024, at 13:01, Aleksander Alekseev wrote: > The proposed patch adds get_bytes() and set_bytes() functions. The > semantics is similar to get_byte() and set_byte() we already have but > the functions operate with bigints rather than bytes and the user can > specify the size of the i

Re: Conflict detection and logging in logical replication

On Wed, Aug 14, 2024 at 8:05 AM Zhijie Hou (Fujitsu) wrote: > > Here is the V14 patch. > Review comments: 1. ReportApplyConflict() { ... + ereport(elevel, + errcode(ERRCODE_INTEGRITY_CONSTRAINT_VIOLATION), + errmsg("conflict detected on relation \"%s.%s\": conflict=%s", +get_namespace_name(Re

[PATCH] Add get_bytes() and set_bytes() functions

Hi, While discussing another patch [1] it was discovered that we don't have a convenient way of casting a bytea to an integer / bigint and vice versa, extracting integers larger than one byte from byteas, etc. For instance, casting '\x11223344' to 0x11223344 may look like this: ``` WITH vals AS

Re: Apply PGDLLIMPORT markings to some GUC variables

On 8/14/24 12:50, Peter Eisentraut wrote: On 13.08.24 21:00, Robert Haas wrote: On Tue, Aug 13, 2024 at 10:38 AM Sofia Kopikova wrote: I noticed that in commit d3cc5ffe81f6 some GUC variables were moved to header files. According to the commit message in 8ec569479, all variables in header fi

Re: Apply PGDLLIMPORT markings to some GUC variables

On 13.08.24 21:00, Robert Haas wrote: On Tue, Aug 13, 2024 at 10:38 AM Sofia Kopikova wrote: I noticed that in commit d3cc5ffe81f6 some GUC variables were moved to header files. According to the commit message in 8ec569479, all variables in header files must be marked with PGDLLIMPORT, am I rig

Re: [bug fix] prepared transaction might be lost when max_prepared_transactions is zero on the subscriber

On Tue, Aug 13, 2024 at 9:48 AM Amit Kapila wrote: > > BTW, this needs to be backpatched till 16 when it has been introduced > by the parallel apply feature as part of commit 216a7848. So, can we > test this patch in back-branches as well? > I was able to reproduce the problem on REL_16_STABLE an

Re: format_datum debugging function

On 12.08.24 23:15, Paul Jungwirth wrote: On 8/12/24 04:32, Aleksander Alekseev wrote: [...] This function takes a Datum and the appropriate out function, and returns a char *. So you can do this: (gdb) call format_datum(range_out, $1) $2 = 0x59162692d938 "[1,4)" I assume a patch like this doe

Re: AIX support

On 14/08/2024 06:31, Srirama Kucherlapati wrote: Hi Heikki & Team, I tried to look at the assembly code changes with our team, in the below file. diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index 29ac6cdcd9..69582f4ae7 100644 --- a/src/include/storage/s_lock.h +++

Re: collect_corrupt_items_vacuum.patch

On 14/08/2024 04:51, Alexander Korotkov wrote: On Tue, Aug 13, 2024 at 10:15 PM Alexander Korotkov wrote: On Tue, Aug 13, 2024 at 9:39 PM Heikki Linnakangas wrote: This causes an assertion failure when executed in a hot standby server: select * from pg_check_visible('pg_database'); TRAP

macOS prefetching support

It seems to me that we could implement prefetching support (USE_PREFETCH) on macOS using the fcntl() command F_RDADVISE. The man page description is a bit terse: F_RDADVISE Issue an advisory read async with no copy to user. But it seems to be the right idea. Was this looked into b

Re: Remove dependence on integer wrapping

On 14/08/2024 06:07, Nathan Bossart wrote: On Tue, Aug 13, 2024 at 04:46:34PM -0500, Nathan Bossart wrote: I've been preparing 0001 for commit. I've attached what I have so far. The main changes are the implementations of pg_abs_* and pg_neg_*. For the former, I've used abs()/i64abs() for the