rowid_column difficulties

[Lobstros]

Hey,

I'm having a trouble setting rowid_column when implementing a write API, and am trying to work out if it's a bug, or me being slow—any help appreciated!

I see that, in the source of the abstract class ForeignDataWrapper, rowid_column is a @property method that raises NotImplementedError, presumably to enforce that child classes override it. But, this means that when I do try to set it, Python angrily throws AttributeError (I guess it expects a setter method or something). So, whether you try to set it or not, it ends up running to one of the two exceptions.

I've ended up forcing the redefinition by doing this:

class MyFDW(ForeignDataWrapper):

    def __init__():
        <snip>
        self._rowid_column = "colname"

    @property
    def rowid_column:
        return self._rowid_column

It seems to work, but I'm worried that it's heavy-handed magic. Was it intended behaviour?

BTW, I notice the property spelled differently in the docs (row_id_column), but the source only makes reference to rowid_column. That did look like a simple typo, though the tests also seem to use the double-underscore variant too. Do they pass, and I'm just missing something?

[rdunklau]

Yes it is, to avoid having unexpected None in there.

As for the difference between the doc and source code, it is a typo in the doc. I just pushed a fix for that. I understand it is confusing with the tests: the tests use an option called row_id_column to configure which column it is, which is probably where the typo in the docs came from.

Thank you for this report fixing the documentation !

[Lobstros]

Thanks for the info (and the doc fixes!). I see now that it's probably intended to allow dynamic choosing of field names, and that @property is syntactic sugar, rather than hinting it should be replaced with an actual property.

On a related note, it only allows for one column name to be specified, which doesn't hold for data sources with composite keys— I've hacked around it by adding a _composite_key_hash field in table definitions, and populating it with the hash of the tuple of these key fields in Python, but it would be a big win to have natively.

Have you ever thought about this as a feature? I went through the C source to see what implementing it would involve, but it seems it would require a fairly substantial restructure, and sadly my skills aren't quite up to the job yet!