Re: pgbench without dbname worked differently with psql and pg_dump

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, miyake_kouta(at)oss(dot)nttdata(dot)com
Cc: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench without dbname worked differently with psql and pg_dump
Date: 2025-02-14 09:37:09
Message-ID: CAExHW5ub1YDMO9Zeyi_MAjxGOKy5ywj3zrGgQoO+T-nLKrSfmg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 22, 2025 at 7:24 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> > ```
>
> Thanks for the confirmation. I found below executables seem to have the same
> logic to decide the dbname:
>
> - pg_amcheck

The documentation just says "If no such options are present, the
default database will be checked.". But it does not mention what the
default database is. So possibly we need to mention what the default
database is.

> - clusterdb
> - reindexdb
> - vacuumdb
>

I confirm that all these utilities need a documentation update.

Regarding the patch 0002: I would prefer to keep the old construct
i.e. "If that is not set, the current system user name is used as the
database name.". But I think your construct is acceptable too; the app
has to connect to the database before doing anything.

vaccumdb changes need a fix - s/d atabase/database/

Interestingly, all these utilities use the value of PGUSER as the
databasename if it exists. I think the doc change needs to reflect
that as well.

>- pgbench

This is the more interesting part and probably changes the perspective
on everything. Digging up history shows that when
412893b4168819955e9bf428036cd95b0832d657 was committed, we really used
the username (variable name login at that time) as the database name.
The username itself defaulted to PGUSER.
f1516ad7b3a9645a316846fa7b2229163bc55907 changed the code to use only
PGUSER and didn't use username at all. That's where I think the
document and the behaviour went out of sync. We should be fixing the
behaviour change caused by f1516ad7b3a9645a316846fa7b2229163bc55907,
which your 0001 tries to do. However, it should check username before
checking existence of PGUSER. It assumes that doConnect() and its
minions would use PGUSER if username is not specified, which is what
412893b4168819955e9bf428036cd95b0832d657 did as well. So it looks
good. I am adding Michael Pacquire and Kota Miyake to this thread for
their opinions.

This makes me wonder whether the real intention of code in clusterdb,
reindexdb and vacuumdb is to use the connection user name as the
database name as the document suggests. But then the current code is
very old - like 22 years old or something. So we should just fix the
documentation. It's quite possible that all the users just pass -d.
That explains why nobody noticed this descripancy for last 22 odd
years. Or may be we are missing something ... .

--
Best Wishes,
Ashutosh Bapat

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Borisov 2025-02-14 09:45:44 Re: Get rid of WALBufMappingLock
Previous Message Álvaro Herrera 2025-02-14 09:36:48 Re: pg_stat_statements and "IN" conditions