Re: Non-text mode for pg_dumpall

On Fri, 24 Jan 2025 at 20:50, jian he <jian.universality@gmail.com> wrote:
>
> On Thu, Jan 23, 2025 at 6:35 PM Mahendra Singh Thalor
> <mahi6run@gmail.com> wrote:
>
> hi.
> After some tests and thinking about your reply, I admit that using
> expand_dbname_patterns
> in pg_restore will not work.
> We need to do pattern matching against the map.dat file.
> Please check the attached v12 series based on your
> v11_pg_dumpall-with-directory-tar-custom-format-21-jan.patch
>
> v12-0001 cosmetic change.
> v12-0002 implement pg_resore --exclude-database=PATTERN.
> main gist of implementation:
> for each database name in map.dat file,
> check if this database name pattern matches with PATTERN or not.
> pattern matching is using processSQLNamePattern.
>
> your substring will not work.
> some of the test cases.
> $BIN10/pg_restore --exclude-database=* -Cd template1 --verbose dir10 >
> dir_format 2>&1
> $BIN10/pg_restore --exclude-database=*x* -Cd template1 --verbose dir10
> > dir_format 2>&1
> $BIN10/pg_restore --exclude-database=?* -Cd template1 --verbose dir10
> > dir_format 2>&1

I merged v12_0001 into the latest patch. There was one bug in v12_001*
which was fixed in v12_0002*.

-extern PGconn **connectDatabase*(const char *dbname,
> - const char *connection_string, const char *pghost,
> - const char *pgport, const char *pguser,
> - trivalue prompt_password, bool fail_on_error,
> - const char *progname, const char **connstr, int *server_version);
> +extern PGconn *v(const char *dbname, const char *connection_string, const
> char *pghost,


As per v12_0002*, I made some changes into the current patch to avoid using
the substring function.

On Tue, 28 Jan 2025 at 11:57, Mahendra Singh Thalor <mahi6run@gmail.com>
wrote:
>
> On Tue, 28 Jan 2025 at 10:19, Srinath Reddy <srinath2133@gmail.com> wrote:
> >
> >
> > Hi mahendra,
> >
> > I have reviewed the code in the v11 patch and it looks good to me.
> >
> > But in common_dumpall_restore.c there's  parseDumpFormat which is
common between pg_dumpall and pg_restore ,as per the discussion in [1]
thread i don't think we should create a common api ,as discussed in the
thread there might chances in the future we might decide that some format
is obsolete and desupport it in pg_dumpall ,while support in pg_restore for
compatibility reasons.

Fixed. In the latest patch, I removed the parseDumpFormat function.
In older versions, I was using the same function for pg_dumpall and
pg_restore but now some common code is already committed from this patch
and as per discussion, we will keep separate handling for parsing so adding
parseDumpFormat function only in pg_dumpall.c file.

On Sun, 26 Jan 2025 at 20:17, jian he <jian.universality@gmail.com> wrote:
>
> hi.
> attached patching trying to refactor ReadOneStatement
> for properly handling the single and double quotes.
> the commit message also has some tests on it.
>
> it is based on your
> v11_pg_dumpall-with-directory-tar-custom-format-21-jan.patch.

Okay. I am doing some more testing and code review for this type of test
cases. I will merge this delta into the next version.

> >
>
> Oaky. Thanks for review. I will make changes as per discussion in
> another thread.
>
>
> On Tue, 28 Jan 2025 at 11:52, Srinath Reddy <srinath2133@gmail.com> wrote:
> >
> > make check-world fails,i think we don't need $port and $filename
instead we can use something like 'xxx'.so fixed it in the below patch.
>
> In offline discussion, Andew already reported this test case. I will
> fix this in the next version.
>
Fixed.

Thanks Jian and Srinath for the testing and review.

Here, I am attaching an updated patch for review and testing.

I merged some of the delta patches that are shared by Jian and did some
fixes also.

-- 
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com