HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Is it possible to know if a Gdn_Model::Save() was successful, without using an autoincrement key?

I've struggling with an apparently failing Save() statement for a couple of hours, only to find out that it actually saves data correctly, but it doesn't return what one would expect. Environment is Vanilla 2.0.8.18.

The table against which the data is saved does not have an autoincrement primary key, but a UUID field, which is passed whenever Gdn_Model::Save() is called (i.e. both for Insert and Update operations). When the data is saved correctly, the following code is executed (source class.database.php):

      // Did this query modify data in any way?
      if ($ReturnType == 'ID') {
         $this->_CurrentResultSet = $this->Connection()->lastInsertId();
      } else {
         if ($ReturnType == 'DataSet') {
            // Create a DataSet to manage the resultset
            $this->_CurrentResultSet = new Gdn_DataSet();
            $this->_CurrentResultSet->Connection = $this->Connection();
            $this->_CurrentResultSet->PDOStatement($PDOStatement);
         }
      }

The issue is on the line that says $this->_CurrentResultSet = $this->Connection()->lastInsertId();. Since there is no autoincrement primary key, there will never be a "last insert ID". Therefore, the method will always return zero as a result, making it impossible to determine if the save was successful (unless one queries the database to check if the data is there, but that would mean doubling the amount of queries).

Adding an autoincrement key to the table would not be a viable workaround. The library I'm working must perform an "INSERT OR UPDATE" import, which means that some rows must be overwritten if they have changed. Since the autoincrement field would not exist in the source table, every Gdn_Model::Save() would trigger an INSERT, thus creating a lot of duplicates.

Back to the question, is there a (reliable) way to determine the result of a Gdn_Model::Save operation, without relying on autoincrement keys?

hgtonightperegrineUnderDog

Answers

  • peregrineperegrine MVP
    edited November 2013

    sometimes @Todd seems willing to lend a hand. if not @Lincoln.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

    businessdad
  • @businessdad said:

          // Did this query modify data in any way?
          if ($ReturnType == 'ID') {
             $this->_CurrentResultSet = $this->Connection()->lastInsertId();
    

    Oh boy...

    Can you figure out how the returntype gets returned? Is there a way you can plug in or hook in to that part of the sourcecode?

  • R_JR_J Ex-Fanboy Munich Admin

    I know no answer for that, sorry.

    But shouldn't that be a bug report? A framework should ensure that an action has happened and if it fails, it should give a reliable response.

    UnderDog
  • businessdadbusinessdad Stealth contributor MVP

    @UnderDog said:
    Can you figure out how the returntype gets returned?

    Unfortunately, no. PDO::lastInsertId() returns zero (int) when no new ID has been generated (whether because the INSERT failed or because there was really no ID to generate).

    Is there a way you can plug in or hook in to that part of the sourcecode?

    I'm afraid it won't be possible, either. There is no hook of any kind, the only solution would be modifying, or overriding, the core class.

  • businessdadbusinessdad Stealth contributor MVP

    @R_J said:
    But shouldn't that be a bug report? A framework should ensure that an action has happened and if it fails, it should give a reliable response.

    Technically, it's not a bug, but a design choice. As long as you use a primary key, the insert returns the new key value upon INSERT, and zero upon error.

    Interestingly, the logic breaks also if you have a composite primary key. The code, which I don't have handy, first fetches the primary key fields (e.g. Field1, Field2, Field3), stores them into an array, then sets its PrimaryKey property to the name of the first field, whatever it is, which is assumed to be, once again, an autoincrement field.

    Despite its limitations, it looks to me a clear design choice (i.e. "we assume all tables have an autoincrement field as primary key, and that's it").

  • can't you use have errorInfo()/errorCode() if the PDO object returned by Connection()?

    grep is your friend.

    businessdad
  • @UnderDog said:

    Sorry to be a pain... Let me ask in another way ...

    if ($ReturnType == 'ID')
    

    which function call generates (returns) $ ReturnType ?

  • businessdadbusinessdad Stealth contributor MVP

    @x00 said:
    can't you use have errorInfo()/errorCode() if the PDO object returned by Connection()?

    That could be an idea. Time for an experiment!

    @UnderDog said:
    which function call generates (returns) $ReturnType?

    It's set in Gdn_SqlDriver::Query():

       public function Query($Sql, $Type = 'select') {
          switch ($Type) {
             case 'insert': $ReturnType = 'ID'; break;
             case 'update': $ReturnType = NULL; break;
             default: $ReturnType = 'DataSet'; break;
          }
       [...]
    

    Anyway, I decided to override base model, because I will also need to handle composite primary keys. The differences between the original model and the updated one will be the following:

    • Original Gdn_Model expects the primary key to be an autoincrement field, generated automatically. If a value is passed for the primary key field, the query is automatically set to UPDATE. The updated model will allow to INSERT data when passing the primary key explicitly.
    • Original Gdn_Model doesn't handle multi-field primary keys. Updated model will handle them.
    • Updated Gdn_Model will implement some more error handling. The result of a Save() operation will be set to false in case of failure.

    I'm planning to add all these to my free AFC plugin, which I will be using as the foundation of most of my other plugins, themes and applications. Logger (aka Rogan), Cron (aka Garan) and AFC (aka Silia) will be my "holy trinity" of Vanilla development.

    UnderDoghgtonight
Sign In or Register to comment.