Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

to-one association is generated when 'UNIQUE' set on a composite key(with multiple columns) #77

Closed
yangyuqian opened this issue Dec 15, 2016 · 9 comments

Comments

@yangyuqian
Copy link

Hi,

It seems a bug to create to-one association when a composite key is unique,

i.e.

When the composite key in user_role(user_id, role_id) is UNIQUE, a to-one association will be generated on both User and UserRole

user(id)
user_role(id, user_id, role_id)
role(id)

And I can fix this by omit the UNIQUE composite keys and still generate to-many associations for them(line number 134):

// bdb/drivers/mysql.go
120 func (m *MySQLDriver) Columns(schema, tableName string) ([]bdb.Column, error) {
121     var columns []bdb.Column
122
123     rows, err := m.dbConn.Query(`
124     select
125     c.column_name,
126     if(c.data_type = 'enum', c.column_type, c.data_type),
127     if(extra = 'auto_increment','auto_increment', c.column_default),
128     c.is_nullable = 'YES',
129         exists (
130             select c.column_name
131             from information_schema.table_constraints tc
132             inner join information_schema.key_column_usage kcu
133                 on tc.constraint_name = kcu.constraint_name and tc.table_name = kcu.table_name and tc.table_schema = kcu.table_schema
+ 134                      and kcu.ORDINAL_POSITION = 1 and kcu.constraint_name = CONCAT('fk_', kcu.table_name, '_', kcu.column_name)
135             where c.column_name = kcu.column_name and tc.table_name = c.table_name and
136                 (tc.constraint_type = 'PRIMARY KEY' or tc.constraint_type = 'UNIQUE')
137         ) as is_unique
138     from information_schema.columns as c
139     where table_name = ? and table_schema = ?;
140     `, tableName, schema)

Do you think this is a feasible fix? I'd like to submit a PR for it

@aarondl
Copy link
Member

aarondl commented Dec 20, 2016

I'm actually a bit confused about what's going on here because I don't know what the indexes/constraints/keys are on your user_role table.

Is it possible to do a:

show create table user;
show create table user_role;
show create table role;

This would help me a lot.

@aarondl
Copy link
Member

aarondl commented Dec 20, 2016

I had to edit my above comment, it was incomplete. Also: If you don't want to share your table structure, omit any of the columns that aren't necessary (everything except primary and foreign keys).

@yangyuqian
Copy link
Author

yangyuqian commented Dec 24, 2016

Hi, @aarondl

Thanks for asking, and sorry for the late reponse, I just made an example with 3 tables:

In t2, there is a unique key combined with t1_id and t3_id, and I expect a to-many relationship between t1 and t2,

CREATE TABLE `t1` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT COMMENT 't1.id',
  `name` varchar(255) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=1000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

CREATE TABLE `t2` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `t1_id` bigint(20) NOT NULL,
  `t3_id` bigint(20) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `t1_id` (`t1_id`,`t3_id`),
  KEY `fk_t2_t3_id` (`t3_id`),
  CONSTRAINT `fk_t2_t1_id` FOREIGN KEY (`t1_id`) REFERENCES `t1` (`id`) ON UPDATE CASCADE,
  CONSTRAINT `fk_t2_t3_id` FOREIGN KEY (`t3_id`) REFERENCES `t3` (`id`) ON UPDATE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=2000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

CREATE TABLE `t3` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT COMMENT 't3.id',
  `name` varchar(255) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=3000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

Attached is the generated orm:

models.zip

@aarondl
Copy link
Member

aarondl commented Dec 27, 2016

So, finally got a chance to look into this in depth. It appears that the constraint finding mechanism is indeed wrong. I think the proper fix is change make it such that a unique constraint only changes a relationship into a one-to-one if the foreign key is the only key in the constraint. Which essentially means that the column count in the constraint affecting the key is 1.

What do you think about this logic?

yangyuqian pushed a commit to yangyuqian/sqlboiler that referenced this issue Jan 3, 2017
Determine unique associations for unique constraints with only 1 keys
@yangyuqian
Copy link
Author

@aarondl Yeah, it works! Thank you!

And I made a PR, trying to fix it
https://github.com/vattle/sqlboiler/pull/82

Schema for testing:

CREATE TABLE `country` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `name` varchar(255) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=3000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;


CREATE TABLE `user` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `name` varchar(255) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=1000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

CREATE TABLE `user_role` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `user_id` bigint(20) NOT NULL,
  `role_id` bigint(20) DEFAULT NULL,
  `country_id` bigint(20) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `user_id` (`user_id`,`role_id`),
  KEY `fk_role_id` (`role_id`),
  KEY `fk_country_id` (`country_id`),
  CONSTRAINT `fk_user_id` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON UPDATE CASCADE,
  CONSTRAINT `fk_role_id` FOREIGN KEY (`role_id`) REFERENCES `role` (`id`) ON UPDATE CASCADE,
  CONSTRAINT `fk_country_id` FOREIGN KEY (`country_id`) REFERENCES `country` (`id`) ON UPDATE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=2000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

CREATE TABLE `role` (
  `id` bigint(20) NOT NULL AUTO_INCREMENT,
  `name` varchar(255) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=3000 DEFAULT CHARSET=utf8 ROW_FORMAT=DYNAMIC;

aarondl added a commit that referenced this issue Jan 5, 2017
- Fix formatting on giant postgres query
- Invert the section that looks for unique constraints to look for a
  constraint and join to the constraint_column_usage to be more in line
  with the mysql query and also it makes a little bit more sense.
- Add more checks to ensure the schema is being enforced in the postgres
  side of things as several pieces in the unique checks were missing it
  which would lead to false positives.
- Fix #77 for postgres
@aarondl
Copy link
Member

aarondl commented Jan 5, 2017

@yangyuqian Is it possible you could test out the dev branch of sqlboiler right now to see if your issue is fixed there?

@aarondl
Copy link
Member

aarondl commented Jan 5, 2017

Oops, didn't mean "right now". Also if it looks good, we'll do a release of sqlboiler :)

@yangyuqian
Copy link
Author

Sure, I'm looking into it

@yangyuqian
Copy link
Author

@aarondl I've tested against my schema, it's working on dev branch, thank you

nullbio added a commit that referenced this issue Jan 6, 2017
* Includes fix #77 #82 (to one associations erroneously generated)
* Includes readme update
@aarondl aarondl closed this as completed in 23b6221 Jan 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants