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

Assumption of PostgreSQL Primary Key Sequence name is incorrect #1069

Closed
fukawi2 opened this issue Oct 4, 2017 · 5 comments
Closed

Assumption of PostgreSQL Primary Key Sequence name is incorrect #1069

fukawi2 opened this issue Oct 4, 2017 · 5 comments

Comments

@fukawi2
Copy link

fukawi2 commented Oct 4, 2017

See issue #124. Also present here: https://github.com/bcosca/fatfree/blob/master/lib/db/sql/mapper.php#L433

Issue #124 states:

The default seq object name syntax is "table_pkfield_seq" i.e.: "article_id_seq"

This assumption is not safe. In my case, I have an inherited table msgs_master which is inherited into msgs_current and msgs_archive. New rows are inserted to msgs_current, while a maintenance script moves rows older than a given time frame to msgs_archive. The primary key for all columns is called mid

Because msgs_current and msgs_archive inherit their columns from msgs_master, the sequence is called msgs_master_mid_seq for all 3 tables. The above referenced code presumes the sequence includes the name of the table you're inserting to, but when inserting to msgs_current the sequence msgs_master_mid_seq will be used.

IMPACT

This breaks the save() function because when it tries to reload the record, it cannot determine the correct value for mid for the select.

Perhaps the RETURNING feature of PostgreSQL INSERT could be used somehow:

INSERT INTO foobar (col1, col2, col3) VALUES (1, 2, 3) RETURNING mid;

This functionality was introduced in PG 8.2, which means it is in all supported versions of PostgreSQL.

@ikkez
Copy link
Collaborator

ikkez commented Oct 26, 2017

Hi @fukawi2
Could you please test if the fix in f3-factory/fatfree-core#230 solves the issue? thanks a lot.

ikkez added a commit to f3-factory/fatfree-core that referenced this issue Oct 28, 2017
@ikkez ikkez closed this as completed Oct 28, 2017
@fukawi2
Copy link
Author

fukawi2 commented Oct 30, 2017

Yes, that works perfectly in my testing. Thanks! :)

@ghost
Copy link

ghost commented Feb 22, 2018

This change (or "fix") has introduced a new, VERY serious problem, which didn't exist when using sequences.

Now it is NOT possible to insert into a view (which in turn uses a trigger for update of multiple tables related to the view), because the $aik is empty.

This produces a 500 error.

PDOStatement: ERROR: zero-length delimited identifier at or near """" LINE 1:
...x2","x3","x4") VALUES ($1,$2,$3,$4) RETURNING "" ^

Therefore this change is indeed very contra-productive.

ikkez added a commit to f3-factory/fatfree-core that referenced this issue Feb 23, 2018
@ikkez
Copy link
Collaborator

ikkez commented Feb 23, 2018

fixed in the latest commit.

@ghost
Copy link

ghost commented Mar 6, 2018

@ikkez Thank you.

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

No branches or pull requests

2 participants