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

Upsert 3.0.0 #117

Open
4 of 6 tasks
pnomolos opened this issue Jun 6, 2019 · 15 comments
Open
4 of 6 tasks

Upsert 3.0.0 #117

pnomolos opened this issue Jun 6, 2019 · 15 comments
Assignees
Milestone

Comments

@pnomolos
Copy link
Collaborator

pnomolos commented Jun 6, 2019

Meta-issue to track everything for the Upsert 3.0.0 release.

@pnomolos pnomolos self-assigned this Jun 6, 2019
@pnomolos pnomolos added this to the 3.0.0 milestone Jun 17, 2019
@pnomolos
Copy link
Collaborator Author

pnomolos commented Jun 17, 2019

To anyone using this gem, I'm trying to limit 3.0.0 to getting the gem working on modern versions of Ruby/JRuby and pulling in a few outstanding API-breaking tickets (As well as a few documentation updates).

Is there anything else that would be a breaking change and is a major blocker that you'd like to see in 3.0.0?

/cc @seamusabshere @olivierlacan @KevinColemanInc @searls

@KevinColemanInc
Copy link
Contributor

I am currently not using the gem, so I don't have any input to give.

@pnomolos
Copy link
Collaborator Author

pnomolos commented Jun 17, 2019

I am currently not using the gem, so I don't have any input to give.

@KevinColemanInc Anything that's blocking you from using it that could be added in a future update, or do you just not need it currently?

@searls
Copy link
Contributor

searls commented Jun 17, 2019

Other than getting working on the latest everythings, my only feature request would be some way to remove old/unnecessary upsert-generated pgsql functions. I have a bunch of these lingering in my production database even though I've since adjusted things such that they're not being needed

@pnomolos
Copy link
Collaborator Author

Other than getting working on the latest everythings, my only feature request would be some way to remove old/unnecessary upsert-generated pgsql functions. I have a bunch of these lingering in my production database even though I've since adjusted things such that they're not being needed

@searls Would a note in the README suffice, or do you think it should be built in to the library? I currently do the following:

    Upsert::MergeFunction::NAME_PREFIX = "upsert"
    DB.synchronize do |conn|
      Upsert.clear_database_functions(conn)
    end

@searls
Copy link
Contributor

searls commented Jun 17, 2019

Honestly, whether it cleans up after itself automatically or you just document it—entirely up to you! Literally the only pain/friction I can point to

@KevinColemanInc
Copy link
Contributor

KevinColemanInc commented Jun 17, 2019

do you just not need it currently?

I just don't need it currently

@searls
Copy link
Contributor

searls commented Jul 3, 2019

Found a thing I'd love in a new version! #120

@searls
Copy link
Contributor

searls commented Aug 19, 2019

Mind cutting a release with where things are? I've been running off a github fork for the last month

@pnomolos
Copy link
Collaborator Author

@searls I’ll be wrapping this up and cutting a release when I’m back in a couple weeks. Sorry for the big delay!

@pnomolos
Copy link
Collaborator Author

@searls @olivierlacan I've cut a 2.9.9 release that's effectively 3.0.0 except there's one small bug with jruby that I'm still ironing out.

@sgeef
Copy link

sgeef commented Mar 18, 2020

@pnomolos I'm unsure on the exact details of the issue I'm seeing, but currently investigating it.

I've upgraded from v2.2.1 to the current latest. and started having issues with a not nullable field, which is not in the setter nor on the selecter and before the update didn't break.. I'll keep you posted, while I investigate, but would you have any idea where this might be related too?

Note, we are forced to get the latest versions to get to v12 off postgres.

@pnomolos
Copy link
Collaborator Author

@sgeef Have you cleared all your upsert functions since the update? I use the following for Sequel:

Upsert::MergeFunction::NAME_PREFIX = "upsert"
DB.synchronize do |conn|
  Upsert.clear_database_functions(conn)
end

run this in a one-off, the big takeaway is to redefine the merge function prefix so it clears everything.

@sgeef
Copy link

sgeef commented Mar 19, 2020

@pnomolos this doesn't seem to be the issue. as our test suite doesn't have the functions by default and are re-generated by default.. So I guess there is another breaking changing, somewhere since v2.2.1

After debugging both the old version and the new version I've figured that the v2.2.1 returned on the use_pg_native? method false before and now started returning true.

Which was related to the unique_index_on_selector?, which before seems to have had a bug (#111). At least now it returns correctly that it has a unique index.

But we now are not based on functions anymore, but straight on the pg_native approach. Which, if it has a not-null field on the insert, not having a value and not being provided, off course will fail.

I'll try to fix this on our side, by providing the correct default value to the non-null column. Thanks for the tip on the clearing of the functions.

@sgeef
Copy link

sgeef commented Mar 20, 2020

Just to leave this closed @pnomolos, thanks for helping out. it turned out we only need updates, which explained why it didn't ever fail with the function.. It seems though that the native approach fails if you lack a non-nullable field.. as the insert would never be able to execute. (Solved!)

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

No branches or pull requests

4 participants