Allow specifying the compare-array for insertIfNotExists() #14766
Conversation
| 'configvalue' => $value, | ||
| 'userid' => $userId, | ||
| 'appid' => $appName, | ||
| ]); |
There was a problem hiding this comment.
What about only having to specify the array keys instead of duplicating the values as well?
There was a problem hiding this comment.
I guess that makes sense, yes
nickvergessen
force-pushed
the
fix-insertifnotexists-poc
branch
from
07c87ea to
0a21608
Compare
|
@nickvergessen I see what you are trying to accomplish - bääh - we should have continue to talk about this days back ... well - shit happens. |
|
What I was thinking about while having a look at the implementation of insertIfNoExists(): |
|
61e4fd7 - contains a unit test to show this approach basically works |
…de the real exception
DeepDiver1975
force-pushed
the
fix-insertifnotexists-poc
branch
from
1e7d3ee to
89be55a
Compare
| if ($row = $result->fetchRow()) { | ||
| $this->numericId = $row['numeric_id']; | ||
| } else { | ||
| throw new \Exception('Storage exists when inserting and does not exist on select... go away'); |
There was a problem hiding this comment.
This is temp code and must not happen
I think we can/should do this, to make it possible to differ between a normal DB exception and this case. Shall I go ahead and implement that @DeepDiver1975 |
89be55a <- already done |
nickvergessen
changed the title
| if (!$exists && $preCondition === null) { | ||
| $sql = 'INSERT INTO `*PREFIX*preferences` (`configvalue`, `userid`, `appid`, `configkey`)'. | ||
| 'VALUES (?, ?, ?, ?)'; | ||
| $this->connection->insertIfNotExist('*PREFIX*preferences', [ |
There was a problem hiding this comment.
this whole code block cries for a second method: InsertOrUpdate - but this is OT
There was a problem hiding this comment.
Does this really work? The backticks are missing and I remind problems on OCI with this. @DeepDiver1975 Can you run the oracle test suite?
There was a problem hiding this comment.
The method adds the backticks. The code around here, does not know anything about sql syntax ;)
There was a problem hiding this comment.
The backticks are in lib/private/db/adapter.php
|
From my POV this is ready to go - :high_five: @nickvergessen |
DeepDiver1975
changed the title
nickvergessen
force-pushed
the
fix-insertifnotexists-poc
branch
from
83b0a40 to
2af8fea
Compare
|
Refer to this link for build results (access rights to CI server needed): |
|
👍 ready to go |
|
@PVince81 @icewind1991 please review - THX |
| */ | ||
| public function insertIfNotExist($table, $input) { | ||
| public function insertIfNotExist($table, $input, array $compare = null) { | ||
| if ($compare === null) { |
There was a problem hiding this comment.
this should be empty() so we use all keys when an empty array is passed aswell :(, will fix asap
|
The inspection completed: 7 new issues, 6 updated code elements |
|
Refer to this link for build results (access rights to CI server needed): |
|
@nickvergessen shall we squash to allow easier backport? |
|
No, I prefer a cherry-pick session over a huge commit that no one understands anymore. |
|
Second review @MorrisJobke @LukasReschke ? |
|
@MorrisJobke @LukasReschke @PVince81 @icewind1991 please review - THX |
|
Works 👍 |
Allow specifying the compare-array for insertIfNotExists()
|
Backport is at #14914 |
No description provided.