Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insert to table with multi primary or without autoicrement primary #105

Merged
merged 6 commits into from Jan 26, 2017

Conversation

@ricco24
Copy link
Contributor

ricco24 commented Oct 4, 2015

Insert to table without autoincrement primary or with multiple column primary returns inserted record.

I cant find bug1342.postgre but i think this solve this bug too. Maybe i can delete this bug report test file.

fixes #41 #80

…turns inserted record
@Unlink

This comment has been minimized.

Copy link
Contributor

Unlink commented Oct 8, 2015

Duplicate #102?
My PR also handles situations, where you have composite primary key and one part of it has autoincrement.
Try this:
https://github.com/nette/database/pull/102/files#diff-a5ef18703e35a05b08eb6089336ad773R21

@ricco24

This comment has been minimized.

Copy link
Contributor Author

ricco24 commented Oct 8, 2015

Yes, it is duplicate. I noticed your PR until after I made ​​my. So i let it here.
But this PR handles composite key with one autoincrement part too.

@dg

This comment has been minimized.

Copy link
Member

dg commented Oct 9, 2015

So ... which way to go? ;-)

@Unlink

This comment has been minimized.

Copy link
Contributor

Unlink commented Oct 9, 2015

@dg I'm not sure, i don't have completed API for IStructrure in my PR and i found there some issues.
so if @ricco24 adjusts them, i'll close my PR.

@dg dg force-pushed the nette:master branch 3 times, most recently from 6c43c50 to 6690dde Nov 4, 2015
@dg dg force-pushed the nette:master branch from 6f377ec to 9948322 Feb 8, 2016
@dg dg force-pushed the nette:master branch 4 times, most recently from 0771c2f to 425ad23 Apr 20, 2016
@dg dg force-pushed the nette:master branch from e5a9b9e to 26cc742 May 8, 2016
@dg dg force-pushed the nette:master branch 2 times, most recently from 59a6661 to b660054 May 30, 2016
@Unlink Unlink mentioned this pull request Jun 10, 2016
@dg dg force-pushed the nette:master branch 2 times, most recently from 5c87d56 to d9fd67a Jun 27, 2016
@dg dg mentioned this pull request Jun 28, 2016
@dg dg force-pushed the nette:master branch 8 times, most recently from fa2defa to 03312a2 Oct 18, 2016
@dg dg force-pushed the nette:master branch 11 times, most recently from 6ff42c6 to d46fee7 Jan 24, 2017
@ricco24 ricco24 changed the base branch from master to v2.4 Jan 26, 2017
@ricco24 ricco24 changed the base branch from v2.4 to master Jan 26, 2017
@ricco24 ricco24 changed the base branch from master to v2.4 Jan 26, 2017
@ricco24

This comment has been minimized.

Copy link
Contributor Author

ricco24 commented Jan 26, 2017

@dg what with this? It is possible to merge or not? In current state nette database is unusable with php7.1, postgres 8.3+ and uuid as priamry key. Every insert fails ...

@dg

This comment has been minimized.

Copy link
Member

dg commented Jan 26, 2017

May this cause a BC break? (Except that it adds new method to interface.) Or is it suitable for 2.4?

@dg dg force-pushed the nette:v2.4 branch from 8fbb6dd to ba4a57b Jan 26, 2017
@@ -1,4 +1,4 @@
/*!40102 SET storage_engine = InnoDB */;
/*!40102 SET default_storage_engine = InnoDB */;

This comment has been minimized.

Copy link
@dg

dg Jan 26, 2017

Member

What about CREATE TABLE ... ENGINE=InnoDB as used in other files?

Samuel Kelemen
// Search for autoincrement key from multi primary key
if (is_array($primaryKey)) {
foreach ($this->getColumns($table) as $column) {
if (in_array($column['name'], $primaryKey) && $column['autoincrement']) {

This comment has been minimized.

Copy link
@dg

dg Jan 26, 2017

Member

in_array() is relative slow operation, is possible this?

		if (is_array($primaryKey)) {
			$keys = array_flip($primaryKey);
			foreach ($this->getColumns($table) as $column) {
				if (isset($keys[$column['name']]) && $column['autoincrement']) {
					return $column['name'];
				}
			}
			return NULL;
		}
@dg

This comment has been minimized.

Copy link
Member

dg commented Jan 26, 2017

What about move searching for keys from getPrimaryAutoincrementKey (and maybe from getPrimaryKeySequence too) to loadStructure and use cache?

@dg dg force-pushed the nette:v2.4 branch from ba4a57b to 8fbb6dd Jan 26, 2017
@ricco24

This comment has been minimized.

Copy link
Contributor Author

ricco24 commented Jan 26, 2017

That seems to be a good idea. I can make another pr with optimized loadStructure() function when this will be merged.

@dg

This comment has been minimized.

Copy link
Member

dg commented Jan 26, 2017

Ok, so I'll merge it. Thanks! 1f6133b

@dg dg merged commit a0ef4c4 into nette:v2.4 Jan 26, 2017
1 of 3 checks passed
1 of 3 checks passed
coverage/coveralls Coverage decreased (-0.04%) to 86.249%
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dg added a commit that referenced this pull request Jan 26, 2017
dg added a commit that referenced this pull request Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.