feat(oracledb)!: Update to SDK V4 #11711
Conversation
erezrokah
requested review from
a team and
yevgenypats
and removed request for
a team
| sortResults(testTable, actualStrings) | ||
| sortResults(testTable, actualRecords) | ||
|
|
||
| for recordIndex, expectedRecord := range expectedRecords { |
There was a problem hiding this comment.
Notice we switched from string comparison to value comparison as everything is arrow types now
| *arrow.Uint8Type, *arrow.Uint16Type, *arrow.Uint32Type, *arrow.Uint64Type: | ||
| return "NUMBER(38)" | ||
| case *arrow.Float16Type, *arrow.Float32Type, *arrow.Float64Type: | ||
| case *arrow.Float16Type, *arrow.Float32Type: |
There was a problem hiding this comment.
This code is only used in the tests.
Previously the OracleDB types were not accurate enough and data was lost during insert. It was not discovered in the tests, due to doing string comparison
| return nil, err | ||
| } | ||
| return toTime(a.Value(i)), nil | ||
| t := toTime(a.Value(i)) |
There was a problem hiding this comment.
This code was also used only in the tests during insert.
Apparently if you use Go time.Time type to insert data using the Oracle driver it allows only up to seconds precision. To fix this we need to use the built in go_ora.TimeStamp, see:
With time.Time we reach here:
https://github.com/sijms/go-ora/blob/41ca433ee475e64869448c27e6e20ad038613ae4/v2/parameter_encode.go#L93
https://github.com/sijms/go-ora/blob/41ca433ee475e64869448c27e6e20ad038613ae4/v2/converters/type_conversion.go#L1117
And with go_ora.TimeStamp:
https://github.com/sijms/go-ora/blob/41ca433ee475e64869448c27e6e20ad038613ae4/v2/parameter_encode.go#L116
https://github.com/sijms/go-ora/blob/41ca433ee475e64869448c27e6e20ad038613ae4/v2/parameter_encode.go#L116
|
Marked this as blocked since it's missing the concurrency option |
erezrokah
force-pushed
the
fix/update_oracledb_source_sdk_v4
branch
4 times, most recently
from
90f4982 to
9a1959f
Compare
erezrokah
changed the title
erezrokah
force-pushed
the
fix/update_oracledb_source_sdk_v4
branch
from
64d5544 to
bd3930b
Compare
#11766 should unblock this, but this update is still a breaking change |
| group, gctx := errgroup.WithContext(ctx) | ||
| group.SetLimit(int(c.Concurrency)) | ||
| for _, table := range c.Tables { | ||
| if err := c.syncTable(gctx, table, res); err != nil { |
There was a problem hiding this comment.
We didn't really use the error group here 🤦
| func (*Spec) SetDefaults() { | ||
| func (s *Spec) SetDefaults() { | ||
| if s.Concurrency == 0 { | ||
| s.Concurrency = 100 |
There was a problem hiding this comment.
This is a more sensible default than the SDK one
erezrokah
force-pushed
the
fix/update_oracledb_source_sdk_v4
branch
from
bd3930b to
3e60164
Compare
erezrokah
force-pushed
the
fix/update_oracledb_source_sdk_v4
branch
from
6691469 to
957e8df
Compare
erezrokah
added
automerge
#### Summary I missed this in #11711. Without this fix we will always migrate all tables. The sync still works on the filtered tables, so this only impacts migration <!--
3 participants
Summary
Same as #11696
Fixes #11757
BEGIN_COMMIT_OVERRIDE
feat!: Upgrades the oracledb source plugin to use plugin-sdk v4. This version does not contain any user-facing breaking changes, but because it is now using CloudQuery gRPC protocol v3, it does require use of a destination plugin that also supports protocol v3. All recent destination plugin versions support this.
END_COMMIT_OVERRIDE