Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Tom Lane wrote: >> I'm kind of inclined to just let the verifiers read the catalogs for >> themselves. AFAICS, a loop around the results of SearchSysCacheList >> is not going to be significantly more code than what this patch does, >> and it avoids presuming that we know what the verifiers will wish to >> look at.
> Hmm, so this amounts to saying the verifier can only run after the > catalog rows are written. Is that okay?
Why not? Surely we are not interested in performance-optimizing for the case of a detectably incorrect CREATE OP CLASS command.
I don't actually care that much about running the verifiers during CREATE/etc OP CLASS at all. It's at least as important to be able to run them across the built-in opclasses, considering all the chances for human error in constructing those things. Even in the ALTER OP CLASS case, the verifier would likely need to look at existing catalog rows as well as the new ones. So basically, I see zero value in exposing CREATE/ ALTER OP CLASS's internal working representation to the verifiers.
I'm OK with validating opclass directly by system catalog, i.e. looping over SearchSysCacheList results. Teodor was telling me something similar personally.
I'll also try to rearrange header according to the comments upthread.
The next revision of patch is attached.
The changes are following:
Definitions of Selectivity, Cost and declarations of IndexAmRoutine, PlannerInfo, IndexPath, IndexInfo are moved into separate header reldecls.h. That allows to get rid of #include "nodes/relation.h" in amapi.h.
amvalidate method now gets opclass oid as parameter. Internally, amvalidate implementations do catalog lookups. opfam_internal.h isn't included from inappropriate places anymore.
I removed amvalidate calls from opclasscmds.c. Validating user-defined opclasses is behavior change which can have issues with backward compatibility. I think if we want to introduce this, it should be considered separately of API rework.
