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

[Hotfix][connector-v2][clickhouse] Fixed an out-of-order BUG with output data fields of clickhouse-sink #5346

Merged

Conversation

wowzx
Copy link
Contributor

@wowzx wowzx commented Aug 21, 2023

If the order of the output data fields is not in accordance with the order of the table_schema fields, the output fields are out of order.
This is caused by the array of fields returned with JdbcBatchStatementExecutorBuilder#getDefaultProjectionFields being sorted in table_schema field order
See: #5176

Purpose of this pull request

Check list

@wowzx wowzx closed this Aug 21, 2023
@wowzx wowzx reopened this Aug 21, 2023
@liugddx liugddx added the First-time contributor First-time contributor label Aug 21, 2023
Copy link
Member

@liugddx liugddx left a comment

Choose a reason for hiding this comment

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

LGTM . @Hisoka-X PTAL

liugddx
liugddx previously approved these changes Aug 21, 2023
@liugddx
Copy link
Member

liugddx commented Aug 21, 2023

Please fix CI error. Run mvn spotless:apply

@wowzx
Copy link
Contributor Author

wowzx commented Aug 22, 2023

Please fix CI error. Run mvn spotless:apply

Fixed, please check it

@Hisoka-X
Copy link
Member

Could you add a test case for this bug fixed? To make sure this bug will not regression.

@Hisoka-X Hisoka-X added this to the 2.3.4 milestone Aug 22, 2023
@wowzx
Copy link
Contributor Author

wowzx commented Aug 22, 2023

Could you add a test case for this bug fixed? To make sure this bug will not regression.

create sql

create table test(
     `field1`          String,
     `field2`          String,
     `field3`          String,
     `field4`          String
)engine=Memory;

config file

env {
  execution.parallelism = 1
  job.mode = "BATCH"
}
source {
  FakeSource {
    source_table_name = "fake"
    schema = {
      fields {
        field1 = string
        field2 = string
        field3 = string
        field4 = string
      }
    }
    rows = [
      {
        kind = INSERT
        fields = ["v_1", "v_2", "v_3","v_4"]
      }
    ]
  }
}
transform {
  FieldMapper {
    source_table_name = "fake"
    result_table_name = "test"
    field_mapper = {
        field2 = field2
        field4 = field4
        field3 = field3
        field1 = field1
    }
  }
}
sink {
  Clickhouse {
    source_table_name = "test"
    host = "127.0.0.1:8123"
    database = "Test"
    table = "test"
    username = "default"
    password = ""
    bulk_size = 1
  }
}

before fix
image

after fix
image

@Hisoka-X
Copy link
Member

@wowzx Could you add UT for this? Thanks.

Copy link

@ZhaoRuidong ZhaoRuidong left a comment

Choose a reason for hiding this comment

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

aggre

@hailin0
Copy link
Member

hailin0 commented Oct 25, 2023

Does anyone want to help add testcase?

@EricJoy2048 EricJoy2048 merged commit fce9dda into apache:dev Oct 26, 2023
4chicat pushed a commit to nexr/incubator-seatunnel that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants