public inbox for [email protected]  

help / color / mirror / Atom feed

From: Ashutosh Bapat <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Chao Li <[email protected]>
Cc: L. pgsql-hackers <[email protected]>
Subject: Re: Property graph: fix error handling when dropping non-existent label property
Date: Wed, 29 Apr 2026 17:40:32 +0530
Message-ID: <CAExHW5so9k_8RsAs00-rTY+4x9rtuO6cmwy8RG6jZx1PZGVHqQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAExHW5s1q7eyp4qJLPzCgnsE9tQ8uKdLyOwKt3cijdMR_0E+Cg@mail.gmail.com>
	<[email protected]>
	<CAExHW5s-4MHjo0wn=bNGQQd=Q+3SW_HGxJCFnoBF+hiy5ni8wA@mail.gmail.com>
	<[email protected]>
	<[email protected]>

On Wed, Apr 29, 2026 at 3:22 PM Álvaro Herrera <[email protected]> wrote:
>
> On 2026-04-29, Chao Li wrote:
>
> >> Looks good to me. However, I did change OidIsValid() and !OidIsValid()
> >> back to (oid) and (!oid) conditions to be consistent with the rest of
> >> the code.
> >
> > In the file, I also see:
> > ```
> >     if (pgrelid == InvalidOid)
> > ```
> >
> > Should we take this opportunity to change to use OidIsValid()
> > everywhere in the file? As this feature is new to PG19, we can cleanup
> > the inconsistency before releasing v19. Otherwise some people might
> > also file a cleanup patch for this in the future.
>
> Yeah, I find "if (oid)" a rather terrible coding pattern. The negative one is perhaps not so bad, but I'd keep both cases similar by using the macro in both, for consistency.
>

I am in favour of doing this change, but let's do it in a separate patch.

-- 
Best Wishes,
Ashutosh Bapat

view thread (7+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Property graph: fix error handling when dropping non-existent label property
  In-Reply-To: <CAExHW5so9k_8RsAs00-rTY+4x9rtuO6cmwy8RG6jZx1PZGVHqQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox