Re: proposal - psql - use pager for \watch command

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: proposal - psql - use pager for \watch command
Date: 2021-03-23 04:25:57
Message-ID: CAFj8pRCSjkCD+Lq-1Hk69x=kPKnPqT1VJ+3x+F9yEe+ieHorKA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

po 22. 3. 2021 v 22:07 odesílatel Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
napsal:

> On Tue, Mar 23, 2021 at 1:53 AM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > po 22. 3. 2021 v 13:13 odesílatel Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
> napsal:
> >> The problem is that Apple's /dev/tty device is defective, and doesn't
> >> work in poll(). It always returns immediately with revents=POLLNVAL,
> >> but pspg assumes that data is ready and tries to read the keyboard and
> >> then blocks until I press a key. This seems to fix it:
> >>
> >> +#ifndef __APPLE__
> >> + /* macOS can't use poll() on /dev/tty */
> >> state.tty = fopen("/dev/tty", "r+");
> >> +#endif
> >> if (!state.tty)
> >> state.tty = fopen(ttyname(fileno(stdout)), "r");
> >
> >
> > it is hell.
>
> Heh. I've recently spent many, many hours trying to make AIO work on
> macOS, and nothing surprises me anymore. BTW I found something from
> years ago on the 'net that fits with my observation about /dev/tty:
>
> https://www.mail-archive.com/bug-gnulib(at)gnu(dot)org/msg00296.html
>
> Curious, which other OS did you put that fallback case in for? I'm a
> little confused about why it works, so I'm not sure if it's the best
> possible change, but I'm not planning to dig any further now, too many
> patches, not enough time :-)
>

Unfortunately, I have no exact evidence. My original implementation was
very primitive

if (freopen("/dev/tty", "rw", stdin) == NULL)
{
fprintf(stderr, "cannot to reopen stdin: %s\n", strerror(errno));
exit(1);
}

Some people reported problems, but I don't know if these issues was related
to tty or to freopen

In some discussion I found a workaround with reusing stdout and stderr -
and then this works well, but I have no feedback about these fallback
cases. And because this strategy is used by "less" pager too, I expect so
this is a common and widely used workaround.

I remember so there was a problems with cygwin and some unix platforms (but
maybe very old) there was problem in deeper nesting - some like

screen -> psql -> pspg.

Directly pspg was working, but it didn't work from psql. Probably somewhere
the implementation of pty was not fully correct.

>
> > Please, can you verify this fix?
>
> It works perfectly for me on a macOS 11.2 system with that change,
> repainting the screen exactly when it should. I'm happy about that
> because (1) it means I can confirm that the proposed change to psql is
> working correctly on the 3 Unixes I have access to, and (2) I am sure
> that a lot of Mac users will appreciate being able to use super-duper
> \watch mode when this ships (a high percentage of PostgreSQL users I
> know use a Mac as their client machine).
>

Thank you for verification. I fixed it in master branch

> >> A minor problem is that on macOS, _GNU_SOURCE doesn't seem to imply
> >> NCURSES_WIDECHAR, so I suspect Unicode will be broken unless you
> >> manually add -DNCURSES_WIDECHAR=1, though I didn't check.
> >
> > It is possible -
> >
> > can you run "pspg --version"
>
> Looks like I misunderstood: it is showing "with wide char support",
> it's just that the "num" is 0 rather than 1. I'm not planning to
> investigate that any further now, but I checked that it can show the
> output of SELECT 'špeĉiäl chârãçtérs' correctly.
>

It is the job of ncursesw - pspg sends data to ncurses in original format
- it does only some game with attributes.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-03-23 04:31:31 Re: a verbose option for autovacuum
Previous Message Mahendra Singh Thalor 2021-03-23 03:51:44 Re: UPDATE ... SET (single_column) = row_constructor is a bit broken from V10 906bfcad7ba7c