Re: session username in default psql prompt?

2024-03-25 Thread Jelte Fennema-Nio
On Mon, 25 Mar 2024 at 14:06, Robert Haas wrote: > On Mon, Mar 25, 2024 at 4:30 AM Jelte Fennema-Nio wrote: > > That problem seems easy to address by adding a newline into the > > default prompt. > > Ugh. Please, no! I guess it's partially a matter of taste, but personally

Re: session username in default psql prompt?

2024-03-25 Thread Jelte Fennema-Nio
On Mon, 25 Mar 2024 at 11:40, jian he wrote: > transaction related information lost. Ah yeah, it seems I somehow lost the %x How about: \set PROMPT1 '%n@%~\n%R%x%# ' Or maybe even this more verbose one, which closely matches the postgresql:// connection string format: \set PROMPT1

Re: session username in default psql prompt?

2024-03-25 Thread Jelte Fennema-Nio
On Mon, 25 Mar 2024 at 09:30, Jelte Fennema-Nio wrote: > \set PROMPT1 '%n@%~%R%\n# ' Obviously I meant to put the \n before the %: \set PROMPT1 '%n@%~%R\n%# '

Re: session username in default psql prompt?

2024-03-25 Thread Jelte Fennema-Nio
On Sat, 23 Mar 2024 at 00:34, Tom Lane wrote: > Prompt space is > expensive and precious, at least for people who aren't in the > habit of working in very wide windows. That problem seems easy to address by adding a newline into the default prompt. Something like this: \set PROMPT1 '%n@%~%R%\n#

Re: Support a wildcard in backtrace_functions

2024-03-22 Thread Jelte Fennema-Nio
On Fri, 22 Mar 2024 at 11:14, Bharath Rupireddy wrote: > A few comments: > > 1. > @@ -832,6 +849,9 @@ matches_backtrace_functions(const char *funcname) > { > const char *p; > > +if (!backtrace_functions || backtrace_functions[0] == '\0') > +return true; > + > > Shouldn't this be

Re: Support a wildcard in backtrace_functions

2024-03-22 Thread Jelte Fennema-Nio
On Thu, 21 Mar 2024 at 15:41, Jelte Fennema-Nio wrote: > Attached is a patch that implements this. Since the more I think about > it, the more I like this approach. I now added a 3rd patch to this patchset which changes the log_backtrace default to "internal", because it seems qu

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-22 Thread Jelte Fennema-Nio
On Thu, 21 Mar 2024 at 03:54, Noah Misch wrote: > > On Mon, Mar 18, 2024 at 07:40:10PM +0100, Alvaro Herrera wrote: > > I enabled the test again and also pushed the changes to dblink, > > isolationtester and fe_utils (which AFAICS is used by pg_dump, > > I recommend adding a libpqsrv_cancel()

Re: UUID v7

2024-03-21 Thread Jelte Fennema-Nio
On Wed, 20 Mar 2024 at 19:08, Andrey M. Borodin wrote: > Timer-based bits contribute to global sortability. But the real timers we > have are not even millisecond adjusted. We can hope for ~few ms variation in > one datacenter or in presence of atomic clocks. I think the main benefit of using

Re: Support a wildcard in backtrace_functions

2024-03-21 Thread Jelte Fennema-Nio
On Wed, 13 Mar 2024 at 16:32, Jelte Fennema-Nio wrote: > How > about the following aproach. It still uses 3 GUCs, but they now all > work together. There's one entry point and two additional filters > (level and function name) > > # Says what log entries to log backtraces f

Re: Refactoring backend fork+exec code

2024-03-21 Thread Jelte Fennema-Nio
On Wed, 20 Mar 2024 at 08:16, Heikki Linnakangas wrote: > Yeah, it's not a very valuable assertion. Removed, thanks! How about we add it as a static assert instead of removing it, like we have for many other similar arrays. v1-0001-Add-child_process_kinds-static-assert.patch Description:

Re: Flushing large data immediately in pqcomm

2024-03-21 Thread Jelte Fennema-Nio
On Thu, 21 Mar 2024 at 01:24, Melih Mutlu wrote: > What if I do a simple comparison like PqSendStart == PqSendPointer instead of > calling pq_is_send_pending() Yeah, that sounds worth trying out. So the new suggestions to fix the perf issues on small message sizes would be: 1. add "inline" to

Re: Flushing large data immediately in pqcomm

2024-03-21 Thread Jelte Fennema-Nio
On Thu, 21 Mar 2024 at 01:45, David Rowley wrote: > As I understand the code, there's no problem calling > internal_flush_buffer() when the buffer is empty and I suspect that if > we're sending a few buffers with "len > PqSendBufferSize" that it's > just so unlikely that the buffer is empty that

Re: What is a typical precision of gettimeofday()?

2024-03-20 Thread Jelte Fennema-Nio
On Wed, 20 Mar 2024 at 07:35, Peter Eisentraut wrote: > If we want to be robust without any guarantees from gettimeofday(), then > arguably gettimeofday() is not the right underlying function to use for > UUIDv7. There's also clock_gettime which exposes its resolution using clock_getres

Re: Possibility to disable `ALTER SYSTEM`

2024-03-20 Thread Jelte Fennema-Nio
On Wed, 20 Mar 2024 at 14:04, Greg Sabino Mullane wrote: >> >> As a bonus, if that GUC is set, we could even check at server startup that >> all the configuration files are not writable by the postgres user, >> and print a warning or refuse to start up if they are. > > > Ugh, please let's not do

Re: Possibility to disable `ALTER SYSTEM`

2024-03-19 Thread Jelte Fennema-Nio
On Tue, 19 Mar 2024 at 17:05, Tom Lane wrote: > I've said this repeatedly: it's not enough. The only reason we need > any feature whatsoever is that somebody doesn't trust their database > superusers to not try to modify the configuration. And as everyone else on this thread has said: It is

Re: Possibility to disable `ALTER SYSTEM`

2024-03-19 Thread Jelte Fennema-Nio
On Tue, 19 Mar 2024 at 15:52, Tom Lane wrote: > I like this idea. The "bonus" is not optional though, because > setting the files' ownership/permissions is the only way to be > sure that the prohibition is even a little bit bulletproof. I don't agree with this. The only "normal" way of

Re: Possibility to disable `ALTER SYSTEM`

2024-03-19 Thread Jelte Fennema-Nio
On Mon, 18 Mar 2024 at 18:27, Robert Haas wrote: > I think for now we > should just file this under "Other platforms and clients," which only > has one existing setting. If the number of settings of this type > grows, we can split it out. Done. I also included a patch to rename

Re: Possibility to disable `ALTER SYSTEM`

2024-03-18 Thread Jelte Fennema-Nio
On Mon, 18 Mar 2024 at 13:57, Robert Haas wrote: > I would have been somewhat inclined to find an existing section > of postgresql.auto.conf for this parameter, perhaps "platform and > version compatibility". I tried to find an existing section, but I couldn't find any that this new GUC would

Re: Possibility to disable `ALTER SYSTEM`

2024-03-15 Thread Jelte Fennema-Nio
On Fri, 15 Mar 2024 at 11:08, Daniel Gustafsson wrote: > Another quirk for the documentation of this: if I disable ALTER SYSTEM I would > assume that postgresql.auto.conf is no longer consumed, but it still is (and > still need to be), so maybe "enable/disable" is the wrong choice of words?

Re: Possibility to disable `ALTER SYSTEM`

2024-03-15 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 22:15, Maciek Sakrejda wrote: > In this case, the end user with access to Postgres superuser > privileges presumably also has access to the outside configuration > mechanism. The goal is not to prevent them from changing settings, but > to offer guard rails that prevent

Re: Possibility to disable `ALTER SYSTEM`

2024-03-14 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 17:37, Robert Haas wrote: > or in the > alternative (2) but with the GUC being PGC_SIGHUP and > GUC_DISALLOW_IN_AUTO_FILE. I believe there would be adequate consensus > to proceed with either of those approaches. Anybody feel like coding > it up? Here is a slightly

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

2024-03-14 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 16:45, Robert Haas wrote: > I feel bad arguing about the patches that you think are a slam-dunk, > but I find myself disagreeing with the design choices. No worries, thanks a lot for responding. I'm happy to discuss this design further. I didn't necessarily mean these

Re: Flushing large data immediately in pqcomm

2024-03-14 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 13:12, Robert Haas wrote: > > On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu wrote: > > 1- Even though I expect both the patch and HEAD behave similarly in case of > > small data (case 1: 100 bytes), the patch runs slightly slower than HEAD. > > I wonder why this happens. It

Re: Flushing large data immediately in pqcomm

2024-03-14 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 12:22, Melih Mutlu wrote: > I did some experiments with this patch, after previous discussions One thing I noticed is that the buffer sizes don't seem to matter much in your experiments, even though Andres his expectation was that 1400 would be better. I think I know the

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-14 Thread Jelte Fennema-Nio
On Thu, 14 Mar 2024 at 11:33, Alvaro Herrera wrote: > Hmm, isn't this basically saying that we're giving up on reliably > canceling queries altogether? I mean, maybe we'd like to instead fix > the bug about canceling queries in extended query protocol ... > Isn't that something you're worried

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-14 Thread Jelte Fennema-Nio
On Wed, 13 Mar 2024 at 20:08, Jacob Champion wrote: > I hit this on my machine. With the attached diff I can reproduce > constantly (including with the most recent test patch); I think the > cancel must be arriving between the bind/execute steps? Nice find! Your explanation makes total sense.

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

2024-03-13 Thread Jelte Fennema-Nio
Fixed some conflicts again, as well as adding a connection option to choose the requested protocol version (as discussed in[1]). This new connection option is not useful when connecting to any of the supported postgres versions. But it can be useful when connecting to PG versions before 9.3. Or

Re: Support a wildcard in backtrace_functions

2024-03-13 Thread Jelte Fennema-Nio
On Wed, 13 Mar 2024 at 15:20, Peter Eisentraut wrote: > Hence the idea > > backtrace_on_error = {all|internal|none} > > which could default to 'internal'. I think one use-case that I'd personally at least would like to see covered is being able to get backtraces on all warnings. How would

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-13 Thread Jelte Fennema-Nio
On Wed, 13 Mar 2024 at 04:53, Tom Lane wrote: > I suspect it's basically just a > timing dependency. Have you thought about the fact that a cancel > request is a no-op if it arrives after the query's done? I agree it's probably a timing issue. The cancel being received after the query is done

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 19:28, Alvaro Herrera wrote: > > On 2024-Mar-12, Alvaro Herrera wrote: > > > Hmm, buildfarm member kestrel (which uses > > -fsanitize=undefined,alignment) failed: > > > > # Running: libpq_pipeline -r 700 cancel port=49975 host=/tmp/dFh46H7YGc > > dbname='postgres' > > test

Re: UUID v7

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 18:18, Sergey Prokhorenko wrote: > Andrey and you simply did not read the RFC a little further down in the text: You're totally right, sorry about that. Maybe it would be good to move those subsections around a bit in the RFC though, so that anything related to only one

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 15:04, Jelte Fennema-Nio wrote: > Attached a tiny change to 0001 One more tiny comment change, stating that pg_cancel is used by the deprecated PQcancel function. v36-0001-libpq-Add-encrypted-and-non-blocking-query-cance.patch Description: Binary data v36-0002-St

Re: UUID v7

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 07:32, Michael Paquier wrote: > Sure, there is no problem in discussing a patch to implement a > behavior. But I disagree about taking a risk in merging something > that could become non-compliant with the approved RFC, if the draft is > approved at the end, of course.

Re: UUID v7

2024-03-12 Thread Jelte Fennema-Nio
On Mon, 11 Mar 2024 at 19:27, Andrey M. Borodin wrote: > Sorry for this long and vague explanation, if it still seems too uncertain we > can have a chat or something like that. I don't think this number picking > stuff deserve to be commented, because it still is quite close to random. RFC >

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-12 Thread Jelte Fennema-Nio
b5edb603e Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Thu, 14 Dec 2023 13:39:09 +0100 Subject: [PATCH v35 2/2] Start using new libpq cancel APIs A previous commit introduced new APIs to libpq for cancelling queries. This replaces the usage of the old APIs in most of the codebase wit

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 10:53, Jelte Fennema-Nio wrote: > I'd rather fail as hard as possible when someone is using the API > wrongly. To be clear, this is my way of looking at it. If you feel strongly about that we should not change conn.status, I'm fine with making that change to the patchset.

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-12 Thread Jelte Fennema-Nio
On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera wrote: > If we do this and we see conn.status is not ALLOCATED, meaning a cancel > is already ongoing, shouldn't we leave conn.status alone instead of > changing to CONNECTION_BAD? I mean, we shouldn't be juggling the elbow > of whoever's doing that,

Re: UUID v7

2024-03-11 Thread Jelte Fennema-Nio
Attached a few comment fixes/improvements and a pgindent run (patch 0002-0004) Now with the added comments, one thing pops out to me: The comments mention that we use "Monotonic Random", but when I read the spec that explicitly recommends against using an increment of 1 when using monotonic

Re: Support a wildcard in backtrace_functions

2024-03-11 Thread Jelte Fennema-Nio
On Fri, 8 Mar 2024 at 17:17, Bharath Rupireddy wrote: > Hm, we may not want backtrace_on_internal_error to be on by default. > AFAICS, all ERRCODE_INTERNAL_ERROR are identifiable with the error > message, so it's sort of easy for one to track down the cause of it > without actually needing to log

Re: Reducing the log spam

2024-03-11 Thread Jelte Fennema-Nio
- the subscriber's server log. + the subscriber's server log if you remove 23505 from + . This seems like a pretty big regression. Being able to know why your replication got closed seems pretty critical. On Mon, 11 Mar 2024 at 03:44, Laurenz Albe wrote: > > On Sat, 2024-03-09 at 14:03

Re: Make query cancellation keys longer

2024-03-09 Thread Jelte Fennema-Nio
On Fri, 8 Mar 2024 at 23:20, Heikki Linnakangas wrote: > Added some documentation. There's more work to be done there, but at > least the message type descriptions are now up-to-date. Thanks, that's very helpful. > The nice thing about relying on the message length was that we could > just

Re: Support a wildcard in backtrace_functions

2024-03-08 Thread Jelte Fennema-Nio
On Fri, 8 Mar 2024 at 15:51, Peter Eisentraut wrote: > What is the relationship of these changes with the recently added > backtrace_on_internal_error? I think that's a reasonable question. And the follow up ones too. I think it all depends on how close we consider backtrace_on_internal_error

Re: Support a wildcard in backtrace_functions

2024-03-08 Thread Jelte Fennema-Nio
On Fri, 8 Mar 2024 at 14:42, Daniel Gustafsson wrote: > My documentation comments from upthread still > stands, but other than those this version LGTM. Ah yeah, I forgot about those. Fixed now. v6-0001-Add-backtrace_functions_min_level.patch Description: Binary data

Re: Support a wildcard in backtrace_functions

2024-03-08 Thread Jelte Fennema-Nio
On Fri, 8 Mar 2024 at 10:59, Alvaro Herrera wrote: > > On 2024-Mar-08, Bharath Rupireddy wrote: > > > This works only if '* 'is specified as the only one character in > > backtrace_functions = '*', right? If yes, what if someone sets > > backtrace_functions = 'foo, bar, *, baz'? > > It throws an

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-07 Thread Jelte Fennema-Nio
Attached is a new patchset with various changes. I created a dedicated 0002 patch to add tests for the already existing cancellation functions, because that seemed useful for another thread where changes to the cancellation protocol are being proposed[1]. [1]:

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-06 Thread Jelte Fennema-Nio
On Wed, 6 Mar 2024 at 19:22, Alvaro Herrera wrote: > Docs: one bogus "that that". will fix > Did we consider having PQcancelConn() instead be called > PQcancelCreate()? Fine by me > Also, the comment still says > "Asynchronously cancel a query on the given connection. This requires > polling

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-03-06 Thread Jelte Fennema-Nio
or PQconnectStart were not listing all relevant statuses, so I fixed that in patch 0001. From 40d3d9b0f4058bcf3041e63f71ce4c56e43e73f2 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 6 Mar 2024 18:33:49 +0100 Subject: [PATCH v32 1/3] Add missing connection statuses to docs The list

Re: Hooking into ExplainOneQuery() complicated by missing standard_ExplainOneQuery

2024-03-06 Thread Jelte Fennema-Nio
This patch would definitely be useful for Citus. We indeed currently copy all of that code into our own explain hook. And it seems we actually have some bug. Because the es->memory branches were not copied (probably because this code didn't exist when we copied it).

Re: Injection points: some tools to wait and wake

2024-03-06 Thread Jelte Fennema-Nio
On Wed, 6 Mar 2024 at 07:17, Michael Paquier wrote: > I'm open to that if there's enough demand for it, but I > don't know how much we should accomodate with the existing > requirements of contrib/ for something that's only developer-oriented. There's quite a few developer-oriented GUCs as well,

Re: Improve readability by using designated initializers when possible

2024-03-05 Thread Jelte Fennema-Nio
On Tue, 5 Mar 2024 at 15:30, Japin Li wrote: > There is a warning if remove it, so I keep it. > > /home/japin/Codes/postgres/build/../src/backend/executor/execExprInterp.c:118:33: > warning: label ‘CASE_EEOP_LAST’ defined but not used [-Wunused-label] > 118 | #define EEO_CASE(name)

Re: Reducing the log spam

2024-03-05 Thread Jelte Fennema-Nio
On Tue, 5 Mar 2024 at 14:55, Jim Jones wrote: > > So what about a parameter "log_suppress_sqlstates" that suppresses > > logging ERROR and FATAL messages with the enumerated SQL states? Big +1 from me for this idea.

Re: Improve readability by using designated initializers when possible

2024-03-05 Thread Jelte Fennema-Nio
On Tue, 5 Mar 2024 at 14:50, Japin Li wrote: > Attach a patch to rewrite dispatch_table array using C99-designated > initializer syntax. Looks good. Two small things: + [EEOP_LAST] = &_EEOP_LAST, Is EEOP_LAST actually needed in this array? It seems unused afaict. If indeed not needed,

Re: Injection points: some tools to wait and wake

2024-03-05 Thread Jelte Fennema-Nio
On Mon, 4 Mar 2024 at 23:23, Michael Paquier wrote: > In my experience, anybody who does serious testing with their product > integrated with Postgres have two or three types of builds with their > own scripts: one with assertions, -DG and other developer-oriented > options enabled, and one for

Re: Injection points: some tools to wait and wake

2024-03-04 Thread Jelte Fennema-Nio
On Mon, 4 Mar 2024 at 06:27, Michael Paquier wrote: > I have mentioned that on a separate thread, Yeah, I didn't read all emails related to this feature > Perhaps we could consider that as an exception in "contrib", or have a > separate path for test modules we're OK to install (the calls had >

Re: Injection points: some tools to wait and wake

2024-03-03 Thread Jelte Fennema-Nio
I noticed this was committed, and took a quick look. It sounds very useful for testing Citus to be able to use injection points too, but I noticed this code is included in src/test/modules. It sounds like that location will make it somewhat hard to install. If that's indeed the case, would it be

Re: Make query cancellation keys longer

2024-03-03 Thread Jelte Fennema-Nio
On Sun, 3 Mar 2024 at 15:27, Jelte Fennema-Nio wrote: > + case EOF: > + /* We'll come back when there is more data */ > + return PGRES_POLLING_READING; > > Nice catch, I'll go steal this for my patchset which adds all the > necessary changes to be able to do a protocol

Re: Make query cancellation keys longer

2024-03-03 Thread Jelte Fennema-Nio
On Fri, 1 Mar 2024 at 06:19, Jelte Fennema-Nio wrote: > This is a preliminary review. I'll look at this more closely soon. Some more thoughts after looking some more at the proposed changes +#define EXTENDED_CANCEL_REQUEST_CODE PG_PROTOCOL(1234,5677) nit: I think the code should be 1234,5

Re: Make query cancellation keys longer

2024-03-02 Thread Jelte Fennema-Nio
On Fri, 1 Mar 2024 at 15:19, Peter Eisentraut wrote: > > One complication with this was that because we no longer know how long > > the key should be, 4-bytes or something longer, until the backend has > > performed the protocol negotiation, we cannot generate the key in the > > postmaster before

Re: Improve readability by using designated initializers when possible

2024-02-29 Thread Jelte Fennema-Nio
2001 From: Jelte Fennema-Nio Date: Tue, 27 Feb 2024 12:26:10 +0100 Subject: [PATCH v9] Simplify pg_enc2gettext_tbl Use designated initialization of pg_enc2gettext_tbl to simplify the implementation of raw_pg_bind_textdomain_codeset. Now iteration over the array is not needed anymore. Instead the desire

Re: ALTER TABLE SET ACCESS METHOD on partitioned tables

2024-02-29 Thread Jelte Fennema-Nio
On Fri, 1 Mar 2024 at 06:15, Michael Paquier wrote: > > On Fri, Mar 01, 2024 at 05:43:25AM +0100, Jelte Fennema-Nio wrote: > > I think we should set the AM OID explicitly. Because an important > > thing to consider is: What behaviour makes sense when later > > def

Re: Make query cancellation keys longer

2024-02-29 Thread Jelte Fennema-Nio
This is a preliminary review. I'll look at this more closely soon. On Thu, 29 Feb 2024 at 22:26, Heikki Linnakangas wrote: > If the client requests the "_pq_.extended_query_cancel" protocol > feature, the server will generate a longer 256-bit cancellation key. Huge +1 for this general idea.

Re: ALTER TABLE SET ACCESS METHOD on partitioned tables

2024-02-29 Thread Jelte Fennema-Nio
On Fri, 1 Mar 2024 at 02:57, Michael Paquier wrote: > When it comes to partitioned tables, there is a still a tricky case: > what should we do when a user specifies a non-default value in the SET > ACCESS METHOD clause and it matches default_table_access_method? > Should the relam be 0 or should

Re: Improve readability by using designated initializers when possible

2024-02-29 Thread Jelte Fennema-Nio
itializer array will be initialized with 0/NULL. But I agree it's more consistent, so see attached. From 1a45e03cdaf0c0cf962a33cea8cb25a7c2783d9d Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 27 Feb 2024 12:26:10 +0100 Subject: [PATCH v8] Simplify pg_enc2gettext_tbl Use d

Re: Improve readability by using designated initializers when possible

2024-02-28 Thread Jelte Fennema-Nio
ve this regression. From a51592bda622746b9f015d7993ca19254bedbc0e Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 27 Feb 2024 12:26:10 +0100 Subject: [PATCH v7] Simplify pg_enc2gettext_tbl Use designated initialization of pg_enc2gettext_tbl to simplify the implementation of raw_pg_bind_

Re: When extended query protocol ends?

2024-02-28 Thread Jelte Fennema-Nio
On Wed, 28 Feb 2024 at 17:51, Vladimir Sitnikov wrote: > > Jelte> If the Execute causes an error, is > Jelte> the Query still executed or not? And what about if the Query fails, is > Jelte> the Execute before committed or rolled back? > > Frankly, if there's a requirement from the backend, I

Re: Support a wildcard in backtrace_functions

2024-02-28 Thread Jelte Fennema-Nio
Because when updating its docs I realized that none of the existing level arrays matched what we wanted. From 5ab507155a1cb60e490209d28a092fc2d7c0f6be Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 20 Dec 2023 11:41:18 +0100 Subject: [PATCH v4 2/2] Add wildcard support to backtrace_

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
On Wed, 28 Feb 2024 at 04:59, Michael Paquier wrote: > Cool. I have applied 0004 and most of 0002. Attached is what > remains, where I am wondering if it would be cleaner to do these bits > together (did not look at the whole, yet). Feel free to squash them if you prefer that. I think now that

Re: libpq: PQfnumber overload for not null-terminated strings

2024-02-27 Thread Jelte Fennema-Nio
On Tue, 27 Feb 2024 at 15:49, Ivan Trofimov wrote: > Caching the result of PQfnumber could be done, but would result in > somewhat of a mess on a call-site. To be clear I meant your wrapper around libpq could internally cache this, then the call sites of users of your wrapper would not need to

Re: Support a wildcard in backtrace_functions

2024-02-27 Thread Jelte Fennema-Nio
On Mon, 12 Feb 2024 at 14:27, Jelte Fennema-Nio wrote: > Would a backtrace_functions_level GUC that would default to ERROR be > acceptable in this case? I implemented it this way in the attached patchset. From 71e2c1f1fa903ecfce4a79ff5981d0d754d134a2 Mon Sep 17 00:00:00 2001 From: Jelte F

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
20 years. The commit that made it unnecessary to null-terminate the array was 9d77708d83ee. Attached is v5 of the patchset that also includes this change (with you listed as author). From 945fae648d0a396e9f99413c8e31542ced9b17ba Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 21 Feb 2024 15:34

Re: libpq: PQfnumber overload for not null-terminated strings

2024-02-27 Thread Jelte Fennema-Nio
On Tue, 27 Feb 2024 at 00:31, Ivan Trofimov wrote: > I see now that I failed to express myself clearly: it's not a per-query > overhead, but rather a per-result-field one. I'm fairly sympathetic to decreasing the overhead of any per-row operation. And looking at the code, it doesn't surprise me

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
On Tue, 27 Feb 2024 at 08:57, Alvaro Herrera wrote: > What this says to me is that ObjectClass is/was a somewhat useful > abstraction layer on top of catalog definitions. I'm now not 100% that > poking this hole in the abstraction (by expanding use of catalog OIDs at > the expense of

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
On Tue, 27 Feb 2024 at 12:52, Jelte Fennema-Nio wrote: > Attached is an updated patchset to also convert pg_enc2icu_tbl and > pg_enc2gettext_tbl. I converted pg_enc2gettext_tbl in a separate > commit, because it actually requires some codechanges too. Another small update to also make a

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
xt_tbl. I converted pg_enc2gettext_tbl in a separate commit, because it actually requires some codechanges too. From 4bdf8991d948a251cdade89dbacf121c64f420c7 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 21 Feb 2024 15:34:38 +0100 Subject: [PATCH v3 2/3] Use designated initializ

Re: Improve readability by using designated initializers when possible

2024-02-27 Thread Jelte Fennema-Nio
On Tue, 27 Feb 2024 at 07:25, Michael Paquier wrote: > > On Mon, Feb 26, 2024 at 05:00:13PM +0800, Japin Li wrote: > > On Mon, 26 Feb 2024 at 16:41, jian he wrote: > >> obvious typo errors. > > These would cause compilation failures. Saying that, this is a very > nice cleanup, so I've fixed

Re: [PATCH] Add --syntax to postgres for SQL syntax checking

2024-02-25 Thread Jelte Fennema-Nio
On Sun, 25 Feb 2024 at 23:34, Josef Šimánek wrote: > Exposing parser to frontend tools makes no sense to me Not everyone seems to agree with that, it's actually already done by Lukas from pganalyze: https://github.com/pganalyze/libpg_query

Re: Improve readability by using designated initializers when possible

2024-02-23 Thread Jelte Fennema-Nio
On Fri, 23 Feb 2024 at 02:57, Jeff Davis wrote: > Sorry, I was unclear. I was asking a question about the reason the > ObjectClass and the object_classes[] array exist in the current code, > it wasn't a direct question about your patch. I did a bit of git spelunking and the reason seems to be

Re: Improve readability by using designated initializers when possible

2024-02-22 Thread Jelte Fennema-Nio
On Thu, Feb 22, 2024, 23:46 Jeff Davis wrote: > > Am I missing something? The main benefits it has are: 1. The order of the array doesn't have to exactly match the order of the enum for the arrays to contain the correct mapping. 2. Typos in the enum variant names are caught by the compiler

Re: When extended query protocol ends?

2024-02-22 Thread Jelte Fennema-Nio
On Thu, 22 Feb 2024 at 13:01, Michael Zhilin wrote: > I would like to say this document states that "at completion... frontend > should issue a Sync message... causes the backend to close the current > transaction" > It looks like the sense of wording is "to complete transaction" at the >

Re: When extended query protocol ends?

2024-02-22 Thread Jelte Fennema-Nio
On Thu, 22 Feb 2024 at 10:28, Vladimir Sitnikov wrote: > > >When splitting a multi insert statement you're going to duplicate some work > > I do not know how this could be made more efficient as I execute parse only > once, and then I send bind+exec+bind+exec > without intermediate sync

Re: When extended query protocol ends?

2024-02-22 Thread Jelte Fennema-Nio
On Wed, 21 Feb 2024 at 17:07, Vladimir Sitnikov wrote: > From many measurements we know that insert into table(id, name) > values(?,?),(?,?),(?,?) is much more efficient than > sending individual bind-exec-bind-exec-bind-exec-sync messages like "insert > into table(id, name) values(?,?)" > For

Re: When extended query protocol ends?

2024-02-21 Thread Jelte Fennema-Nio
On Wed, 21 Feb 2024 at 16:35, Vladimir Sitnikov wrote: > We can't send complete parse-bind-execute commands for every "savepoint" call > as it would hurt performance. Would performance suffer that much? I'd expect the simple and extended protocol to have roughly the same overhead for simple

Improve readability by using designated initializers when possible

2024-02-21 Thread Jelte Fennema-Nio
Usage of designated initializers came up in: https://www.postgresql.org/message-id/flat/ZdWXhAt9Tz4d-lut%40paquier.xyz#9dc17e604e58569ad35643672bf74acc This converts all arrays that I could find that could clearly benefit from this without any other code changes being necessary. There were a few

Re: Add lookup table for replication slot invalidation causes

2024-02-20 Thread Jelte Fennema-Nio
On Tue, 20 Feb 2024 at 12:11, Bharath Rupireddy wrote: > Thoughts? Seems like a good improvement overall. But I'd prefer the definition of the lookup table to use this syntax: const char *const SlotInvalidationCauses[] = { [RS_INVAL_NONE] = "none", [RS_INVAL_WAL_REMOVED] =

Re: Have pg_basebackup write "dbname" in "primary_conninfo"?

2024-02-19 Thread Jelte Fennema-Nio
On Tue, 20 Feb 2024 at 00:34, Ian Lawrence Barwick wrote: > With the addition of "pg_sync_replication_slots()", there is now a use-case > for > including "dbname" in "primary_conninfo" and the docs have changed from > stating [1]: > > Do not specify a database name in the primary_conninfo

Re: Avoid switching between system-user and system-username in the doc

2024-02-19 Thread Jelte Fennema-Nio
On Mon, 19 Feb 2024 at 09:52, Bertrand Drouvot wrote: > Please find attached a tiny patch to clean that up. LGTM

Re: Add trim_trailing_whitespace to editorconfig file

2024-02-19 Thread Jelte Fennema-Nio
e edited by hand) 3. Imported/autogenerated files The first one is definitely the most useful to me personally. From 419d2d0e45d40a4cddb942fbc63c3c54f4dd479d Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 19 Feb 2024 16:03:31 +0100 Subject: [PATCH v4 2/5] Include test outp

Re: Add trim_trailing_whitespace to editorconfig file

2024-02-15 Thread Jelte Fennema-Nio
On Thu, 15 Feb 2024 at 16:57, Peter Eisentraut wrote: > Is there a command-line tool to verify the syntax of .editorconfig and > check compliance of existing files? > > I'm worried that expanding .editorconfig with detailed per-file rules > will lead to a lot of mistakes and blind editing, if we

Re: Add trim_trailing_whitespace to editorconfig file

2024-02-15 Thread Jelte Fennema-Nio
On Wed, 14 Feb 2024 at 23:19, Daniel Gustafsson wrote: > > +1 from me. But when do we want it to be false? That is, why not > > declare it true for all file types? > > Regression test .out files commonly have spaces at the end of the line. (Not > to mention the ECPG .c files but they probably

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-02-14 Thread Jelte Fennema-Nio
On Wed, 14 Feb 2024 at 18:41, Alvaro Herrera wrote: > Hmm, I think the changes to libpq_pipeline in 0005 should be in 0004. Yeah, you're correct. Fixed that now. v32-0005-Start-using-new-libpq-cancel-APIs.patch Description: Binary data

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-02-14 Thread Jelte Fennema-Nio
On Sun, 4 Feb 2024 at 16:39, Alvaro Herrera wrote: > Maybe this is okay? I'll have a look at the whole final situation more > carefully later; or if somebody else wants to share an opinion, please > do so. Attached is a new version of the final patches, with much improved docs (imho) and the

Add trim_trailing_whitespace to editorconfig file

2024-02-14 Thread Jelte Fennema-Nio
This brings our .gitattributes and .editorconfig files more in line. I had the problem that "git add" would complain often about trailing whitespaces when I was changing sgml files specifically. v1-0001-Configure-trailing-whitespace-trimming-in-editorc.patch Description: Binary data

Re: Support a wildcard in backtrace_functions

2024-02-12 Thread Jelte Fennema-Nio
On Mon, 12 Feb 2024 at 14:14, Daniel Gustafsson wrote: > > The main problem it currently has is that it adds backtraces to all > > LOG level logs too. So probably we want to make backtrace_functions > > only log backtraces for ERROR and up (or maybe WARNING/NOTICE and up), > > or add a

Re: Possibility to disable `ALTER SYSTEM`

2024-02-07 Thread Jelte Fennema-Nio
On Wed, 7 Feb 2024 at 11:35, Gabriele Bartolini wrote: > This is mostly the approach I have taken in the patch, except allowing to > change the value in the configuration file. (I had missed the patch in the long thread). I think it would be nice to have this be PGC_SIGHUP, and set

Re: Possibility to disable `ALTER SYSTEM`

2024-02-07 Thread Jelte Fennema-Nio
On Wed, 7 Feb 2024 at 11:16, Peter Eisentraut wrote: > On 06.02.24 16:22, Jelte Fennema-Nio wrote: > > On Tue, 30 Jan 2024 at 18:49, Robert Haas wrote: > >> I also think that using the GUC system to manage itself is a little > >> bit suspect. I wonder if it would be b

Re: Possibility to disable `ALTER SYSTEM`

2024-02-06 Thread Jelte Fennema-Nio
On Tue, 30 Jan 2024 at 18:49, Robert Haas wrote: > I also think that using the GUC system to manage itself is a little > bit suspect. I wonder if it would be better to do this some other way, > e.g. a sentinel file in the data directory. For example, suppose we > refuse ALTER SYSTEM if

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-02-02 Thread Jelte Fennema-Nio
On Fri, 2 Feb 2024 at 16:06, Alvaro Herrera wrote: > Now, looking at this list, I think it's surprising that the nonblocking > request for a cancellation is called PQcancelPoll. PQcancelSend() is at > odds with the asynchronous query API, which uses the verb "send" for the > asynchronous

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

2024-02-02 Thread Jelte Fennema-Nio
rd ("been") in a comment that I noticed while reading your fixes. From 7736e940567878c32355c2143cddba3b13bfa71e Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 26 Jan 2024 16:47:51 +0100 Subject: [PATCH v30 3/5] libpq: Change some static functions to extern This is in preparation of

Re: Call pqPipelineFlush from PQsendFlushRequest

2024-02-01 Thread Jelte Fennema-Nio
On Thu, 1 Feb 2024 at 21:00, Tom Lane wrote: >Note that the request is not itself flushed to the server > automatically; >use PQflush if necessary. > > Doesn't that require some rewording? I agree that the current wording is slightly incorrect, but I think I prefer we keep it

Re: psql not responding to SIGINT upon db reconnection

2024-01-31 Thread Jelte Fennema-Nio
On Wed, 31 Jan 2024 at 19:07, Tristan Partin wrote: > I was looking for documentation of PQsocket(), but didn't find any > standalone (unless I completely missed it). So I just copied how > PQsocket() is documented in PQconnectPoll(). I am happy to document it > separately if you think it would

<    1   2   3   4   5   6   >