updateOrCreate($model->toArray()); with double primary key returns SQLSTATE[23502]: Not null violation

[Jeroenvanderholst]

• Laravel Version: 6.9.0
• PHP Version: 7.3.9
• Database Driver & Version: PostgreSQL 12

Description:

I am fleshing out a multi level XML part that I am loading into a new model instance. This model consists of a double primary key: id and language.
The ->save() method works fine, but I need to update the data table which means that I could use the ->updateOrCreate() method, but I need to transform the object to an array first, as this is what the updateOrCreate() method eats. It does not take any model objects. I find that to be somewhat inconsistent.

When I reload the exact same data using this approach, it all works fine.

However, when I test my code by throwing away the first few records, the SQL server blows up, It gives a SQLSTATE[23502]: Not null violation error. The first record is not found, and it tries to create it, but for some vague reason the first primary key is returning "NULL"

Steps To Reproduce:

Here's my store method:

public function store(Request $request, EtimUpdate $etim_update, EtimUnit $etim_unit)
{
    // DB::table('etim_units')->delete();
    $units =$etim_update->harvest("Units");

    // ddd($unit->Translations->Translation[0]->attributes()["language"]->__toString());
    foreach($units->children() as $unit) {
        foreach ($unit->Translations->children() as $translation) {

           $etim_unit = new EtimUnit;
            $etim_unit->id = $unit->Code->__toString();
            $etim_unit->language = $translation->attributes()["language"];
            $etim_unit->description = $translation->Description->__toString();
            $etim_unit->abbreviation = $translation->Abbreviation->__toString();

            $etim_unit->updateOrCreate($etim_unit->toArray());
            // $etim_unit->save();
        }
    }
}

And the harvest() metthod asused above is coming from an inherited base class EtimUpdate.php, see code below:

public function harvest($element)
{
    $reader = new XMLReader();

   $reader->open($this->path . $this->filename) ;
   $result = [];

      while ($reader->read()) {

        if ($reader->name == $element && $reader->nodeType == XMLReader::ELEMENT) {
            // $result = new SimpleXMLElement($reader->readOuterXml());
            $result = simplexml_load_string($reader->readOuterXml());
            
            
         return $result;   
         
        }
    }
    $reader->close();
}

And here's a sample of a unit element which is loaded from a very large XML:

  <Units>
    <Unit>
      <Code>EU000002</Code>
      <Translations>
        <Translation language="de-DE">
          <Description>Hekto-Pascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="EN">
          <Description>Hecto Pascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="fi-FI">
          <Description>hehtopascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="fr-BE">
          <Description>hectopascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="it-IT">
          <Description>hPa</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="nb-NO">
          <Description>Hekto-Pascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="nl-BE">
          <Description>Hecto Pascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
        <Translation language="nl-NL">
          <Description>Hectopascal</Description>
          <Abbreviation>hPa</Abbreviation>
        </Translation>
      </Translations>
    </Unit>
  </Units>

The temporary workaround is to empty the data table and reload everything, but that is not the truly right way to maintain this data table. I need the updateOrCreate() method to work with a bunch of other tables that need to keep historic data too.

[driesvints]

        $etim_unit->id = $unit->Code->__toString();

This isn't case sensitive by any chance? Also: you're absolutely sure that all the data you're inserting isn't null at the time of writing to the DB?

[staudenmeir]

What's the whole error message?

Have you logged the executed queries and their bindings or looked at the content of $etim_unit->toArray()? Are the inserted values correct?

The ->save() method works fine, but I need to update the data table which means that I could use the ->updateOrCreate() method

updateOrCreate() only updates records if you provide a second argument: https://laravel.com/docs/eloquent#other-creation-methods

[Jeroenvanderholst]

@driesvints @staudenmeir thanks for your comments so far.
I think that case sensitivity is not the issue here, I tested nearly every step of the way with a ddd(); statement to see what is happening.
My best guess is that the updateOrCreate is missing an argument.
I tried that, but still the same error:

SQLSTATE[23502]: Not null violation: 7 ERROR:  null value in column "id" violates not-null constraint

DETAIL: Failing row contains (null, de-DE, Hekto-Pascal, hPa, 2020-01-13 21:07:33, 2020-01-13 21:07:33). (SQL: insert into "etim_units" ("language", "description", "abbreviation", "updated_at", "created_at") values (de-DE, Hekto-Pascal, hPa, 2020-01-13 21:07:33, 2020-01-13 21:07:33))

I see what went wrong here, the methods are written for models with standard autoincremental id's. In this case I have turned this off.

In the model I have stated:
public $incrementing = false;

What the updateOrCreate method is trying to do is to create a new record when the model is not present, but it gives all parameteres BUT the id. Even when $incrementing = false.

So now we know why. The question to answer now @driesvints is: does the framework wish to facilitate the updateOrCreate() method for models where id's are fixed non incremental.. if so, the method needs adjustments.

Dankjewel, danke schön!

Keep up the good work

[staudenmeir]

Does the model's $fillable array contain the id attribute?

[Jeroenvanderholst]

euuhhh nope...
hmmm good point.
I tried that, but now the update part blows up:

    SQLSTATE[23505]: Unique violation: 7 ERROR:  duplicate key value violates unique constraint "etim_units_pkey"
DETAIL:  Key (id, language)=(EU000003, de-DE) already exists. (SQL: update "etim_units" set "language" = de-DE, "updated_at" = 2020-01-14 08:13:40 where "id" = EU000003)

What the method is trying to do is to set the language to "de-DE" (1st pk) of all records that have id (2nd pk) = "EU000003". That results in language changes , ending up with all instances of EU000003 with the same language. So my sql-server blows up as i have set a unige constraint on the double primary key.

The actual sql query I am looking to get in the case of a record update is the following:

update "etim_units" set 
"updated_at" = '2020-01-14 08:13:40', 
"description" = 'Kilo-Blindleistung', 
"abbreviation" = 'kvar' 
where 
"id" = 'EU000003' AND "language" = 'de-DE'

I have to have the unique constraint on the double primary key. I only need to overwrite the record if it allready exists. Any ideas?

[driesvints]

Hmm, I think it's probably going to be setKeysForSaveQuery. I don't think Eloquent really works very well with double primary keys. We could maybe look into it to support it better for these use-cases. But I guess that would be more of an enhancement than a bug fix.

[staudenmeir]

Due to the composite primary key, updateOrCreate() does indeed not work in this case.

You can use the query builder's updateOrInsert() instead:

DB::table('etim_units')->updateOrInsert([
    'id' => $etim_unit->id,
    'language' => $etim_unit->language,
], [
    'description' => $etim_unit->description,
    'abbreviation' => $etim_unit->abbreviation,
]);

A different approach would be using UPSERT queries, I've created a package for that. The advantage here is that you can insert and update all your records with a single query (instead of executing two queries per record):

EtimUnit::upsert([
    [
        'id' => ...,
        'language' => ...,
        'description' => ...,
        'abbreviation' => ...,
    ],
    [
        'id' => ...,
        ...
    ],
], ['id', 'language']);;

[driesvints]

@Jeroenvanderholst do any of the options above work for you?

[Jeroenvanderholst]

Hmm, I think it's probably going to be setKeysForSaveQuery. I don't think Eloquent really works very well with double primary keys. We could maybe look into it to support it better for these use-cases. But I guess that would be more of an enhancement than a bug fix.

I agree, not a bug, but an enhancement and clarification request for the docs.

[Jeroenvanderholst]

Yes these options will work fine I think.
Many thanks for your help.