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

Zend/Db/Sql/Insert - implement insert into select construction #5260

Merged
merged 2 commits into from
Mar 7, 2014
Merged

Zend/Db/Sql/Insert - implement insert into select construction #5260

merged 2 commits into from
Mar 7, 2014

Conversation

turrsis
Copy link
Contributor

@turrsis turrsis commented Oct 12, 2013

recreate the #5143

@Maks3w
Copy link
Member

Maks3w commented Oct 21, 2013

@turrsis Can you update this PR with the late bindings of #4907 ?

@turrsis
Copy link
Contributor Author

turrsis commented Oct 22, 2013

@Maks3w see my comment in your PR

@turrsis
Copy link
Contributor Author

turrsis commented Oct 23, 2013

@Maks3w updated.

@Maks3w
Copy link
Member

Maks3w commented Oct 23, 2013

@turrsis Could you highlight your changes? Seems that you'v added more stuff since my last review

@turrsis
Copy link
Contributor Author

turrsis commented Oct 23, 2013

*
* @param string|TableIdentifier $table
* @return Insert
*/
public function into($table)
{
$this->table = $table;
if ($table instanceof \Zend\Db\Sql\Ddl\CreateTable) {
Copy link
Member

Choose a reason for hiding this comment

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

Please add a use for this.

@ralphschindler
Copy link
Member

After chewing over this, I think I'd feel better if this was a class of its own. This feels like a mostly different responsibility, one which I think most people would not directly associate with the normal use cases of INSERT. That said, what if we created a class called InsertSelect that extended Insert with this additional functionality?

@turrsis
Copy link
Contributor Author

turrsis commented Nov 19, 2013

To my mind this is the same responsibility - http://en.wikipedia.org/wiki/Insert_(SQL)
In all manuals you can read two case, analog :

function insert($data) {
   if ($data instanceof Select) {
      $data = $data->execute()->toArray();
   } elseif ($data instanceof Url){
      ....
   }
   ...
}

As simple user i want put in $data parameter anything, such as http://www.accuweather.com/en/us/new-orleans-la/70112/weather-forecast/348585 , but programmers is limit my desires :)

@ralphschindler
Copy link
Member

Sold. Good arguments, I'll merge to develop.

@ralphschindler
Copy link
Member

Question on the API though- while I like select(Select $select);, using select() and values() are mutually exclusive. So why not have select() simply be a convenience method that proxies to values(), and as for teh internals, the $values property would either be an array of values or a Select object. Thoughts?

@turrsis
Copy link
Contributor Author

turrsis commented Nov 24, 2013

@ralphschindler That's what you've suggested?

@ralphschindler
Copy link
Member

Yeah, I think that looks pretty good.

@turrsis
Copy link
Contributor Author

turrsis commented Jan 10, 2014

@ralphschindler Can you merge this, if ok?

@weierophinney
Copy link
Member

@ralphschindler Based on your comments, I'm marking this for 2.3.0. Change the milestone if you feel differently.

@weierophinney weierophinney added this to the 2.3.0 milestone Mar 3, 2014
@ralphschindler
Copy link
Member

@turrsis Overall I like this, but I think it makes sense (until 3.0) to just use the protected $values to house the Select object. Also, (to that effect), I think it makes sense that values() accepts the Select object. I am going to start working on these small changes and unit tests to merge, unless you have an immediate objection.

ralphschindler pushed a commit that referenced this pull request Mar 7, 2014
Merge branch 'hotfix/db-sql-insert-into-select' of git://github.com/turrsis/zf2 into feature/5260-db-insert-with-select

* 'hotfix/db-sql-insert-into-select' of git://github.com/turrsis/zf2:
  optimize API
  implement INSERT INTO SELECT construction
ralphschindler pushed a commit that referenced this pull request Mar 7, 2014
Merge branch 'feature/5260-db-insert-with-select' into develop

* feature/5260-db-insert-with-select:
  Zend\Db\Sql\Insert * Added exceptions to Insert for Select/array inconsistencies * reverted property back to 'values' * CS fixes * Additional tests
  optimize API
  implement INSERT INTO SELECT construction
@ralphschindler ralphschindler merged commit b664b62 into zendframework:develop Mar 7, 2014
@ralphschindler ralphschindler self-assigned this Mar 11, 2014
@turrsis turrsis deleted the hotfix/db-sql-insert-into-select branch March 12, 2014 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants