Validator methods are not called for synonyms and inherited columns

Issue #3048 duplicate
Dariusz Górecki created an issue

Consider following example of joined-table inheritance:

class Parent(DeclarativeBase):
    # ...
    parent_column = Column(String(255))

class Child(Parent):
    # ...
    child_column = synonym('parent_column')

    @validates('child_column')  # @validates('parent_column') also does not work
    def validator(self, name, value):
        # not called at all
        raise ValueError()

The validator method is never called

Comments (13)

  1. Mike Bayer repo owner

    validate against "parent_column", not the synonym. the patch in 2943 breaks backwards compatibility for apps that may rely upon a validator silently failing so this is targeted at 1.0 for the moment.

  2. Mike Bayer repo owner

    also you can see there is some decision to be made if both validators should run, or only one. I really have no idea what ppl would prefer.

  3. Dariusz Górecki reporter

    @zzzeek sorry for dup; IMO I think that both validators should run, and probably validator for synonyms also should be possible, this could form a nice validation chain, from specialist models through base ones. Maybe a default-false mapper attribute to turn on "new behaviour"?

  4. Mike Bayer repo owner

    if only the "sub" validator runs, and the "super" runs by calling it explicitly, then the user can control it.

  5. Dariusz Górecki reporter

    The most specific one should run first, and still I think every one should run, without exceptions, object should be valid from any object perspective, base one, and "child" one.

  6. Mike Bayer repo owner

    what if I want to control the order in which they run? wouldn't using super() be more pythonic?

  7. Mike Bayer repo owner

    e.g.

    class B(A):
        @validates('data', override=True)
        def validate(self, key, value):
            return "yup B " + super(B, self).validate(key, value)
    
  8. Mike Bayer repo owner

    synonyms is a whole can of worms. does the synonym-bound validator run only if one accesses the attribute via the synonym name, or by either it's syn name + real name? how about for chained scenarios? this is very complicated to implement as synonyms aren't resolved until late. for now im just going to have that raise.

  9. Dariusz Górecki reporter

    With big power ('configurability') comes big responsibility, IMO this should be kept simple, like a convention: From base model point of view any sub-model provided value should be valid input (go through base model validation nicely), child classes should only make more specific validations. I think we should not over-engineer that.

    What if base model validation assumes that string column should be 'numeric' would you allow child model to pass non numeric input, and in reality make base model "invalid" from its point of view? This sounds wrong from the first place for me.

    But this is only my opinion, others may disagree.

  10. Mike Bayer repo owner

    on #2943 I've proposed 'override' can be set to 'add' or 'replace'. this isn't an excessive amount of engineering, it's just another line of code, and allows both versions to take place thus releasing me of being hit with a use case we aren't thinking of at the moment.

  11. Dariusz Górecki reporter

    If it's not a hard to implement then it's ok. I'll try to design some unit tests for yours patch, but I'm not familiar with sqla code, so it may take a while. Now good night, it's almost 3am here in London ;)

  12. Log in to comment