search

confluentinc / kafka-connect-jdbc

Kafka Connect connector for JDBC-compatible databases
Other
1.01k stars 953 forks source link

Option for not inserting primary keys into Sql Server tables #1343

Open miller45 opened 1 year ago

miller45 commented 1 year ago

Problem

Inserting values into IDENTITY columns on SQL Server (MS) will result in SQL Error: insert explicit value for identity column in table 'tablename' when IDENTITY_INSERT is set to OFF The are valid scenarios where you don´t want to (or cannot) alter the target tables of database just to "make kafka-connect happy". But you might need to include the primary key in you kafka-connect config because you have to include it for the upsert mode/UPDATE case. Related to #1287 and #837

Solution

Added option mssql.insert.primary.keys which defaults to true. If set to false the INSERT will omit the primary key columns. This allows allows the database to create its own primary keys, if they are configured with IDENTITY(n,n) for autoincremental keys. This works for mode=insert and mode=upsert

Does this solution apply anywhere else?
If yes, where?

Test Strategy

Testing done:

Release Plan

This feature is non-breaking by design. Given there is no bug, it will not change any existing behavior. So it can be released without any migration effort.

cla-assistant[bot] commented 11 months ago

CLA assistant check
All committers have signed the CLA.

miller45 commented 11 months ago

Problem

Inserting values into IDENTITY columns on SQL Server (MS) will result in SQL Error: insert explicit value for identity column in table 'tablename' when IDENTITY_INSERT is set to OFF There are valid scenarios where you don´t want to (or cannot) alter the target tables of database just to "make kafka-connect happy". But you might need to include the primary key in you kafka-connect config because you have to include it for the upsert mode/UPDATE case. Related to #1287 and #837

Solution

Added option mssql.insert.primary.keys which defaults to true. If set to false the INSERT will omit the primary key columns. This allows allows the database to create its own primary keys, if they are configured with IDENTITY(n,n) for autoincremental keys. This works for mode=insert and mode=upsert

Does this solution apply anywhere else?
  • [ ] yes
  • [x] no
If yes, where?

Test Strategy

Testing done:
  • [x] Unit tests
  • [ ] Integration tests
  • [x] System tests
  • [x] Manual tests

Release Plan

This feature is non-breaking by design. Given there is no bug, it will not change any existing behavior. So it can be released without any migration effort.

AnthonyWhitaker commented 4 months ago

@Tanish0019 Is it possible to get an ETA or other information on the likelihood of this getting merged? The changes look good to me and judging from StackOverflow and GitHub Issues - it would benefit a large number of people.

Tanish0019 commented 4 months ago

Hi @AnthonyWhitaker, yes looks like a valid issue. I will try to get to reviewing it soon.