Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Add insert ignore #336

Merged
merged 4 commits into from
Dec 31, 2019
Merged

Conversation

samsonasik
Copy link
Contributor

@samsonasik samsonasik commented Aug 11, 2018

  • Are you creating a new feature?
    • Why is the new feature needed? What purpose does it serve?
      Useful to ignore duplicate entry error.
    • How will users use the new feature?
$insert= new InsertIgnore('tablename');
$insert->values($data);

$statement = $sql->prepareStatementForSqlObject($insert);
$statement->execute();
  • Base your feature on the develop branch, and submit against that branch.
  • Add only one feature per pull request; split multiple features over multiple pull requests
  • Add tests for the new feature.
  • Add a CHANGELOG.md entry for the new feature.

@samsonasik
Copy link
Contributor Author

As INSERT IGNORE syntax seems not work in all database platform, I am not add insertIgnore() function into Sql and AbstractTableGateway class.

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't check all the tests, but the added functionality is simple and easy to understand - some nitpicks apply.

* @var array Specification array
*/
protected $specifications = [
self::SPECIFICATION_INSERT => 'INSERT IGNORE INTO %1$s (%2$s) VALUES (%3$s)',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all supported drivers capable of evaluating INSERT IGNORE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably MySQL only, that's why I didn't add insertIgnore() function to Sql and AbstractTableGateway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can we please survey that before introducing it then?

class InsertIgnore extends Insert
{
/**
* @var array Specification array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please specify the type of the array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is copied originally from Insert class, do you have sample of "type of the array"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string[] in this case, or even array<string, string> (unsure if the tooling craps out with that, though)

}

/**
* @covers \Zend\Db\Sql\InsertIgnore::into
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest dropping all these @covers, ans only adding one at class level

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed @covers

@Ocramius Ocramius added this to the 2.10.0 milestone Aug 12, 2018
@michalbundyra michalbundyra modified the milestones: 2.10.0, 2.11.0 Nov 26, 2019
michalbundyra added a commit that referenced this pull request Dec 31, 2019
michalbundyra added a commit that referenced this pull request Dec 31, 2019
michalbundyra added a commit that referenced this pull request Dec 31, 2019
@michalbundyra michalbundyra merged commit e83d278 into zendframework:develop Dec 31, 2019
@michalbundyra
Copy link
Member

Thanks, @samsonasik!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants