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

Skip generated columns in copy_rows method #41

Merged
merged 2 commits into from
Mar 15, 2023

Conversation

Ceda
Copy link
Contributor

@Ceda Ceda commented Feb 22, 2023

I have same issue like #40 this PR detect generated columns from schema and prevent inserting.
Is working for Postgres, same issue for MySQL here #28

@bricct
Copy link
Contributor

bricct commented Mar 9, 2023

This looks good except for one small problem and that is that some Identity columns can be generated.

There are two cases for generated identity columns
GENERATED BY DEFAULT and GENERATED ALWAYS
The latter of which causes problems by making insert statements fail.

In the case that an IDENTITY column is GENERATED BY DEFAULT the column should simply be inserted as normal.

In the case that an IDENTITY column is GENERATED ALWAYS the insert statement should have OVERRIDING SYSTEM VALUE following the column list e.g.

INSERT INTO user 
     (id, name, password) 
OVERRIDING SYSTEM VALUE 
VALUES
      (1, 'user', 'password123');

An identity columns' generated status can be discerned via the pg_attribute.attidentity field.
This field can be one of three values:

  • the zero byte '' by default,
  • 'a' if it is GENERATED ALWAYS
  • or 'd' if it is GENERATED BY DEFAULT

I'll do my best to include these changes in my free-time but feel free to take care of it yourself as well. Thank you!

P.S. here's some relevant links to this problem Stack Overflow, PG Docs

@bricct bricct self-assigned this Mar 9, 2023
@bricct
Copy link
Contributor

bricct commented Mar 9, 2023

I have made a PR off of your PR that will add the requested changes that I commented. Once you merge that PR, I can merge this one as well. Thanks!

Link to PR

commit dc03868
Merge: adfe83b 5f380a3
Author: Trey Briccetti <[email protected]>
Date:   Wed Mar 8 19:17:52 2023 -0800

    Merge pull request TonicAI#2 from bricct/override-generated-ids

    override generated ids when necessary PG

commit 5f380a3
Merge: 0792980 adfe83b
Author: Trey Briccetti <[email protected]>
Date:   Wed Mar 8 19:17:35 2023 -0800

    Merge branch 'master' into override-generated-ids

commit adfe83b
Merge: 4dafc49 cf32d30
Author: Trey Briccetti <[email protected]>
Date:   Wed Mar 8 19:16:15 2023 -0800

    Merge pull request #1 from Ceda/skip-generated-columns

    Skip generated columns in copy_rows method

commit 0792980
Author: Trey Briccetti <[email protected]>
Date:   Wed Mar 8 19:11:28 2023 -0800

    override generated ids when necessary PG
@Ceda
Copy link
Contributor Author

Ceda commented Mar 15, 2023

I have made a PR off of your PR that will add the requested changes that I commented. Once you merge that PR, I can merge this one as well. Thanks!

Link to PR

Nice work, thank you... I squashed your code into my branch...

@bricct
Copy link
Contributor

bricct commented Mar 15, 2023

Thank you @Ceda !

@bricct bricct merged commit 6a222e7 into TonicAI:master Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants