Re: No longer possible to query catalogs for index capabilities? - Mailing list pgsql-hackers

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>   - this still has everything in amapi.c rather than creating any new
>     files. Also, the regression tests are in create_index.sql for lack
>     of any obviously better place.

This more than doubles the size of amapi.c, so it has a definite feel
of tail-wags-dog for me, even if that seemed like an appropriate place
otherwise which it doesn't really.  I think a new .c file under adt/ is
the way to go, with extern declarations in builtins.h.

Maybe we need a new regression test case file too.  I don't much like
adding this to create_index because (a) it doesn't particularly seem
to match that file's purpose of setting up indexes on the standard
regression test tables, and (b) that file is a bottleneck in parallel
regression runs because it can't run in parallel with much else.

> The list of column properties is:

>   ordered  - (same as "amcanorder" AM capability)
>   ordered_asc
>   ordered_desc
>   ordered_nulls_first
>   ordered_nulls_last

> If "ordered" is true then exactly one of _asc/_desc and exactly one of
> _nulls_first/_last will be true; if "ordered" is false then all the
> others will be false too.

Check, but do we need the "ordered_" prefixes on the last four?  Seems
like mostly useless typing.  And the name space here is never going
to be so large that it'd be confusing.

> Comments?

Why did you go with "capability" rather than "property" in the exposed
function names?  The latter seems much closer to le mot juste, especially
for things like asc/desc.

I'd expect the property keywords to be recognized case-insensitively,
as for example are the keywords in has_table_privilege() and its ilk.
That being the case, you should be using pg_strcasecmp().  This stuff:if (namelen == 10 && memcmp(nameptr,
"amcanorder",10) == 0)
 
is ugly and hard to maintain as well as being unnecessarily non-user-
friendly.  Just convert the input to a C string and be done with it.
It's not like saving a couple of nanoseconds is critical here.

I'd personally cut the list of pg_am replacement properties way back,
as I believe much of what you've got there is not actually of use to
applications, and some of it is outright counterproductive.  An example
is exposing amcanreturn as an index-AM boolean.  That's just wrong these
days.  If you want that, it should be an index column property.  And
because what used to be an index-AM property no longer is in that case,
I think it's a fine example of why we shouldn't be reporting more than
the absolute minimum here.  It would tie our hands with respect to making
other improvements of that kind later.  (This might be a good argument for
moving as much as we can over to the index-column-property function, even
if it can't be set per-column today.  If there's a chance that it could be
per-column in the future, let's not call it an AM property.)

Also, while I see the point of the amgettuple and amgetbitmap properties,
I would call them something like "index_scan" and "bitmap_scan" because
that's the user's-eye view of what is supported.  I do not think we should
slavishly tie the property names to the old, never-intended-as-user-facing
column names.  I'd drop the "am" prefix, for starters.
        regards, tom lane