Imply NOT_IN_SAMPLE for GucFlags::NO_SHOW_ALL

[Ngalstyan4]

Hi all,

firstly, thanks for giving pgrx to the world!

We use pgrx at Lantern¹ and when configuring GUCs noticed a small issue with GUC flags. Postgres requires GUC_NOT_IN_SAMPLE GUC flag to be set whenever NO_SHOW_ALL is set². Make applying GucFlags::NO_SHOW_ALL also apply NOT_IN_SAMPLE, as NOT_IN_SAMPLE cannot be set.

After applying this diff, we no longer have a direct name matching between pgrx GUC names and postgres ones. An alternative might be to deprecate NO_SHOW_ALL and introduce something like NO_SHOW_ALL_AND_NO_SAMPLE, but not sure if it is worth it.

Footnotes

1. https://github.com/lanterndata/lantern ↩

2. https://github.com/postgres/postgres/blob/dbedc461b4e7a9cb4d6f5777174bdf180edb95fd/src/backend/utils/misc/guc.c#L1506-L1519 ↩

[workingjubilee]

@Ngalstyan4 your "source" link doesn't work at all, it seems.

[Ngalstyan4]

sorry, this is what I was referring to: https://github.com/postgres/postgres/blob/dbedc461b4e7a9cb4d6f5777174bdf180edb95fd/src/backend/utils/misc/guc.c#L1506-L1519

Fixed the link in the description above as well.

[workingjubilee]

Thank you.

...I wonder why they don't just make one flag imply... well, anyway.

[workingjubilee]

This technically prevents independently setting these. Or at least not in an easy and error-free way.

Not sure I or anyone else cares about that.

[Ngalstyan4]

Just to clarify, you mean it prevents independently setting pg_sys::NO_SHOW_ALL, right?

pg_sys::GUC_NOT_IN_SAMPLE is not exposed via a safe wrapper as GucFlags member, but my understanding is that if/when that is exposed, then that and all the other flags can still be set independently.

pg_sys::NO_SHOW_ALL cannot be set independently but that seems to be a postgres invariant (even though the invariant is not enforced in C)

[workingjubilee]

by set independently, I mean if one did this:

let guc_flags = GucFlags::NO_SHOW_ALL;
let guc_flags = guc_flags & !GucFlags::NOT_IN_SAMPLE;

that would be obviously incorrect.
more subtle:

let guc_flags = GucFlags::NOT_IN_SAMPLE;
let guc_flags = guc_flags & !GucFlags::NO_SHOW_ALL;

...I think I'd have to use some function calls on the bitflags API surface instead, since I think it doesn't auto-implement operators, but you get the idea. It does implement a trait that has the equivalent functions as those operators.

[workingjubilee]

Anyways, we will worry this slightly unintuitive behavior if we ever have anyone ask for NOT_IN_SAMPLE.