volatiletech / sqlboiler

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

Incorrect result of Inner Join & Bind #491

Closed a-bahmani closed 5 years ago

a-bahmani commented 5 years ago

sqlboiler :v3.2.0 go-sql-driver/mysql v1.4.1

I had run this query in PHPMyAdmin via sql query:

select trades.quantity as "quantity", trades.updated_at as "updated_at",
offers.price as "price", offers.buy as "buy", offers.sell as "sell" from trades 
inner join offers on trades.offer_id = offers.id 

And run it in my GoLang code via SQLBoiler:

type TradeAndOffer struct {
    Quantity  types.Decimal     `boil:"quantity" json:"quantity" toml:"quantity" yaml:"quantity"`
    UpdatedAt time.Time         `boil:"updated_at" json:"updated_at" toml:"updated_at" yaml:"updated_at"`
    Buy       bool              `boil:"buy" json:"buy" toml:"buy" yaml:"buy"`
    Sell      bool              `boil:"sell" json:"sell" toml:"sell" yaml:"sell"`
    Price     types.NullDecimal `boil:"price" json:"price" toml:"price" yaml:"price"`
}
var tradAndOffers []TradeAndOffer
err = models.NewQuery(
        qm.SQL(`select trades.quantity as "quantity", trades.updated_at as "updated_at",
                offers.price as "price", offers.buy as "buy", offers.sell as "sell" from trades 
                inner join offers on trades.offer_id = offers.id`),
    ).Bind(context.Background(), db, &tradAndOffers)

But the results are not same! In fact, the result was provided by sqlboiler is incorrect, why?

Schema

CREATE TABLE `offers` (
  `id` int(11) NOT NULL,
  `quantity` decimal(32,8) NOT NULL,
  `buy` tinyint(1) NOT NULL,
  `sell` tinyint(1) NOT NULL,
  `price` decimal(32,8) DEFAULT NULL,
  `is_active` tinyint(1) NOT NULL,
  `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `updated_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

INSERT INTO `offers` (`id`, `quantity`, `buy`, `sell`, `price`, `is_active`, `created_at`, `updated_at`) VALUES
(75, '100.00000000', 1, 0, '9.00000000', 1, '2019-02-21 03:15:54', '2019-02-21 03:15:54'),
(76, '110.00000000', 1, 0, '8.00000000', 1, '2019-02-21 03:16:01', '2019-03-01 10:29:26'),
(77, '140.00000000', 1, 0, '7.00000000', 1, '2019-02-21 03:16:11', '2019-03-01 10:29:51'),
(78, '100.00000000', 1, 0, '6.00000000', 1, '2019-02-21 03:16:16', '2019-02-21 03:16:16');

ALTER TABLE `offers`
  ADD PRIMARY KEY (`id`);

ALTER TABLE `offers`
  MODIFY `id` int(11) NOT NULL AUTO_INCREMENT, AUTO_INCREMENT=157;

CREATE TABLE `trades` (
  `id` int(11) NOT NULL,
  `offer_id` int(11) NOT NULL,
  `quantity` decimal(32,8) NOT NULL,
  `description` text NOT NULL,
  `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `updated_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

INSERT INTO `trades` (`id`, `offer_id`, `quantity`, `description`, `created_at`, `updated_at`) VALUES
(1, 75, '100.00000000', '', '2019-02-21 15:19:33', '2019-02-21 15:19:33'),
(2, 76, '120.00000000', '', '2019-02-21 15:42:04', '2019-02-28 22:48:45'),
(3, 77, '105.00000000', '', '2019-02-21 15:43:15', '2019-03-01 10:30:21');

ALTER TABLE `trades`
  ADD PRIMARY KEY (`id`);

ALTER TABLE `trades`
  MODIFY `id` int(11) NOT NULL AUTO_INCREMENT, AUTO_INCREMENT=4;

ALTER TABLE `trades`
  ADD CONSTRAINT `trades_offer` FOREIGN KEY (`offer_id`) REFERENCES `offers` (`id`);
COMMIT;
aarondl commented 5 years ago

I'm not seeing the issue here.

When I run the query you gave via mysql console:

mysql> select trades.quantity as "quantity", trades.updated_at as "updated_at",
    -> offers.price as "price", offers.buy as "buy", offers.sell as "sell" from trades 
    -> inner join offers on trades.offer_id = offers.id ;
+--------------+---------------------+------------+-----+------+
| quantity     | updated_at          | price      | buy | sell |
+--------------+---------------------+------------+-----+------+
| 100.00000000 | 2019-02-21 15:19:33 | 9.00000000 |   1 |    0 |
| 120.00000000 | 2019-02-28 22:48:45 | 8.00000000 |   1 |    0 |
| 105.00000000 | 2019-03-01 10:30:21 | 7.00000000 |   1 |    0 |
+--------------+---------------------+------------+-----+------+

And when I run a sample program with the code you provided and your schema:

select trades.quantity as "quantity", trades.updated_at as "updated_at",
                offers.price as "price", offers.buy as "buy", offers.sell as "sell" from trades 
                inner join offers on trades.offer_id = offers.id
[]
([]main.TradeAndOffer) (len=3 cap=4) {
 (main.TradeAndOffer) {
  Quantity: (types.Decimal) 105.00000000,
  UpdatedAt: (time.Time) 2019-02-21 15:19:33 +0000 UTC,
  Buy: (bool) true,
  Sell: (bool) false,
  Price: (types.NullDecimal) 7.00000000
 },
 (main.TradeAndOffer) {
  Quantity: (types.Decimal) 105.00000000,
  UpdatedAt: (time.Time) 2019-02-28 22:48:45 +0000 UTC,
  Buy: (bool) true,
  Sell: (bool) false,
  Price: (types.NullDecimal) 7.00000000
 },
 (main.TradeAndOffer) {
  Quantity: (types.Decimal) 105.00000000,
  UpdatedAt: (time.Time) 2019-03-01 10:30:21 +0000 UTC,
  Buy: (bool) true,
  Sell: (bool) false,
  Price: (types.NullDecimal) 7.00000000
 }
}

Can you update this issue with the part that's incorrect or unexpected (you didn't fill out the issue template which does specifically ask for "what you expected" and "what actually happened")?

a-bahmani commented 5 years ago

Exactly what I said. Just look deeper at "Quantity" & "Price" columns. I put your results besides to compare them.

Quantity(Query) | Quantity(SQLBoiler) | Price(Query) | Price(SQLBoiler)
100.00000000    | 105.00000000        | 9.00000000   | 7.00000000
120.00000000    | 105.00000000        | 8.00000000   | 7.00000000
105.00000000    | 105.00000000        | 7.00000000   | 7.00000000
aarondl commented 5 years ago

Sorry must be blind. I could have swore i accounted for it all... I thought I specifically checked that haha. I'll look into it further.

a-bahmani commented 5 years ago

Thank you for your prompt reply. Special thanks for your great library. I'll be waiting for your updates.

aarondl commented 5 years ago

I understand what's happening now. When I initially looked at my results I was looking at the date field, which awkwardly is the only one that's working here, you'll notice that the fields that are not working are the Decimal fields. There's also a test that "proves" this works so I was majorly confused while debugging this.

The way Bind works in the case where you give it []TradeAndOffer as a slice to bind to, is it creates a single object on the heap (one chunk of memory) and binds to it repeatedly. When the slice is appended to, that value is copied by value. This is the problem.

types.Decimal includes a pointer, which even if it's copied by value it's of course going to be copying that reference. That pointer to the Decimal gets Scan'd into repeatedly and we end up with the last row scanned replacing the result for the pointer's chunk of memory which all TradeAndOffers are pointing to. You can see this if you do this in your sample code:

fmt.Printf("%p %p %p\n", tradeAndOffers[0].Quantity.Big, tradeAndOffers[1].Quantity.Big, tradeAndOffers[2].Quantity.Big)

Which on my machine outputs:

0xc00017a070 0xc00017a070 0xc00017a070

All the same pointer.

So as for how to solve this issue all you need to do is use []*TradeAndOffer as your slice type and it will be fine. And I will add a bit of documentation to Bind() warning people of reference types when trying to bind to a slice of by-value structs.