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

[ISSUE-154] Support Array[Tuple] clickhouse type #155

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

qingfei1994
Copy link
Contributor

@qingfei1994 qingfei1994 commented Sep 2, 2024

Why are the changes needed?

Close #154 .

Brief change log

  • support Row type during serialization and deserialization

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Test case:
Clickhouse DDL

set flatten_nested=0;
create table test_tuple3(arr Array(Tuple(a Int32, b Int32, c Int32)), z Int32) Engine=MergeTree() Order By tuple();

Flink SQL

 CREATE TABLE test_tuple (
    arr Array<Row<a int,b int,c int>>,
    z int
  ) WITH (
    'connector' = 'clickhouse',
    'url' = '......',
    'database-name' = 'default',
    'table-name' = 'test_tuple3',
  .....
  );
  insert into test_tuple values(Array[(1,2,3)],700);

Query result

SELECT *
FROM test_tuple3
WHERE z = 700
   ┌─arr───────┬───z─┐
1. │ [(1,2,3)] │ 700 │
   └───────────┴─────┘

1 row in set. Elapsed: 0.004 sec.

Documentation

  • Does this pull request introduce a new feature? (yes / no) no
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@czy006 czy006 requested review from czy006 and itinycheng September 2, 2024 08:22
Copy link
Owner

@itinycheng itinycheng left a comment

Choose a reason for hiding this comment

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

Hi @qingfei1994

Good PR, but it will be better if it works well with ClickHouseCatalog.
Run the command ./mvnw spotless:apply before submitting can help us to ensure the code format is correct.

@qingfei1994
Copy link
Contributor Author

Hi @qingfei1994

Good PR, but it will be better if it works well with ClickHouseCatalog. Run the command ./mvnw spotless:apply before submitting can help us to ensure the code format is correct.

Sure I've fix the checkstyle violation, and will also check if it works with ClickhouseCatalog

@qingfei1994
Copy link
Contributor Author

Hi @qingfei1994
Good PR, but it will be better if it works well with ClickHouseCatalog. Run the command ./mvnw spotless:apply before submitting can help us to ensure the code format is correct.

Sure I've fix the checkstyle violation, and will also check if it works with ClickhouseCatalog

@itinycheng @czy006
I've tested and fixed the ClickhouseCatalog with Row data type.
Flink SQL:

CREATE catalog clickhouse_catalog WITH (
  'type' = 'clickhouse',
  'url' = '.....',
  'database-name' = 'test',
  'username' = '',
  'password' = ''
);

create table print_sink
(
    arr Array<Row<a int,b int, c int>>,
    z int
) with (
'connector'='print'
);

insert into print_sink
select
  arr,
  z
from
  clickhouse_catalog.`default`.test_tuple3;

query result:

2> +I[[+I[1, 2, 3]], 600]
2> +I[[+I[1, 2, 3]], 600]
2> +I[[+I[4, 5, 6]], 800]
2> +I[[+I[4, 5, 6]], 800]
2> +I[[+I[1, 2, 3]], 700]

please check the latest submitted code.

@itinycheng itinycheng merged commit 165a969 into itinycheng:master Sep 12, 2024
2 checks passed
@itinycheng
Copy link
Owner

Hi @qingfei1994

I have merged this PR.
One more thing, tuple elements without names are not included in
the test cases above, like ‘Tuple(T1, T2, ...)’

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Flink Logical Type ROW is not supported for Array<Tuple> clickhouse type
3 participants