Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Tom Lane wrote:
> >> ... btw, what is the point of catalog/opfam_internal.h? I see you added
> >> it in b488c580aef4e05f, but it seems quite useless to have split it out
> >> as a separate header, since only commands/opclasscmds.c uses it.
>
> > Oh, that slipped in there, didn't it. The JSON DDL-deparse bits need
> > it -- last posted by Alex Shulgin here:
> > https://www.postgresql.org/message-id/CACACo5Q_UXYwF117LBhjZ3xaMPyrgqnqE%3DmXvRhEfjJ51aCfwQ%40mail.gmail.com
>
> I still don't see any reference to OpFamilyMember in there?
Sorry, that posting doesn't have the part that generates the JSON. It's
ddl_deparse.c in the .tar.gz earlier in that thread:
http://www.postgresql.org/message-id/CACACo5QQuAV+n4Gi+YA1JF_a+QenR6SJuP8CYdPSrXKa+FHS3A@mail.gmail.com
> > I suppose it shouldn't have been committed, and be part of the extension
> > instead.
>
> Previously, OpFamilyMember was just a transient internal data structure
> inside commands/opclasscmds.c. Unless we intended that some chunks of
> the code in there be exposed for use by extensions, it's not terribly
> clear to me why extensions would need to access this struct. Perhaps
> we ought to just revert struct OpFamilyMember back into opclasscmds.c.
The whole point of splitting the struct declaration to a new header was
to get a DDL deparser to examine the list of objects being created, so
that it could construct the deparsed command. If the struct is opaque
to the outside world, there's no way to do that (as I recall, you can't
construct the full command starting from the parsed version only -- you
need access to the OIDs of the ops/procs actually added to the opclass.)
> Regardless of that, I'm a bit skeptical that any of these structs ought
> to be defined as part of the amapi.h interface. They seem to be making
> premature judgments as to what an opclass verifier will care about. In
> any case, tying the opclass verifier API to DDL-command implementation
> details doesn't seem like a good plan.
That's a different argument, with which I don't necessarily disagree.
I think it's not unlikely that a verifier will want to examine the
contents of the struct, but I don't think that means we need to expose
the struct in amapi.h.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
