Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, 16 Aug 2024 at 00:39, Jacob Champion wrote: > > On Thu, Aug 15, 2024 at 3:04 PM Heikki Linnakangas wrote: > > Perhaps we should even change it to return > > 30 for protocol version 3.0, and just leave a note in the docs like > > "in older versions of libpq, this returned 3 for protoco

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

On 15.08.24 19:25, Matthias van de Meent wrote: Apart from the above issue, I'm -0.5 on what to me equates with automated spam to -hackers: the volume of mails would put this around the 16th most common sender on -hackers, with about 400 mails/year (based on 80 new patches for next CF, and 5 CF

Re: Fix memory counter update in reorderbuffer

On Wed, 7 Aug 2024 at 11:48, Amit Kapila wrote: > > On Wed, Aug 7, 2024 at 7:42 AM Masahiko Sawada wrote: > > > > On Tue, Aug 6, 2024 at 1:12 PM Amit Kapila wrote: > > > > > > On Sat, Aug 3, 2024 at 1:21 AM Masahiko Sawada > > > wrote: > > > > > > > > I found a bug in the memory counter update

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

On Thu, 15 Aug 2024 13:58:03 +0300 Aleksander Alekseev wrote: > Hi, > > > Perhaps we should also add casts between bytea and the integer/numeric > > types. That might be easier to use than functions in some > > circumstances. > > > > When casting a numeric to an integer, the result is rounded to

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

Hi, > When we add such casts between bytea and the integer/numeric types, > one of the problems mentioned the first of the thread, that is, > "we don't have a convenient way of casting a bytea to an integer / bigint > and vice versa", would seem be resolved. > > On the other hand, I suppose get_by

[PROPOSAL] : Disallow use of empty column name in (column_name '') in ALTER or CREATE of foreign table.

Hi, -- Actual column names used while creation of foreign table are not allowed to be an empty string, but when we use column_name as an empty string in OPTIONS during CREATE or ALTER of fo

Re: Pgoutput not capturing the generated columns

On Fri, 16 Aug 2024 at 10:04, Shubham Khanna wrote: > > On Thu, Aug 8, 2024 at 12:43 PM Peter Smith wrote: > > > > Hi Shubham, > > > > I think the v25-0001 patch only half-fixes the problems reported in my > > v24-0001 review. > > > > ~ > > > > Background (from the commit message): > > This commi

Re: Parallel CREATE INDEX for BRIN indexes

On 8/15/24 15:48, Peter Eisentraut wrote: > On 13.04.24 23:04, Tomas Vondra wrote: While preparing a differential code coverage report between 16 and HEAD, one thing that stands out is the parallel brin build code. Neither on coverage.postgresql.org nor locally is that code reac

Re: Conflict detection and logging in logical replication

On Fri, Aug 16, 2024 at 12:19 PM Amit Kapila wrote: > > On Fri, Aug 16, 2024 at 11:48 AM shveta malik wrote: > > > > On Fri, Aug 16, 2024 at 10:46 AM shveta malik > > wrote: > > > > > > 3) > > > For update_exists(), we dump: > > > Key (a, b)=(2, 1) > > > > > > For delete_missing, update_missing

Re: race condition in pg_class

On 14/07/2024 20:48, Noah Misch wrote: I've pushed the two patches for your reports. To placate cfbot, I'm attaching the remaining patches. inplace090-LOCKTAG_TUPLE-eoxact-v8.patch: Makes sense. A comment would be in order, it looks pretty random as it is. Something like: /* * Tuple locks

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

On 14.08.24 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 integer. This allows

Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY

Hello, everyone. I have updated the spec to reproduce the issue, now it includes cases with both CREATE INDEX and REINDEX. To run: make -C src/test/modules/injection_points/ check Issue reproduced on empty index, but it may happen on index of any with the same probability. It is not critic

Re: Useless parameter 'cur_skey' in IndexScanOK

On 03/07/2024 16:46, Daniel Gustafsson wrote: On 3 Jul 2024, at 15:41, Aleksander Alekseev wrote: The 'cur_skey' parameter in `IndexScanOK` funciton doesn't seem to be useful. Good catch. As I understand it is not used for anything since a78fcfb51243 (dated 2006) and this is a static functio

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

On Fri, 16 Aug 2024 11:41:37 +0300 Aleksander Alekseev wrote: > Hi, > > > When we add such casts between bytea and the integer/numeric types, > > one of the problems mentioned the first of the thread, that is, > > "we don't have a convenient way of casting a bytea to an integer / bigint > > and

Re: refactor the CopyOneRowTo

On 31/07/2024 16:30, Junwang Zhao wrote: On Fri, Jul 5, 2024 at 12:26 AM jian he wrote: overall less "if else" logic, also copy format don't change during copy, we don't need to check binary or nor for each datum value. Committed, thanks. For the archives: this code is in a very hot path dur

Re: Vacuum statistics

On Thu, Aug 15, 2024 at 4:49 PM Alena Rybakina wrote: > > Hi! I've applied all the v5 patches. 0002 and 0003 have white space errors. + +Number of times blocks of this index were already found +in the buffer cache by vacuum operations, so that a read was not necessary +

Re: Drop database command will raise "wrong tuple length" if pg_database tuple contains toast attribute.

Hi Ayush, On 8/13/24 07:37, Ayush Tiwari wrote: > Hi hackers, > > We encountered an issue lately, that if the database grants too many > roles `datacl` is toasted, following which, the drop database command > will fail with error "wrong tuple length". > > To reproduce the issue, please follow be

Re: [PROPOSAL] : Disallow use of empty column name in (column_name '') in ALTER or CREATE of foreign table.

Oops... I forgot to attach the patch. Thanks to Amul Sul for pointing that out. :) On Fri, Aug 16, 2024 at 2:37 PM Nishant Sharma < nishant.sha...@enterprisedb.com> wrote: > Hi, > > > > -

Re: Conflict detection and logging in logical replication

Hello! > I think you might misunderstand the behavior of CheckAndReportConflict(), even > if it found a conflict, it still inserts the tuple into the index which means > the change is anyway applied. > In the above conditions where a concurrent tuple insertion is removed > or rolled back before C

Ineffective Assert-check in CopyMultiInsertInfoNextFreeSlot()

Hi, The Assert(buffer != NULL) is placed after the buffer is accessed, which could lead to a segmentation fault before the check is executed. Attached a small patch to correct that. -- Regards, Amul Sul EDB: http://www.enterprisedb.com diff --git a/src/backend/commands/copyfrom.c b/src/backend/c

Re: Relation bulk write facility

On 03/07/2024 06:41, Noah Misch wrote: On Tue, Jul 02, 2024 at 02:42:50PM +0300, Heikki Linnakangas wrote: On 02/07/2024 02:24, Noah Misch wrote: On Tue, Jul 02, 2024 at 12:53:05AM +0300, Heikki Linnakangas wrote: Fortunately, fsync() on a file that's already flushed to disk is pretty cheap.

Re: Make query cancellation keys longer

On Thu, Aug 15, 2024 at 6:07 PM Heikki Linnakangas wrote: > Ok, I've read through that thread now, and opined there too. One > difference is with libpq option name: My patch adds "protocol_version", > while Jelte proposes "max_protocol_version". I don't have strong > opinions on that. I hope the e

Re: generic plans and "initial" pruning

On Fri, Aug 16, 2024 at 12:35 AM Robert Haas wrote: > On Thu, Aug 15, 2024 at 8:57 AM Amit Langote wrote: > > TBH, it's more of a hunch that people who are not involved in this > > development might find the new reality, whereby the execution is not > > racefree until ExecutorRun(), hard to reaso

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Thu, Aug 15, 2024 at 6:03 PM Heikki Linnakangas wrote: > On the default for "max_protocol_version": I'm pretty disappointed if we > cannot change the default to "latest". I realize that that won't work > with poolers that don't implement NegotiateProtocolVersion. But I'm > afraid if we make the

Re: thread-safety: gmtime_r(), localtime_r()

On Tue, Jul 23, 2024 at 10:52 PM Peter Eisentraut wrote: > Let's look at what this code actually does. It just takes the current > time and then loops through all possible weekdays and months to collect > and cache the localized names. The current time or time zone doesn't > actually matter for

Re: thread-safety: gmtime_r(), localtime_r()

On Sat, Aug 17, 2024 at 1:09 AM Thomas Munro wrote: > This change complements CF #5170's change strftime()->strftime_l(), to > make the function fully thread-safe. (Erm, I meant its standard library... of course it has its own global variables to worry about still.)

Re: [BUG?] check_exclusion_or_unique_constraint false negative

Hello! > In addition, I think the bug is not a blocker for the conflict detection > feature. As the feature simply reports the current behavior of the logical > apply worker (either unique violation or tuple missing) without introducing any > new functionality. Furthermore, I think that the new Ex

Re: Make query cancellation keys longer

On 16/08/2024 15:31, Robert Haas wrote: On Thu, Aug 15, 2024 at 6:07 PM Heikki Linnakangas wrote: Ok, I've read through that thread now, and opined there too. One difference is with libpq option name: My patch adds "protocol_version", while Jelte proposes "max_protocol_version". I don't have st

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

Peter Eisentraut writes: > On 15.08.24 19:25, Matthias van de Meent wrote: >> Apart from the above issue, I'm -0.5 on what to me equates with >> automated spam to -hackers: the volume of mails would put this around >> the 16th most common sender on -hackers, with about 400 mails/year >> (based

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On 16/08/2024 15:55, Robert Haas wrote: On Thu, Aug 15, 2024 at 6:03 PM Heikki Linnakangas wrote: On the default for "max_protocol_version": I'm pretty disappointed if we cannot change the default to "latest". I realize that that won't work with poolers that don't implement NegotiateProtocolVer

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, 16 Aug 2024 at 16:51, Heikki Linnakangas wrote: > That said, I think we *should* change the default for the time being, so > that developers working on the bleeding edge and building from git get > some exposure to it. Hopefully that will nudge some of the poolers to > adopt NegotiateProto

Re: [PROPOSAL] : Disallow use of empty column name in (column_name '') in ALTER or CREATE of foreign table.

Nishant Sharma writes: > Actual column names used while creation of foreign table are not allowed to > be an > empty string, but when we use column_name as an empty string in OPTIONS > during > CREATE or ALTER of foreign tables, it is allowed. Is this really a bug? The valid remote names are det

Re: Make query cancellation keys longer

On Fri, Aug 16, 2024 at 10:37 AM Heikki Linnakangas wrote: > If we envision accepting ranges like that in the future, it would be > good to do now rather than later. Otherwise, if someone wants to require > features from protocol 3.2 today, they will have to put > "protocol_version=3.2" in the con

Re: Parallel CREATE INDEX for BRIN indexes

On 16.08.24 11:22, Tomas Vondra wrote: These pageinspect tests added a new use of the md5() function.  We got rid of those in the tests for PG17.  You could write the test case with something like  SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END), -   (C

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, Aug 16, 2024 at 10:51 AM Heikki Linnakangas wrote: > Now, I think we should still do it, but it might not warrant changing > the default. Unfortunately that means that it will get very little > adoption. It will only be adopted as a side-effect of some other changes > that make people chan

Re: thread-safety: gmtime_r(), localtime_r()

Here is an updated patch version. I have changed the backend call from localtime() to gmtime() but then also to gmtime_r(). I moved the _POSIX_C_SOURCE definition for MinGW from the header file to a command-line option (-D_POSIX_C_SOURCE). This matches the treatment of _GNU_SOURCE and simil

Re: Remove dependence on integer wrapping

On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote: > This updated version LGTM, I agree it's a good use of pg_abs_s32(). Committed. -- nathan

Re: First draft of PG 17 release notes

On Wed, Jul 17, 2024 at 03:32:45PM +0900, Kisoon Kwon wrote: > Hi, > > In the PG17 release notes, I noticed it is mentioned as > "pg_attribute.stxstattarget" which seems incorrect. > In my opinion, it should be "pg_statistic_ext.stxstattarget" because the > "stxstattarget" column is part of the "p

Re: First draft of PG 17 release notes

On Fri, Jul 26, 2024 at 01:22:24PM +0900, Yugo Nagata wrote: > I found the following in the release notes: > > Change file boundary handling of two WAL file name functions > (Kyotaro Horiguchi, Andres Freund, Bruce Momjian) > > The functions pg_walfile_name() and pg_walfile_name_offset() used

Re: First draft of PG 17 release notes

On Thu, Aug 8, 2024 at 08:55:53AM -0500, Justin Pryzby wrote: > > Add server variable huge_page_size to report the use of huge pages by > > The new variable is huge_page_status; h_p_size is several years old. Fixed. I created this mistake when I was adding links to the SGML file. > BTW, I was

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

On Wed, Aug 14, 2024 at 3:40 PM Jelte Fennema-Nio wrote: > > 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 sho

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, Aug 16, 2024 at 12:05 AM Jelte Fennema-Nio wrote: > > On Fri, 16 Aug 2024 at 00:39, Jacob Champion > wrote: > > > > On Thu, Aug 15, 2024 at 3:04 PM Heikki Linnakangas wrote: > > > Perhaps we should even change it to return > > > 30 for protocol version 3.0, and just leave a note in t

Re: Remove dependence on integer wrapping

Hello Nathan and Joe, 16.08.2024 19:52, Nathan Bossart wrote: On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote: This updated version LGTM, I agree it's a good use of pg_abs_s32(). Committed. Thank you for working on that issue! I've tried `make check` with CC=gcc-13 CPPFLAGS=

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, Aug 16, 2024 at 1:44 PM Jacob Champion wrote: > > https://github.com/psycopg/psycopg2/blob/658afe4cd90d3e167d7c98d22824a8d6ec895b1c/tests/test_async.py#L89 > > https://github.com/infusion/PHP/blob/7ebefb6426bb4b4820a30cca5c3a10bfd757b6ea/ext/pgsql/pgsql.c#L864 IMHO these example

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

On Fri, Aug 16, 2024 at 10:23 AM Maciek Sakrejda wrote: > > 1. A very short blurb like: "This thread was added to the commitfest > > with ID 1234" > > 2. A link to the commitfest entry > > 3. A link to the cfbot CI runs > > 4. A link to the diff on GitHub > > 5. Any other suggestions? > > I would

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

On Thu, Aug 15, 2024 at 9:33 AM Peter Eisentraut wrote: > But a more serious concern here is that the patches created by the cfbot > are not canonical. There are various heuristics when they get applied. It's true that the code required for CFBot to simply apply a patch is nontrivial. We're acco

Re: Remove dependence on integer wrapping

On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote: > Sp it looks like jsonb_array_element_text() still needs the same > treatment as jsonb_array_element(). D'oh. I added a test for that but didn't actually fix the code. I think we just need something like the following. diff --gi

Re: Remove dependence on integer wrapping

On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote: > On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote: >> #6  0x5576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at >> bitmapset.c:691 >> 691 if (result >= 0 || HAS_MULTIPLE_ONES(w))

Re: macOS prefetching support

On 14.08.24 16:39, Peter Eisentraut wrote: 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 p

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On 16/08/2024 21:01, Robert Haas wrote: On Fri, Aug 16, 2024 at 1:44 PM Jacob Champion wrote: https://github.com/psycopg/psycopg2/blob/658afe4cd90d3e167d7c98d22824a8d6ec895b1c/tests/test_async.py#L89 https://github.com/infusion/PHP/blob/7ebefb6426bb4b4820a30cca5c3a10bfd757b6ea/ext/p

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, 16 Aug 2024 at 15:26, Heikki Linnakangas wrote: > On 16/08/2024 21:01, Robert Haas wrote: > > On Fri, Aug 16, 2024 at 1:44 PM Jacob Champion > > wrote: > >> > https://github.com/psycopg/psycopg2/blob/658afe4cd90d3e167d7c98d22824a8d6ec895b1c/tests/test_async.py#L89 > >> > https://github.c

Re: pg_verifybackup: TAR format backup verification

On Wed, Aug 14, 2024 at 12:44 PM Robert Haas wrote: > 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-r

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On 16/08/2024 22:45, Dave Cramer wrote: On Fri, 16 Aug 2024 at 15:26, Heikki Linnakangas > wrote: On 16/08/2024 21:01, Robert Haas wrote: > On Fri, Aug 16, 2024 at 1:44 PM Jacob Champion > mailto:jacob.champ...@enterprisedb.com>> wrote: >> https://

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, 16 Aug 2024 at 15:54, Heikki Linnakangas wrote: > On 16/08/2024 22:45, Dave Cramer wrote: > > On Fri, 16 Aug 2024 at 15:26, Heikki Linnakangas > > wrote: > > > > On 16/08/2024 21:01, Robert Haas wrote: > > > On Fri, Aug 16, 2024 at 1:44 PM Jacob Champion

Re: pg_verifybackup: TAR format backup verification

On Fri, Aug 16, 2024 at 3:53 PM Robert Haas wrote: > +int64 num = strtoi64(relpath, &suffix, 10); Hit send too early. Here, seems like this should be strtoul(), not strtoi64(). The documentation of --format seems to be cut-and-pasted from pg_basebackup and the language isn't really appro

Re: Remove dependence on integer wrapping

On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote: > On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote: >> Sp it looks like jsonb_array_element_text() still needs the same >> treatment as jsonb_array_element(). > > D'oh. I added a test for that but didn't actually fix

Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

On Fri, Aug 16, 2024 at 4:03 PM Dave Cramer wrote: > Admittedly I'm a bit late into this discussion so I may be off base. > Ultimately we need to negotiate the protocol. From what I can tell for libpq > we are providing a function that returns a number, currently 3. > > The proposal is to change

Re: thread-safety: gmtime_r(), localtime_r()

On Sat, Aug 17, 2024 at 3:43 AM Peter Eisentraut wrote: > I moved the _POSIX_C_SOURCE definition for MinGW from the header file to > a command-line option (-D_POSIX_C_SOURCE). This matches the treatment > of _GNU_SOURCE and similar. I was trying to figure out what else -D_POSIX_C_SOURCE does to

Re: macOS prefetching support

On Sat, Aug 17, 2024 at 6:58 AM Peter Eisentraut wrote: > What to do about the order of the symbols and include files. I threw > something into src/include/port/darwin.h, but I'm not sure if that's > good. Alternatively, we could not use __darwin__ but instead the more > standard and predefined

Re: macOS prefetching support

Thomas Munro writes: > Hmm. fd.h and fd.c test for F_NOCACHE, which is pretty closely > related. Now I'm wondering why we actually need this in > pg_config_manual.h at all. Who would turn it off at compile time, and > why would they not be satisfied with setting relevant GUCs to 0? +1 for not

Re: Ineffective Assert-check in CopyMultiInsertInfoNextFreeSlot()

On Fri, 16 Aug 2024 at 23:48, Amul Sul wrote: > The Assert(buffer != NULL) is placed after the buffer is accessed, > which could lead to a segmentation fault before the check is executed. Yeah, that's not great. Technically the Assert does not add any value in terms of catching bugs in the code,

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

On Thu, 15 Aug 2024 at 10:40, Jelte Fennema-Nio wrote: > > 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

Re: Streaming I/O, vectored I/O (WIP)

On Wed, Jul 10, 2024 at 07:21:59PM +0300, Nazir Bilal Yavuz wrote: > Hi, > > It seems that Heikki's 'v9.heikki-0007-Trivial-comment-fixes.patch' > [1] is partially applied, the top comment is not updated. The attached > patch just updates it. > > [1] > https://www.postgresql.org/message-id/289a1

Re: macOS prefetching support

On Sat, Aug 17, 2024 at 6:58 AM Peter Eisentraut wrote: > solaris fake I'm half tempted to suggest that we take this exception out. If it's there, we call it. It doesn't do anything right now, but it's a cheap empty user space function, and I heard they are thinking about adding a real

Re: race condition in pg_class

Thanks for reviewing. On Fri, Aug 16, 2024 at 12:26:28PM +0300, Heikki Linnakangas wrote: > On 14/07/2024 20:48, Noah Misch wrote: > > I've pushed the two patches for your reports. To placate cfbot, I'm > > attaching > > the remaining patches. > > inplace090-LOCKTAG_TUPLE-eoxact-v8.patch: Makes

Re: Speed up Hash Join by teaching ExprState about hashing

On Thu, 15 Aug 2024 at 19:50, Alexey Dvoichenkov wrote: > I gave v3 another look. One tiny thing I've noticed is that you > removed ExecHashGetHashValue() but not its forward declaration in > include/executor/nodeHash.h Fixed > I also reviewed the JIT code this time, it looks reasonable to > me.