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

Support multi-row insert on SQL builder class #1333

Merged
merged 14 commits into from Jun 29, 2019

Conversation

@sivdead
Copy link

commented Aug 23, 2018

make AbstratSQL.INSERT_VALUES could be invoke multiple times to do batch insert.

@harawata

This comment has been minimized.

Copy link
Member

commented Sep 30, 2018

Thank you for the PR, @sivdead !

It could break backward compatibility, though.
Please see the following test case. It will fail if this PR is merged.

  @Test
  public void variableLengthArgumentOnIntoColumnsAndValues() {
    final String sql = new SQL() {{
      INSERT_INTO("TABLE_A").INTO_COLUMNS("a", "b").INTO_VALUES("#{a}").INTO_VALUES("#{b}");
    }}.toString();
    assertEquals("INSERT INTO TABLE_A\n (a, b)\nVALUES (#{a}, #{b})", sql);
  }

I'm not sure what's the best approach here, but it might be better to add a new method instead of modifying INTO_VALUES.

Cc-ing @h3adache @kazuki43zoo

@kazuki43zoo

This comment has been minimized.

Copy link
Member

commented Sep 30, 2018

it might be better to add a new method instead of modifying INTO_VALUES.

I agree with it.

For example, one approach is as follow:

  @Test
  public void multipleRowsInsert() {
    final String sql = new SQL() {{
      INSERT_INTO("TABLE_A")
          .INTO_COLUMNS("a", "b")
          .INTO_VALUES("#{a1}", "#{b1}")
          .ADD_ROW()
          .INTO_VALUES("#{a2}", "#{b2}")
      ;
    }}.toString();

    assertEquals("INSERT INTO TABLE_A\n (a, b)\nVALUES \n (#{a1}, #{b1}),\n (#{a2}, #{b2})", sql);
  }

NOTE: Multiple rows insert style is different by database vender and versions.

@sivdead

This comment has been minimized.

Copy link
Author

commented Oct 17, 2018

it might be better to add a new method instead of modifying INTO_VALUES.

I agree with it.

For example, one approach is as follow:

  @Test
  public void multipleRowsInsert() {
    final String sql = new SQL() {{
      INSERT_INTO("TABLE_A")
          .INTO_COLUMNS("a", "b")
          .INTO_VALUES("#{a1}", "#{b1}")
          .ADD_ROW()
          .INTO_VALUES("#{a2}", "#{b2}")
      ;
    }}.toString();

    assertEquals("INSERT INTO TABLE_A\n (a, b)\nVALUES \n (#{a1}, #{b1}),\n (#{a2}, #{b2})", sql);
  }

NOTE: Multiple rows insert style is different by database vender and versions.

thanks for the advice,so I add a method ADD_ROW to avoid that multi INTO_VALUES in single insert problem.
For example:

  @Test
  public void batchInsertWithMultipleInsertValues(){
    final String sql = new SQL() {{
      INSERT_INTO("TABLE_A");
      INTO_COLUMNS("a", "b");
      INTO_VALUES("#{a1}");
      INTO_VALUES("#{b1}");
      ADD_ROW();
      INTO_VALUES("#{a2}");
      INTO_VALUES("#{b2}");
    }}.toString();
    System.out.println(sql);
    assertEquals("INSERT INTO TABLE_A\n (a, b)\nVALUES \n (#{a1}, #{b1}),\n (#{a2}, #{b2})", sql);
  }

I see that batch insert in oracle is different than others,but I don't know how to get database type or version in sqlBuilder-,-

luo.qianhong
@harawata

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

Thank you for the update, @sivdead !

@kazuki43zoo ,
Does ADD_ROW() look OK to you?
According to Wikipedia, this syntax is the standard one, so I personally think it's OK even if it does not work with all DBs.

I may request a few minor code changes later.

@harawata harawata changed the title support batch insert in AbstractSQL Add multi-row insert support to SQL builder Oct 18, 2018

@kazuki43zoo

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

Hi @harawata , sorry for late reply.

Does ADD_ROW() look OK to you?

I think it is OK.

According to Wikipedia, this syntax is the standard one, so I personally think it's OK even if it does not work with all DBs.

I thinks so, too!

@harawata
Copy link
Member

left a comment

Hi @sivdead ,
I'm sorry about the delay! There were some high priority issues.
I have added a few comments.
Please take a look when you have time.

@kazuki43zoo ,
Thanks! Feel free to add comments if you noticed anything.

@sivdead

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

Hi @sivdead ,
I'm sorry about the delay! There were some high priority issues.
I have added a few comments.
Please take a look when you have time.

@kazuki43zoo ,
Thanks! Feel free to add comments if you noticed anything.

Hi @sivdead ,
I'm sorry about the delay! There were some high priority issues.
I have added a few comments.
Please take a look when you have time.

@kazuki43zoo ,
Thanks! Feel free to add comments if you noticed anything.

thanks for correcting my mistake! and also sorry about the delay reply :(

luo.qianhong and others added some commits Jun 21, 2019

Merge branch 'master' into gh/1333-sql-multirow-insert
# Conflicts:
#	src/main/java/org/apache/ibatis/jdbc/AbstractSQL.java
#	src/test/java/org/apache/ibatis/jdbc/SQLTest.java
@harawata

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Thank you for the update, @sivdead !
I have resolved the conflict and made a few minor adjustment.

@kazuki43zoo ,
Could you do the final check and merge if everything is OK?
Thanks in advance!

@kazuki43zoo
Copy link
Member

left a comment

@sivdead

I've added some small comments. If you have a time, please check it.

@kazuki43zoo kazuki43zoo added this to the 3.5.2 milestone Jun 22, 2019

luo.qianhong added some commits Jun 23, 2019

luo.qianhong
Merge remote-tracking branch 'origin/master'
# Conflicts:
#	src/main/java/org/apache/ibatis/jdbc/AbstractSQL.java
@sivdead

This comment has been minimized.

Copy link
Author

commented Jun 23, 2019

@sivdead

I've added some small comments. If you have a time, please check it.

i have do as you suggested,again thank you for helping me a lot on this pr :)

@kazuki43zoo
Copy link
Member

left a comment

@sivdead Thanks for your rework! But I have some change requests. Please check new comment and replying comments.

luo.qianhong
@sivdead

This comment has been minimized.

Copy link
Author

commented Jun 26, 2019

@sivdead Thanks for your rework! But I have some change requests. Please check new comment and replying comments.

get it fixed

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this pull request Jun 29, 2019

@kazuki43zoo kazuki43zoo merged commit 7476718 into mybatis:master Jun 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kazuki43zoo kazuki43zoo self-assigned this Jun 29, 2019

kazuki43zoo added a commit that referenced this pull request Jun 29, 2019

@kazuki43zoo kazuki43zoo changed the title Add multi-row insert support to SQL builder Support multi-row insert on SQL builder class Jul 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.