> Thank you for a review Nitin, here is a v2 version of the patch with extended > test coverage.
Thanks Alexander, v2 addresses my earlier comment. The added src2 casts now cover the case where the source type's schema is the distinguishing part of the cast key, so both the castsource and casttarget comparisons are exercised. I noticed one small test-completeness nit: the test creates four casts, but I see regex checks for only three emitted CREATE CAST statements. The unchecked one appears to be: CREATE CAST (public.dump_cast_src2 AS public.dump_cast_tgt) WITH INOUT; Since this is the public-side member of the new source-type ambiguity pair, would it be worth adding a matching regexp for symmetry/completeness? A couple of minor readability thoughts: Now that the test covers two separate cases, target-side and source-side ambiguity, perhaps the test type names could make that distinction explicit. For example, instead of using dump_cast_src2, names along these lines might make the intent clearer: public.dump_cast_src_for_target_test, public.dump_cast_src_for_source_test,dump_cast_schema.dump_cast_src_for_source_test. That would make it easier to see which casts exercise the target-type tie-breaker and which casts exercise the source-type tie-breaker. The DO_TRANSFORM comment says "Same unqualified-typname ambiguity as casts; break by type." That is understandable, but maybe it could mention why comparing only trftype is enough here: the language name has already been compared as part of dobj.name, so trftype is the remaining natural-key field that can distinguish the two transform objects. These are minor; the code change itself looks reasonable to me. Best Regards, Nitin Jadhav Azure Database for PostgreSQL Microsoft
