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

remove ON DUPLICATE #630

Closed
anonymous-matomo-user opened this issue Mar 25, 2009 · 9 comments
Closed

remove ON DUPLICATE #630

anonymous-matomo-user opened this issue Mar 25, 2009 · 9 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Milestone

Comments

@anonymous-matomo-user
Copy link

removing mysqlish in order to improve compatibliy with other database backend.
So replace the ON DUPLICATE with : SELECT ? UPDATE : INSERT;

As it will be ok because the ON DUPLICATE clause is use only on queries wchich not used a lot. It can have faillure if it is used concurrently, the time to do a fail update, an insert can hit the database and then the next insert (from the failed update) will failled.
However we can use a sql function if needed (it will be able to catch the error and make the right thing).

@anonymous-matomo-user
Copy link
Author

ahem, not SELECT?UPDATE:INSERT; but UPDATE?:INSERT;

@anonymous-matomo-user
Copy link
Author

Attachment: remove ON DUPLICATE
[remove_duplicate.patch](http://issues.piwik.org/attachments/630/remove_duplicate.patch)

@mattab
Copy link
Member

mattab commented Mar 30, 2009

did you test all the affected features after applying your patch? including the profiler?
also why do you round the time in ms?

@anonymous-matomo-user
Copy link
Author

Replying to [matt](comment:3):

> did you test all the affected features after applying your patch? including the profiler?
> also why do you round the time in ms?

I didn’t test the query profiler.

In the patch there is 2 extra ‘array’ in the last part :/ (before both $paramsBind).

About the round : actually there is number_fomat and in the db a double precision (float). I find better to have integer on both side. (I think I had an issue on that part and just crop)

@mattab
Copy link
Member

mattab commented Mar 31, 2009

please make sure you test your patch fully before submitting it for review..

@anonymous-matomo-user
Copy link
Author

Replying to [matt](comment:5):

> please make sure you test your patch fully before submitting it for review..

yes you are right, I did trust too much the unit test.

@robocoder
Copy link
Contributor

Can we create a wrapper function for this? I would prefer to keep the ON DUPLICATE query for mysql and provide the alternate UPDATE/INSERT query for other DB's.

@anonymous-matomo-user
Copy link
Author

adding a wrapper mean starting handle multiple database.
Even if I am pro-postgresql backend, I prefer see more standard sql code than wrappers.

@mattab
Copy link
Member

mattab commented May 18, 2009

I agree it would be better if the postgresql wrapper would special handling this type of queries. You could use the event 'Reporting.createDatabase' to create your special database object that would overload the default ones to special handle this type of queries. Please let us know if this works as expected.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

No branches or pull requests

3 participants