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

fix: support insert ... on conflict...update for reWriteBatchedInserts=true #1130

Merged
merged 3 commits into from
Mar 8, 2018

Conversation

vlsi
Copy link
Member

@vlsi vlsi commented Feb 18, 2018

pgjdbc will avoid rewriting the query if a bind is identified after values(...)

That is ON CONFLICT... DO update set x=? is NOT rewrite-compatible, and update set x='default' is rewrite-compatible

"reported" here: https://stackoverflow.com/questions/47664889/jdbc-batch-operations-understanding/48349524?noredirect=1#comment84691562_48349524

…s=true

pgjdbc will avoid rewriting the query if a bind is identified after values(...)

That is ON CONFLICT... DO update set x=? is NOT rewrite-compatible, and update set x='default' is rewrite-compatible

"reported" here: https://stackoverflow.com/questions/47664889/jdbc-batch-operations-understanding/48349524?noredirect=1#comment84691562_48349524
@vlsi vlsi added this to the 42.2.2 milestone Feb 18, 2018
@codecov-io
Copy link

Codecov Report

Merging #1130 into master will increase coverage by 0.03%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master    #1130      +/-   ##
============================================
+ Coverage     67.25%   67.28%   +0.03%     
- Complexity     3673     3680       +7     
============================================
  Files           170      170              
  Lines         15640    15643       +3     
  Branches       2531     2533       +2     
============================================
+ Hits          10519    10526       +7     
+ Misses         3932     3928       -4     
  Partials       1189     1189

Copy link

@Daenyth Daenyth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really appreciate getting this in - it would be a huge benefit for us.

ps.setString(2, "53");
ps.addBatch();
int[] actual = ps.executeBatch();
BatchExecuteTest.assertSimpleInsertBatch(2, actual);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to test that it executed only a single statement? The fact that this asserts the same as the above test makes me feel uncertain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Daenyth
Copy link

Daenyth commented Feb 23, 2018

I believe this fixes #783

@Daenyth
Copy link

Daenyth commented Mar 4, 2018

Is there anything I could do to help speed this branch along?

@vlsi vlsi changed the title fix: support insert ... on conflict...update for reWriteBatchedInsert…s=true fix: support insert ... on conflict...update for reWriteBatchedInserts=true Mar 8, 2018
@vlsi vlsi merged commit 1ca0c58 into pgjdbc:master Mar 8, 2018
@Daenyth
Copy link

Daenyth commented May 10, 2018

I notice there's no tests in here that cover having a WHERE clause in the ON CONFLICT section.

I haven't had time to test if it works; do you think it would?

@vlsi
Copy link
Member Author

vlsi commented May 10, 2018

@Daenyth , I think it would, however more tests would definitely make sense there

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.

None yet

3 participants