On Fri, Jan 22, 2021 at 3:22 PM Corey Huinker <corey.huin...@gmail.com> wrote: >> I decided not to deviate from pk_ terminology so that the new code >> doesn't look too different from the other code in the file. Although, >> I guess we can at least call the main function >> ri_ReferencedKeyExists() instead of ri_PrimaryKeyExists(), so I've >> changed that. > > I think that's a nice compromise, it makes the reader aware of the concept. >> >> I've attached the updated patch. > > Missing "break" added. Check. > Comment updated. Check. > Function renamed. Check. > Attribute mapping matching test (and assertion) added. Check. > Patch applies to an as-of-today master, passes make check and check world. > No additional regression tests required, as no new functionality is > introduced. > No docs required, as there is nothing user-facing.
Thanks for the review. > Questions: > 1. There's a palloc for mapped_partkey_attnums, which is never freed, is the > prevailing memory context short lived enough that we don't care? > 2. Same question for the AtrrMap map, should there be a free_attrmap(). I hadn't checked, but yes, the prevailing context is AfterTriggerTupleContext, a short-lived one that is reset for every trigger event tuple. I'm still inclined to explicitly free those objects, so changed like that. While at it, I also changed mapped_partkey_attnums to root_partattrs for readability. Attached v4. -- Amit Langote EDB: http://www.enterprisedb.com
v4-0001-Export-get_partition_for_tuple.patch
Description: Binary data
v4-0002-Avoid-using-SPI-for-some-RI-checks.patch
Description: Binary data
