When systable_beginscan() and systable_beginscan_ordered() choose an
index scan, they remap the attribute numbers in the passed-in scan keys
to the attribute numbers of the index, and then write those remapped
attribute numbers back into the scan key passed by the caller. This
second part is surprising and gratuitous. It means that a scan key
cannot safely be used more than once (but it might sometimes work,
depending on circumstances). Also, there is no value in providing these
remapped attribute numbers back to the caller, since they can't do
anything with that.
I propose to fix that by making a copy of the scan keys passed by the
caller and make the modifications there.
In order to prove to myself that there are no other cases where
caller-provided scan keys are modified, I went through and
const-qualified all the APIs. This works out correctly. Several levels
down in the stack, the access methods make their own copy of the scan
keys that they store in their scan descriptors, and they use those in
non-const-clean ways, but that's ok, that's their business. As far as
the top-level callers are concerned, they can rely on their scan keys to
be const after this.
I'm not proposing this second patch for committing at this time, since
that would modify the public access method APIs in an incompatible way.
I've made a proposal of a similar nature in [0]. At some point, it
might be worth batching these and other changes together and make the
change. I might come back to that later.
[0]:
https://www.postgresql.org/message-id/flat/14c31f4a-0347-0805-dce8-93a9072c05a5%40eisentraut.org
While researching how the scan keys get copied around, I noticed that
the index access methods all use memmove() to make the above-mentioned
copy into their own scan descriptor. This is fine, but memmove() is
usually only used when something special is going on that would prevent
memcpy() from working, which is not the case there. So to avoid the
confusion for future readers, I changed those to memcpy(). I suspect
that this code has been copied between the different index AM over time.
(The nbtree version of this code is literally unchanged since July 1996.)
