interchange / interchange6-schema

DBIC schema for Interchange 6
8 stars 7 forks source link

Attributes not unique by name, causes run-time error #111

Closed murwiz closed 9 years ago

murwiz commented 10 years ago

I don't know if this is a code or schema issue.

select * from attributes where name='size'; attributes_id | name | type | title | dynamic | priority ---------------+------+------+-------+---------+---------- 302 | size | | | f | 0 293 | size | | | f | 0

causes the following (note--line numbers are approximate as I'd been messing around in the source to track this down):

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /home/camp/.plenv/versions/5.18.2/lib/perl5/site_perl/5.18.2/Interchange6/Schema/Base/Attribute.pm line 165 at /home/camp/.plenv/versions/5.18.2/lib/perl5/site_perl/5.18.2/Interchange6/Schema/Base/Attribute.pm line 163.

(It's basically this line:

my $attribute = $self->result_source->schema->resultset('Attribute')->find( \%attr );

In my case, the "size" attribute was added a second time by my DB loading script in error. I think attributes should be unique by name, and that should be enforced by a constraint.

racke commented 10 years ago

Attributes could be of different type for example. At any rate, if you use find on a non-unique column this is an error you should cope with (or start with using search).

murwiz commented 10 years ago

Right, but this is in Interchange6::Schema::Base::Attribute::find_attribute_value:

... my $attribute = $self->result_source->schema->resultset('Attribute')->find( \%attr ); ...

so the only control I have would be passing in something that makes the attribute "find" operation unique: the type if I knew it, for instance. However, as noted in my example above, the schema will allow completely identical (except for PK value) attributes.

SysPete commented 9 years ago

Best solution here I think is a unique constraint on name and type discussed on IRC today with racke:

<SysPete> and I've been thinking about possible constraints in Attribute and maybe AttributeValue
<SysPete> ref IC6S #111
<SysPete> for batch loading it would be nice to be able to do find_or_create based on unique attribute name+type
<SysPete> and for me it makes sense if we have that constraint
<SysPete> might also be a good thing to use in product->add_variants so att/att_val can be created as part of add_variants instead of having to pre-declare them
<SysPete> thoughts?
<racke> so unique on type+name ?
<SysPete> yup
<racke> so how would that work on my size example for products
<racke> size (T-Shirts) XS S L ...
<racke> size (Skirts) 48 50 52 ?
<SysPete> I don't see an issue there since PAV points at the appropriate ones
<racke> ok
<racke> in this case I'm fine with it
<SysPete> the only problem would be if you wanted name => foo, title => Foo and also name => foo, title => Banana with type => variant in both cases
<SysPete> but is that realistic?
<SysPete> I'm also going to review all variant methods to make sure they all check/set type correctly
<racke> ok
<racke> can't think of a case right now