Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-22 Thread Michael Paquier
On Thu, Mar 16, 2023 at 01:17:59PM +0530, Bharath Rupireddy wrote: > FWIW, I rebased the tests tweaking patch and attached it here as v9. > This should keep the pg_walinspect tests consistent across comments, > spaces, new lines and using count(*) >= 1 for all successful function > executions.

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-16 Thread Bharath Rupireddy
On Thu, Mar 16, 2023 at 12:48 PM Michael Paquier wrote: > > On Wed, Mar 15, 2023 at 06:50:01PM +0900, Michael Paquier wrote: > > This is a duplicate of what describe.c uses, with a COLLATE clause. > > The main goal was to have a simple check, so I'd still stand by the > > simplest choice and move

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-16 Thread Michael Paquier
On Wed, Mar 15, 2023 at 06:50:01PM +0900, Michael Paquier wrote: > This is a duplicate of what describe.c uses, with a COLLATE clause. > The main goal was to have a simple check, so I'd still stand by the > simplest choice and move on. Please note that I have done something about that with

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-15 Thread Michael Paquier
On Wed, Mar 15, 2023 at 12:40:17PM +0530, Bharath Rupireddy wrote: > -1 for removing \dx+ for pg_walinspect version 1.0, because we wanted > to show the diff of functions along with testing the upgrade path in > the oldextversions.sql. Therefore, I prefer something like [1]: This is a duplicate

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-15 Thread Bharath Rupireddy
On Wed, Mar 15, 2023 at 12:27 PM Michael Paquier wrote: > > On Tue, Mar 14, 2023 at 07:05:20PM -0700, Andres Freund wrote: > > It's using ICU, but not a specific collation. The build I linked to is WIP > > hackery to add ICU support to windows CI. Here's the initdb output: > >

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-15 Thread Michael Paquier
On Tue, Mar 14, 2023 at 07:05:20PM -0700, Andres Freund wrote: > It's using ICU, but not a specific collation. The build I linked to is WIP > hackery to add ICU support to windows CI. Here's the initdb output: >

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-14 Thread Andres Freund
Hi, On 2023-03-15 09:56:10 +0900, Michael Paquier wrote: > On Tue, Mar 14, 2023 at 02:54:40PM -0700, Andres Freund wrote: > > Object description > > --- > > function pg_get_wal_record_info(pg_lsn) > > - function

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-14 Thread Michael Paquier
On Tue, Mar 14, 2023 at 10:35:43AM +0530, Bharath Rupireddy wrote: > My thoughts are simple here - how would one (an end user, not me and > not you) figure out how to get info/stats till the end of WAL? I'm > sure it would be difficult to find that out without looking at the > code or commit

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-14 Thread Michael Paquier
On Tue, Mar 14, 2023 at 02:54:40PM -0700, Andres Freund wrote: > Object description > --- > function pg_get_wal_record_info(pg_lsn) > - function pg_get_wal_records_info(pg_lsn,pg_lsn) > function

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-14 Thread Andres Freund
Hi, I just rebased a patch over commit 1f282c24e46 Author: Michael Paquier Date: 2023-03-13 13:03:29 +0900 Refactor and improve tests of pg_walinspect and got a test failure: https://cirrus-ci.com/task/5693041982308352

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-13 Thread Bharath Rupireddy
On Tue, Mar 14, 2023 at 5:02 AM Michael Paquier wrote: > > So let's be clean and drop these slots to keep the tests > self-contained. pg_walinspect in REL_15_STABLE gets that right, IMV, > and that's no different from the role cleanup, as one example. Hm, added replication slot drop back. > >

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-13 Thread Michael Paquier
On Mon, Mar 13, 2023 at 03:53:37PM +0530, Bharath Rupireddy wrote: > On Mon, Mar 13, 2023 at 12:26 PM Michael Paquier wrote: >> +-- Make sure checkpoints don't interfere with the test. >> +SELECT 'init' FROM >> pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, >> false);

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-13 Thread Bharath Rupireddy
On Mon, Mar 13, 2023 at 12:26 PM Michael Paquier wrote: > > On Fri, Mar 10, 2023 at 04:45:06PM +0530, Bharath Rupireddy wrote: > > After that comes the rest of the patch, and I have found a couple of > mistakes. > > - pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn) > +

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-13 Thread Michael Paquier
On Fri, Mar 10, 2023 at 04:45:06PM +0530, Bharath Rupireddy wrote: > Any comments on the attached v5 patch? I have reviewed the patch, and found it pretty messy. The tests should have been divided into their own patch, I think. This is rather straight-forward once the six functions have their

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-10 Thread Bharath Rupireddy
On Fri, Mar 10, 2023 at 9:54 AM Michael Paquier wrote: > > Hmm. I think this patch ought to have a result simpler than what's > proposed here. > > First, do we really have to begin marking the functions as non-STRICT > to abide with the treatment of NULL as a special value? The part that > I've

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-10 Thread Michael Paquier
On Fri, Mar 10, 2023 at 04:37:23PM +0800, Julien Rouhaud wrote: > isn't '0/0' the same as InvalidXLogRecPtr? but my point is that we > shouldn't require to spell it explicitly, just rely on the default value. Perhaps. Still the addition of a DEFAULT to the function definitions and its value

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-10 Thread Julien Rouhaud
On Fri, 10 Mar 2023, 16:14 Michael Paquier, wrote: > On Fri, Mar 10, 2023 at 04:04:15PM +0800, Julien Rouhaud wrote: > > As long as we provide a sensible default value (so I guess '0/0' to > > mean "no upper bound") and that we therefore don't have to manually > > specify an upper bound if we

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-10 Thread Michael Paquier
On Fri, Mar 10, 2023 at 04:04:15PM +0800, Julien Rouhaud wrote: > As long as we provide a sensible default value (so I guess '0/0' to > mean "no upper bound") and that we therefore don't have to manually > specify an upper bound if we don't want one I'm fine with keeping the > functions marked as

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-10 Thread Julien Rouhaud
On Fri, Mar 10, 2023 at 12:24 PM Michael Paquier wrote: > > On Wed, Mar 08, 2023 at 01:40:46PM +0530, Bharath Rupireddy wrote: > > 1. When start_lsn is NULL or invalid ('0/0'), emit an error. There was > > a comment on the functions automatically determining start_lsn to be > > the oldest WAL

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-09 Thread Bharath Rupireddy
On Wed, Mar 8, 2023 at 1:40 PM Bharath Rupireddy wrote: > > On Tue, Mar 7, 2023 at 11:17 AM Julien Rouhaud wrote: > > Here I'm with the v3 patch addressing the above comments. Please > review it further. Needed a rebase. v4 patch is attached. I'll address the latest review comments in a bit.

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-09 Thread Michael Paquier
On Wed, Mar 08, 2023 at 01:40:46PM +0530, Bharath Rupireddy wrote: > 1. When start_lsn is NULL or invalid ('0/0'), emit an error. There was > a comment on the functions automatically determining start_lsn to be > the oldest WAL LSN. I'm not implementing this change now, as it > requires extra work

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-09 Thread Michael Paquier
On Tue, Mar 07, 2023 at 01:47:01PM +0800, Julien Rouhaud wrote: > So keep this "deprecated" C function working, as it would only be a few lines > of code? Yes, I guess that this would be the final picture, moving forward I'd like to think that we should just remove the SQL declaration of the

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-08 Thread Bharath Rupireddy
On Tue, Mar 7, 2023 at 11:17 AM Julien Rouhaud wrote: > > On Tue, Mar 07, 2023 at 01:56:24PM +0900, Michael Paquier wrote: > > On Tue, Mar 07, 2023 at 12:42:20PM +0800, Julien Rouhaud wrote: > > > ah right I should have checked. but the same ABI compatibility concern > > > still exists for

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Julien Rouhaud
On Tue, Mar 07, 2023 at 01:56:24PM +0900, Michael Paquier wrote: > On Tue, Mar 07, 2023 at 12:42:20PM +0800, Julien Rouhaud wrote: > > ah right I should have checked. but the same ABI compatibility concern > > still exists for version 1.0 of the extension. > > Yes, we'd better make sure that the

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Michael Paquier
On Tue, Mar 07, 2023 at 12:42:20PM +0800, Julien Rouhaud wrote: > ah right I should have checked. but the same ABI compatibility concern > still exists for version 1.0 of the extension. Yes, we'd better make sure that the past code is able to run, at least. Now I am not really convinced that we

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Julien Rouhaud
On Tue, 7 Mar 2023, 12:36 Michael Paquier, wrote: > On Tue, Mar 07, 2023 at 09:13:46AM +0800, Julien Rouhaud wrote: > > It's problematic to install the extension if we rely on upgrade scripts > only. > > We could also provide a pg_walinspect--1.2.sql file and it would just > work, and > > that

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Michael Paquier
On Tue, Mar 07, 2023 at 09:13:46AM +0800, Julien Rouhaud wrote: > It's problematic to install the extension if we rely on upgrade scripts only. > We could also provide a pg_walinspect--1.2.sql file and it would just work, > and > that may have been a good idea if there wasn't also the problem of

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Julien Rouhaud
On Mon, Mar 06, 2023 at 08:36:17PM +0530, Bharath Rupireddy wrote: > On Mon, Mar 6, 2023 at 2:22 PM Julien Rouhaud wrote: > > > Also: > > > > /* > > - * Get info and data of all WAL records from start LSN till end of WAL. > > + * NB: This function does nothing and stays here for backward > >

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Matthias van de Meent
On Mon, 6 Mar 2023 at 16:37, Bharath Rupireddy wrote: > > On Mon, Mar 6, 2023 at 8:52 PM Matthias van de Meent > wrote: > > > > On Mon, 6 Mar 2023 at 16:06, Bharath Rupireddy > > wrote: > > > If we try to make these functions figure out the oldest WAl file and > > > start from there, then it'll

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Bharath Rupireddy
On Mon, Mar 6, 2023 at 8:52 PM Matthias van de Meent wrote: > > On Mon, 6 Mar 2023 at 16:06, Bharath Rupireddy > wrote: > > If we try to make these functions figure out the oldest WAl file and > > start from there, then it'll unnecessarily complicate the APIs and > > functions. If we still think

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Matthias van de Meent
On Mon, 6 Mar 2023 at 16:06, Bharath Rupireddy wrote: > If we try to make these functions figure out the oldest WAl file and > start from there, then it'll unnecessarily complicate the APIs and > functions. If we still think we need a better function for the users > to figure out the oldest WAL

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Bharath Rupireddy
On Mon, Mar 6, 2023 at 2:22 PM Julien Rouhaud wrote: > > > > I'm attaching a patch doing the $subject with the following behavior: > > > 1. If start_lsn is NULL, error out/return NULL. > > Maybe naive and unrelated question, but is that really helpful? If for some > reason I want to see

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-06 Thread Julien Rouhaud
On Wed, Mar 01, 2023 at 08:30:00PM +0530, Bharath Rupireddy wrote: > On Wed, Mar 1, 2023 at 1:00 PM Bharath Rupireddy > wrote: > > > > In a recent discussion [1], Michael Paquier asked if we can combine > > pg_walinspect till_end_of_wal functions with other functions > > pg_get_wal_records_info

Re: Combine pg_walinspect till_end_of_wal functions with others

2023-03-01 Thread Bharath Rupireddy
On Wed, Mar 1, 2023 at 1:00 PM Bharath Rupireddy wrote: > > Hi, > > In a recent discussion [1], Michael Paquier asked if we can combine > pg_walinspect till_end_of_wal functions with other functions > pg_get_wal_records_info and pg_get_wal_stats. The code currently looks > much duplicated and the