On 2015-10-05 19:25, Alexander Korotkov wrote:
On Sun, Oct 4, 2015 at 4:27 PM, Amit Kapila <amit.kapil...@gmail.com
On Sat, Oct 3, 2015 at 5:07 PM, Petr Jelinek <p...@2ndquadrant.com
On 2015-10-03 08:27, Amit Kapila wrote:
On Fri, Oct 2, 2015 at 8:14 PM, Alexander Korotkov
> I agree about staying with one SQL-visible function.
Okay, this does not necessarily mean there should be only one
validation function in the C struct though. I wonder if it would
be more future proof to name the C interface as something else
than the current generic amvalidate. Especially considering that
it basically only does opclass validation at the moment (It's
IMHO saner in terms of API evolution to expand the struct with
more validator functions in the future compared to adding
arguments to the existing function).
I also agree with you that adding more arguments in future might
not be a good idea for exposed API. I don't know how much improvement
we can get if we use structure and then keep on adding more members
to it based on future need, but atleast that way it will be less
I think adding multiple validator functions is another option, but that
also doesn't sound like a good way as it can pose difficulty in
understanding the right version of API to be used.
I think the major priority is to keep compatibility. For now, user can
easily define invalid opclass and he will just get the error runtime.
Thus, the opclass validation looks like improvement which is not
strictly needed. We can add new validator functions in the future but
make them not required. Thus, old access method wouldn't loose
compatibility from this.
Yes that was what I was thinking as well. We don't want to break
anything in this patch as it's mainly API change, but we want to have
API that can potentially evolve. I think evolving the API by adding more
interfaces in the *Routine struct so far worked well for the FDW for
example and given that those structs are nodes, the individual pointers
get initialized to NULL automatically so it's easy to add optional
interfaces (like validators) without breaking anything. Besides, it's
not unreasonable to expect that custom AM authors will have to check if
their implementation is compatible with new major version.
But this is also why I don't think it's good idea to call the opclass
validator just "amvalidate" in the IndexAmRoutine struct because it
implies to be the only validate function we'll ever have.
Other than the above gripe and the following spurious change, the patch
seems good to me now.
- Form_pg_am aform;
@@ -1178,6 +1243,7 @@ RelationInitIndexAccessInfo(Relation relation)
+ Form_pg_am aform;
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Sent via pgsql-hackers mailing list (firstname.lastname@example.org)
To make changes to your subscription: