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

Upsert with condition? #47

Closed
Shamshiel opened this issue Mar 14, 2019 · 20 comments
Closed

Upsert with condition? #47

Shamshiel opened this issue Mar 14, 2019 · 20 comments
Labels
enhancement New feature or request

Comments

@Shamshiel
Copy link

Would it be possible to implement an upserts with a condition like "upsert if date is newer"?
I only want to update my entity if the entity that I received is newer.
Or is this already possible?

@APIWT
Copy link
Contributor

APIWT commented Mar 14, 2019

Hey @Shamshiel!

I think this would be related to something like this: #40

Take these MySQL-pseudo example queries (both should do pretty much the same thing):

INSERT INTO `LastVisit` (UserId, VisitDate) VALUES ('1', '2019-03-14')
ON DUPLICATE KEY UPDATE VisitDate = VALUES(VisitDate) WHERE VisitDate < VALUES(VisitDate)
INSERT INTO `LastVisit` (UserId, VisitDate) VALUES ('1', '2019-03-14')
ON DUPLICATE KEY UPDATE VisitDate = IF(VisitDate < VALUES(VisitDate), VALUES(VisitDate), VisitDate)

That being said, I'm not sure this is yet possible with the library. If you give me some more detail (ie: Your database engine) I would be happy to help you construct a query manually. I think @artiomchi is working on adding support for something like this but as far as I know it might be a while since it gets quite complicated with expression matchers.

@Shamshiel
Copy link
Author

I'm currently working with postgresql. Basically, I only want to insert/update my data if it is newer then the data in the database.

@artiomchi
Copy link
Owner

I'm currently working on upgrading the matching algorithm to make the update statements more flexible.

I'll look into adding filter clauses as well for the updates, that's a great idea!

@artiomchi artiomchi added the enhancement New feature or request label Mar 15, 2019
@APIWT
Copy link
Contributor

APIWT commented May 6, 2019

@artiomchi Hope you are doing well :) Any update on this? I'm trying to do this in my WhenMatched:

.WhenMatched(h => new History
{
    Count = h.Count + countDelta < 0 ? 0 : h.Count + countDelta,
})

@artiomchi
Copy link
Owner

Hey @APIWT

I've worked on improving the expression parser, and I have the tertiary expression support working at last!
This has been released in 2.1.0 just now

I'm still looking at getting the WHERE clause added, but I'm finally back working on the project, so should have something ready soon too

@artiomchi
Copy link
Owner

Hey @APIWT

INSERT INTO `LastVisit` (UserId, VisitDate) VALUES ('1', '2019-03-14')
ON DUPLICATE KEY UPDATE VisitDate = VALUES(VisitDate) WHERE VisitDate < VALUES(VisitDate)

You mentioned this syntax for MySQL purely as pseudocode, or as a working MySQL syntax?

I can't find a mention of the WHERE support in MariaDB, and after I tried to add this functionality to the library, MySQL threw a syntax error in my face :)

So far, as far as I can see, this specific syntax isn't supported in MySQL, but works fine in Sqlite, Postgres and MSSQL.

P.S. Alternatively I can add an "ugly mode" for MySQL where this is simulated.. and for every property in the update statement it'll have a CASE statement, testing the same clause :)

@APIWT
Copy link
Contributor

APIWT commented May 21, 2019

Hey @APIWT

INSERT INTO `LastVisit` (UserId, VisitDate) VALUES ('1', '2019-03-14')
ON DUPLICATE KEY UPDATE VisitDate = VALUES(VisitDate) WHERE VisitDate < VALUES(VisitDate)

You mentioned this syntax for MySQL purely as pseudocode, or as a working MySQL syntax?

I can't find a mention of the WHERE support in MariaDB, and after I tried to add this functionality to the library, MySQL threw a syntax error in my face :)

So far, as far as I can see, this specific syntax isn't supported in MySQL, but works fine in Sqlite, Postgres and MSSQL.

P.S. Alternatively I can add an "ugly mode" for MySQL where this is simulated.. and for every property in the update statement it'll have a CASE statement, testing the same clause :)

@artiomchi

Yeah I think that unfortunately you have to use IF (my second version). Sorry for the confusion, I'm not sure what I was thinking when I originally wrote the comment.

Hope all is well!

@artiomchi
Copy link
Owner

Uuuuugh.. MySQL is so frustrating!

This is the query I ended up generating in my unit tests:

INSERT INTO `TestEntities` (`Num1`, `Num2`, `NumNullable1`, `Text1`, `Text2`, `Updated`)
VALUES (@p0, @p1, @p2, @p3, @p4, @p5)
ON DUPLICATE KEY UPDATE
  `Num2` = IF (`Num2` != VALUES(`Num2`), VALUES(`Num2`), `Num2`),
  `NumNullable1` = IF (`Num2` != VALUES(`Num2`), VALUES(`NumNullable1`), `NumNullable1`),
  `Text1` = IF (`Num2` != VALUES(`Num2`), VALUES(`Text1`), `Text1`),
  `Text2` = IF (`Num2` != VALUES(`Num2`), VALUES(`Text2`), `Text2`),
  `Updated` = IF (`Num2` != VALUES(`Num2`), VALUES(`Updated`), `Updated`)

Annoyingly enough, each update is processed separately.. So after Num2 is updated, the other columns are not updated, since Num2 is now the same as the value we tried to insert... That leaves the table half updated!

To avoid really bad and unexpected results, I'll just mark this as an unsupported feature, unless someone comes up with a really cool idea on how to do it 😞

@artiomchi
Copy link
Owner

Other than MySql limitations, this is now working.. Both conditional expressions:

dbContex.TestEntities.Upsert(newItem)
    .On(j => j.Num1)
    .WhenMatched((je, jn) => new TestEntity
    {
        Num2 = je.Num2 - jn.Num2 > 0 ? je.Num2 - jn.Num2 : 0,
    })
    .Run();

and update filter expressions:

dbContex.TestEntities.Upsert(newItem)
    .On(j => j.Num1)
    .WhenMatched((e1, e2) => new TestEntity
    {
        Num2 = e2.Num2,
    })
    .UpdateIf((ed, en) => ed.Num2 != en.Num2)
    .Run();

@APIWT
Copy link
Contributor

APIWT commented May 21, 2019

Wow that is very annoying and clever of you to test for. Did you keep it implemented even though there is thing strange behavior? I could see it as being something that is noted in the documentation as something to look out for rather than excluding it all together.

@artiomchi
Copy link
Owner

I didn't keep the implementation, since it would cause unexpected behaviour / bug requests. At the moment, if you try to use the update filter expressions with MySql it'll throw an exception with a link to Wiki: MySql-Conditional-Updates

But with conditional expression support, one could implement the same thing by hand, with the flexibility of having the option which columns will get the test:

dbContex.TestEntities.Upsert(newItem)
    .On(j => j.Num1)
    .WhenMatched((e1, e2) => new TestEntity
    {
        Num2 = ed.Num2 != en.Num2 ? e2.Num2 : e1.Num2,
        Updated = ed.Num2 != en.Num2 ? e2.Updated : e1.Updated,
    })
    .Run();

I don't mind reconsidering and coming up with alternative options, it was just the best option I had at the time :)

@APIWT
Copy link
Contributor

APIWT commented May 22, 2019

@artiomchi
I've got an idea, but I'll need your help testing it. It involves abusing the order of operations and user-defined variables:

INSERT INTO `TestEntities` (`Num1`, `Num2`, `NumNullable1`, `Text1`, `Text2`, `Updated`)
VALUES (@p0, @p1, @p2, @p3, @p4, @p5)
ON DUPLICATE KEY UPDATE
  `Num2` = COALESCE(IF((@Num2 := `Num2`), NULL, NULL), IF((@Text1 := `Text1`), NULL, NULL), IF((@Text2 := `Text2`), NULL, NULL), IF((@Updated := `Updated`), NULL, NULL), IF (@Num2 != VALUES(`Num2`), VALUES(`Num2`), @Num2)),
  `NumNullable1` = IF (@Num2 != VALUES(`Num2`), VALUES(`NumNullable1`), @NumNullable1),
  `Text1` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text1`), @Text1),
  `Text2` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text2`), @Text2),
  `Updated` = IF (@Num2 != VALUES(`Num2`), VALUES(`Updated`), @Updated)

(Make sure you scroll all the way to the right on that first line with COALESCE or you might not see it all, especially if you are on a Mac like I am)

I did a little bit of testing last night and it works as far as I can tell. I did not try with multiple records at once. Let me know if you need more of an explanation how the voodoo magic works. Credit to the following article for giving me the idea: https://thewebfellas.com/blog/conditional-duplicate-key-updates-with-mysql/

@artiomchi
Copy link
Owner

Well DAMN.. That's pretty awesome, in a horrific-wtf-mysql kind of way 👍 But it seems to work exactly as promised, and passes all the unit tests!

I had to tinker a bit with the query generator and add more extensibility, but I kept it pretty clean inside, so that keeps me happy.

Thanks a lot for the idea.
Going to push another minor release with this.

P.S. I love the OSS community :)

@APIWT
Copy link
Contributor

APIWT commented May 23, 2019

I am very happy that it worked out :) Great job implementing it as well. Did you get a chance to test it with multiple records in a single upsert?

PS: I love the OSS community too! Libraries like yours really make the world better.

@artiomchi
Copy link
Owner

Yup, one of the unit tests tries it with one column, another one tries it with multiple columns. In fact, the SQL I used as an example above was from that unit test.

The generated query is quite similar to what you listed. I just prefixed the variables with x to make sure there's no chance of them clashing with query parameters.

@APIWT
Copy link
Contributor

APIWT commented May 23, 2019

@artiomchi Quick question: Are you also replacing the reference to the variables in the part of the statement that is not the condition?

In other words, is the query like this:

INSERT INTO `TestEntities` (`Num1`, `Num2`, `NumNullable1`, `Text1`, `Text2`, `Updated`)
VALUES (@p0, @p1, @p2, @p3, @p4, @p5)
ON DUPLICATE KEY UPDATE
  `Num2` = COALESCE(IF((@Num2 := `Num2`), NULL, NULL), IF((@Text1 := `Text1`), NULL, NULL), IF((@Text2 := `Text2`), NULL, NULL), IF((@Updated := `Updated`), NULL, NULL), IF (@Num2 != VALUES(`Num2`), VALUES(`Num2`), @Num2)),
  `NumNullable1` = IF (@Num2 != VALUES(`Num2`), VALUES(`NumNullable1`), @NumNullable1),
  `Text1` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text1`), @Text1),
  `Text2` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text2`), @Text2),
  `Updated` = IF (@Num2 != VALUES(`Num2`), VALUES(`Updated`), @Updated)

or is it like this?

INSERT INTO `TestEntities` (`Num1`, `Num2`, `NumNullable1`, `Text1`, `Text2`, `Updated`)
VALUES (@p0, @p1, @p2, @p3, @p4, @p5)
ON DUPLICATE KEY UPDATE
  `Num2` = COALESCE(IF((@Num2 := `Num2`), NULL, NULL), IF((@Text1 := `Text1`), NULL, NULL), IF((@Text2 := `Text2`), NULL, NULL), IF((@Updated := `Updated`), NULL, NULL), IF (@Num2 != VALUES(`Num2`), VALUES(`Num2`), `Num2`)),
  `NumNullable1` = IF (@Num2 != VALUES(`Num2`), VALUES(`NumNullable1`), `NumNullable1`),
  `Text1` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text1`), `Text1`),
  `Text2` = IF (@Num2 != VALUES(`Num2`), VALUES(`Text2`), `Text2`),
  `Updated` = IF (@Num2 != VALUES(`Num2`), VALUES(`Updated`), `Updated`)

@artiomchi
Copy link
Owner

No, I didn't replace them.. So the generated query looks like the second statement.

Upon looking onto it, and running some tests to be sure, the column mentioned in each statement is only updated by that one statement, so it didn't seem to make a difference to use one or the other.

So for simplicity's sake, I left it as is.

Do you think this could cause an issue?

@APIWT
Copy link
Contributor

APIWT commented May 23, 2019

To be 100% honest, I'm not entirely sure. I suppose it just depends on something like this example:

Consider a record where the visits currently equals 5 and ignore the obvious duplication of unnecessary data for the sake of example.

INSERT INTO `VisitCount` (`Username`, `Visits`, `Visits2`)
VALUES ('Artiom', '1', '1')
ON DUPLICATE KEY UPDATE
   `Visits` = `Visits` + VALUES(`Visits`)
   `Visits2` = `Visits` + VALUES(`Visits`)

Will the value of Visits2 be 6 or 7 after the operation runs?

@artiomchi
Copy link
Owner

Oof man.. yeah, that's true.

Although to be honest, that opens up a whole new can of worms - just a regular update could cause that too, without the whole filter expressions addition.

Whilst this is most certainly an issue, I think it's more of an issue with MySQL than this library, so I'm considering to leave this outside the scope of this project. At least for the time being.

The alternative would be to add the whole variable shabang on every single update using MySql. While it would make it behave like other RDBMSs, it would make the queries more complex, when in 99% of the cases it would be unnecessary.

If you (or anyone reading this later) considers to be an issue worth fixing, we can always open a new issue, and tackle it separately :)

@APIWT
Copy link
Contributor

APIWT commented May 23, 2019

As long as I was able to bring it up, my conscious is clear :) I totally agree with you that it makes things far more complicated. Thanks for responding so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants