Horton

Re: Schema variables - new implementation for Postgres 15

Julien Rouhaud <rjuju123@gmail.com>

View thread
From: Julien Rouhaud <rjuju123@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>, dean.a.rasheed@gmail.com, er@xs4all.nl, joel@compiler.org, pgsql-hackers@lists.postgresql.org
Date: 2022-11-05T17:25:09Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow underscores in integer and numeric constants.

  2. Remove special outfuncs/readfuncs handling of RangeVar.catalogname.

  3. Remove extra space from dumped ALTER DEFAULT PRIVILEGES.

  4. Create FKs properly when attaching table as partition

  5. psql: improve tab-complete's handling of variant SQL names.

Hi,

On Sat, Nov 05, 2022 at 05:04:31PM +0100, Tomas Vondra wrote:
>
> I did a quick initial review of this patch series - attached is a
> version with "review" commits for some of the parts. The current patch
> seems in pretty good shape, most of what I noticed are minor issues. I
> plan to do a more thorough review later.

Thanks!

I agree with all of your comments, just a few answers below

> - NamesFromList and IdentifyVariable seem introduced unnecessarily
> early, as they are only used in 0002 and 0003 parts (in the original
> patch series). Not sure if the plan is to squash everything into a
> single patch, or commit individual patches.

The split was mostly done to make the patch easier to review, as it adds quite
a bit of infrastructure.

There have been some previous comments to have a more logical separation and
fix similar issues, but there are still probably other oddities like that
laying around.  I personally didn't focus much on it as I don't know if the
future committer will choose to squash everything or not.

> - AFAIK patches don't need to modify typedefs.list.

I think this was discussed a year or so ago, and my understanding is that the
general rule is that it's now welcome, if not recommended, to maintain
typedefs.list in each patchset.

> Which I think means this:
>
>     if (filter_lxid && svar->drop_lxid == MyProc->lxid)
>         continue;
>
> accesses drop_lxid, which was not initialized in init_session_variable.

Agreed.