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

Add RETURNING 1 to insertJob plan #100

Merged
merged 1 commit into from
Jan 11, 2019
Merged

Add RETURNING 1 to insertJob plan #100

merged 1 commit into from
Jan 11, 2019

Conversation

yammine
Copy link
Contributor

@yammine yammine commented Jan 9, 2019

Hey 👋 first of all thanks for this awesome library.
Turns out some of the logic that determines whether or not to return an id on publish requires that INSERT have some output.

I'm using typeorm(https://github.com/typeorm/typeorm/) and while I've gotten everything else to work when using the typeorm connection, this small hurdle is blocking me.

I've confirmed that this works after editing the plan locally.

Think we can roll this in?

Hey 👋 first of all thanks for this awesome library.
Turns out some of the logic that determines whether or not to return an `id` on publish requires that INSERT have some output.

I'm using `typeorm`(https://github.com/typeorm/typeorm/) and while I've gotten everything else to work when using the typeorm connection, this small hurdle is blocking me.

I've confirmed that this works after editing the plan locally.

Think we can roll this in?
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 096f0a6 on ChrisYammine:patch-1 into a8d2992 on timgit:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 096f0a6 on ChrisYammine:patch-1 into a8d2992 on timgit:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 096f0a6 on ChrisYammine:patch-1 into a8d2992 on timgit:master.

@timgit
Copy link
Owner

timgit commented Jan 10, 2019

Hey there! Thanks for the PR. The change looks innocent enough, and I may actually just switch it over to the inserted id instead of 1 while I'm at it. By the way, with typeorm, are you saying that typeorm won't allow you run an insert statement against pg without a returning? I didn't see any issues posted in their repo, and it does sound like quite an odd requirement since it's just executing arbitrary SQL. Can you tell me a bit more about why this fixed your issue?

@timgit timgit merged commit 3740756 into timgit:master Jan 11, 2019
@yammine
Copy link
Contributor Author

yammine commented Jan 13, 2019

👋 Hey @timgit
So when using a typeorm's connection query fun to execute SQL it appears as though INSERTs with no RETURNING statement simply returns an empty array (even though Postgres spits out something like INSERT oid count)

I imagine Sequelize/other ORMs correctly bubble up the inserted row count and therefore make it possible to use pg-boss without specifying a RETURNING for job insert

This pull request was closed.
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.

3 participants