volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.77k stars 548 forks source link

Missing shared objects in M-M relationships #877

Open namco1992 opened 3 years ago

namco1992 commented 3 years ago

What version of SQLBoiler are you using (sqlboiler --version)?

v4.2.0

What is your database and version (eg. Postgresql 10)

MySQL 5.7

Please provide a relevant database schema so we can replicate your issue (Provided you are comfortable sharing this)

(It's not a very realistic example but pls bear with it 😅 ) We have 3 entities here: person, bike and wheel. Person-bike and bike-wheel are M-M relationships. For example, Person A and Person B share the same bike, and the bike have 2 wheels. The relationships look like this:

person A --\          / Wheel A
              bike A 
person B --/          \ Wheel B

The DB schema:

# Dump of table persons
# ------------------------------------------------------------

CREATE TABLE `persons` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(256) DEFAULT NULL,
  `deleted_at` datetime DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

LOCK TABLES `persons` WRITE;

INSERT INTO `persons` (`id`, `name`, `deleted_at`)
VALUES
    (1,'persons_a',NULL),
    (2,'persons_b',NULL);

UNLOCK TABLES;

# Dump of table wheels
# ------------------------------------------------------------

CREATE TABLE `wheels` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(256) DEFAULT NULL,
  `deleted_at` datetime DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

LOCK TABLES `wheels` WRITE;

INSERT INTO `wheels` (`id`, `name`, `deleted_at`)
VALUES
    (1,'wheels_a',NULL),
    (2,'wheels_b',NULL);

UNLOCK TABLES;

# Dump of table bikes
# ------------------------------------------------------------

CREATE TABLE `bikes` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(256) DEFAULT NULL,
  `deleted_at` datetime DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

LOCK TABLES `bikes` WRITE;

INSERT INTO `bikes` (`id`, `name`, `deleted_at`)
VALUES
    (1,'bikes_a',NULL);

UNLOCK TABLES;

# Dump of table persons_bikes
# ------------------------------------------------------------

CREATE TABLE `persons_bikes` (
  `person_id` int(11) unsigned NOT NULL,
  `bike_id` int(11) unsigned NOT NULL,
  PRIMARY KEY (`person_id`,`bike_id`),
  KEY `persons_bikes_fk_bikes` (`bike_id`),
  CONSTRAINT `persons` FOREIGN KEY (`person_id`) REFERENCES `persons` (`id`),
  CONSTRAINT `persons_bikes_fk_bikes` FOREIGN KEY (`bike_id`) REFERENCES `bikes` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

LOCK TABLES `persons_bikes` WRITE;

INSERT INTO `persons_bikes` (`person_id`, `bike_id`)
VALUES
    (1,1),
    (2,1);

UNLOCK TABLES;

# Dump of table bikes_wheels
# ------------------------------------------------------------

CREATE TABLE `bikes_wheels` (
  `bike_id` int(11) unsigned NOT NULL,
  `wheels_id` int(11) unsigned NOT NULL,
  PRIMARY KEY (`bike_id`,`wheels_id`),
  KEY `bikes_wheels_fk_wheels` (`wheels_id`),
  CONSTRAINT `bikes_wheels_fk_wheels` FOREIGN KEY (`wheels_id`) REFERENCES `wheels` (`id`),
  CONSTRAINT `bikes_wheels_fk_bikes` FOREIGN KEY (`bike_id`) REFERENCES `bikes` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

LOCK TABLES `bikes_wheels` WRITE;

INSERT INTO `bikes_wheels` (`bike_id`, `wheels_id`)
VALUES
    (1,1),
    (1,2);

UNLOCK TABLES;

Further information. What did you do, what did you expect?

When we load the person's bikes and wheels like the code shows below:

    persons, err := models.Persons(
        qm.WhereIn("id in ?", 1, 2),
        qm.Load("Bikes.Wheels"),
    ).All(boil.GetDB())
    if err != nil {
        commons.Logger.Err(err).Msg("Cannot get RequestOrders from database")
        return
    }
    for _, p := range persons {
        fmt.Printf("Person %s has %d bike(s)\n", p.Name.String, len(p.R.Bikes))
        for _, b := range p.R.Bikes {
            fmt.Printf("Bike %s has %d wheels\n", b.Name.String, len(b.R.Wheels))
        }
    }

We would expect that the bike should have 2 wheels since it's the same bike shared by 2 persons. However, you will get output like this:

Person persons_a has 1 bike(s)
Bike bikes_a has 2 wheels
Person persons_b has 1 bike(s)
Bike bikes_a has 0 wheels

As you see here, the second bike entity referred by person B doesn't have wheels, even though it's the same bike as person A owns.

The problem is in the M-M eager loading template. https://github.com/volatiletech/sqlboiler/blob/6ccc92cffb06c37bd14183ea1e103ff942aee22b/templates/09_relationship_to_many_eager.go.tpl#L152-L154

Firstly, LoadBikes actually loads 2 identical records from the DB, but they are different objects in our code. Then the LoadWheels only load 2 wheels shared among the bikes. However, the outer loop exhausted the foreign results and assigned them to the first local object, therefore nothing left for the second one.

It can be fixed by simply swapping the outer and inner loops. I've fixed it in our customized templates and it works fine. I can create a PR if you agree that it's something worth fixing. Thanks!

aarondl commented 3 years ago

Hi @namco1992. I think in the past we've had similar issues but the other way around. We've tried to reverse these loops but end up with similar behaviors. What really needs to be done is to remove the break. But that causes all kinds of other problems. One day the hope is to deal with this via a rewrite of the eager loading bits.

In the meantime if you can find a solution that works for -all cases- I'd be glad to accept it.

namco1992 commented 3 years ago

Hello @aarondl, yes removing the break will do too. May I ask what are the other problems you mentioned and what kind of rewrite you want to do?

aarondl commented 3 years ago

The main one is addressing the lack of inner joins where it would be possible to minimize round trips. It's one of the primary motivations to redo this susyem.