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

Make the Model work without auto_increment primary key #3134

Closed
MashinaMashina opened this issue Jun 19, 2020 · 12 comments
Closed

Make the Model work without auto_increment primary key #3134

MashinaMashina opened this issue Jun 19, 2020 · 12 comments
Labels
new feature PRs for new features

Comments

@MashinaMashina
Copy link
Contributor

Describe the bug

  1. Table:
CREATE TABLE test (
key varchar(20),
value integer,
primary key (key)
)
  1. Code:
$testModel = new TestModel;
$testModel->insert($data);
  1. Error: pg_query(): Query failed: ERROR: lastval is not yet defined in this session

I can create PR, but without description. I think to create property CodeIgniter\Model::$has_autoincrement and add checking it in \system\Model.php:731, but I don't know if is a good idea

if ($result and $this->has_autoincrement)
{
	$this->insertID = $this->db->insertID();
}
@MashinaMashina MashinaMashina added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 19, 2020
@michalsn
Copy link
Member

It's not really a bug - more like a feature request.

Contributions are always welcomed - but please be sure to add some tests, to make sure this change would not break anything. Also, take into consideration that we have to support $this->insertID in a model (it has to work) even if there is no auto_increment in the database set.

@michalsn michalsn added new feature PRs for new features and removed bug Verified issues on the current code behavior or pull requests that will fix them labels Jun 20, 2020
@michalsn michalsn changed the title Bug: Postgres insert() dont work without autoincrement primary key Make the Model work without auto_increment primary key Jun 20, 2020
@MashinaMashina
Copy link
Contributor Author

MashinaMashina commented Jun 20, 2020

For get inserted primary key value, the best way is using "RETURNING primaryKey" in query, but database driver dont know primary key, model can't change SQL string.

I dont know a way

A create initial PR, how much could https://github.com/MashinaMashina/CodeIgniter4/tree/Model_without_autoincrement_key

@michalsn
Copy link
Member

@MashinaMashina I think you should create a PR. This way we can easier discuss your code.

In general, if the "auto increment" option is disabled and there is no data[primary_key] available, we should throw an exception.

Additionally, we will need some tests and docs updates. If you're willing to work on this I will be glad to help with your questions if you will have any.

@crustamet
Copy link
Contributor

I wanted this feature before, this going to be commited ? or it will just remain like it is right now ?

@michalsn
Copy link
Member

@crustamet There is nothing we can merge yet. I'm not sure if @MashinaMashina will try to work on this though. If you're motivated enough, you can prepare a PR for this, but we will need some tests too.

@MashinaMashina
Copy link
Contributor Author

@michalsn Do we have tests for Model.php?

@michalsn
Copy link
Member

michalsn commented Jul 1, 2020

@MashinaMashina
Copy link
Contributor Author

@michalsn I try to create tests, but it runs on sqllite, not postgres https://github.com/MashinaMashina/CodeIgniter4/tree/Model_without_autoincrement_key

Is good this?

@michalsn
Copy link
Member

michalsn commented Jul 2, 2020

@MashinaMashina This is the default behavior. You can change DBDriver via Config/Database.php or .env file.

If you will be ready, please send a PR so we can review your changes.

@MashinaMashina
Copy link
Contributor Author

@michalsn how do work postgres on tests? I dont know access to postgres on tests

@michalsn
Copy link
Member

michalsn commented Jul 3, 2020

@crustamet
Copy link
Contributor

crustamet commented Jul 26, 2020

I just found out the model without a primary key works as intended, the problem with update is a little bit weird how you need to update the row.

But this works only if you have one row in your table.

If you have multiple rows i don't see why you don't need a primary key.

Example

class TestModel extends Model
{
	protected $table      = 'test';

	protected $returnType = 'array';
	
	protected $allowedFields = [
		'test', 'test2'
	];
}

When you insert

$insert_arr['test'] = 'test';
$insert_arr['test2'] = 'test2';

new TestModel()->insert($insert_arr);

When you update

$update_arr['test'] = 'testtest';
$update_arr['test2'] = 'testtest2';

new TestModel()->where(1, 1)->set($update_arr)->update();

I think the update works even without the where(1, 1); but i didn't test this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature PRs for new features
Projects
None yet
Development

No branches or pull requests

3 participants