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

State manager failes when gets value with \u0000 #13814

Closed
DoNotPanicUA opened this issue Jun 15, 2022 · 3 comments · Fixed by #13854 or #13887
Closed

State manager failes when gets value with \u0000 #13814

DoNotPanicUA opened this issue Jun 15, 2022 · 3 comments · Fixed by #13854 or #13887
Assignees
Labels
needs-triage type/bug Something isn't working

Comments

@DoNotPanicUA
Copy link
Contributor

DoNotPanicUA commented Jun 15, 2022

Current Behavior

If we use a binary attribute as a cursor for the incremental sync and a source provides a value that contains \u0000, the sync operation fails.
This is a typical issue for any Postgres DB. (#3476)

Expected Behavior

The state manager should not fail in any case.

Logs

logs-7.txt

Example of bad value:
Screenshot from 2022-06-15 15-54-09

Steps to Reproduce

  1. Create MsSql source.
    1.1 Create a table with a binary(10) field
    1.2 Insert value shorter than 10. (MsSql populates missing chars by \u0000)
  2. Setup a connector to any destination using the binary field for the Incremental sync
  3. 💣
@DoNotPanicUA
Copy link
Contributor Author

Hi @jstettnerbc,
I did a small investigation inside the Airbyte. I said that it might be unsafe to use binary fields as cursors. Now I can confidently say that binary fields are not supported as a cursor so far.
While we process data, we are looking for a maximal cursor value by comparing them. Right now we compare binary values as Strings.

@DoNotPanicUA DoNotPanicUA self-assigned this Jun 16, 2022
@jstettnerbc
Copy link

@DoNotPanicUA thanks for checking a level deeper!
So maybe we could add a Hint to the MSSQL connector that using rowversion[1] as cursor-field ist not possible?

[1] https://docs.microsoft.com/en-us/sql/t-sql/data-types/rowversion-transact-sql?view=sql-server-ver16

@DoNotPanicUA
Copy link
Contributor Author

Hi @jstettnerbc,
I've released MsSql source version 0.4.3, which prevents technical failure, but the principal issue of using binary attributes still exists.
To handle this issue properly, I've created issue airbytehq/airbyte-internal-issues#682 and put it to scoping candidate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage type/bug Something isn't working
Projects
None yet
3 participants