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

[bug fix] fix build sql error in ClickHouseDynamicTableSource #65

Closed

Conversation

Tan-JiaLiang
Copy link

simlar to FLINK-27268, when projection not match any column, it will return 'ROW<> NOT NULL'.

@itinycheng
Copy link
Owner

Hi, @Tan-JiaLiang

  1. Code format error found during the code compilation;
  2. Is there any specific scenario for this generated SQL? If this SQL is meaningless, does directly reporting an exception seem more appropriate?

@Tan-JiaLiang
Copy link
Author

Hi, @itinycheng

  1. fixed;
  2. I think it was the flink projection bug, when i try the SQL 'SELECT count(*) from ${table}', projection will be return a ROW<> NOT NULL, but i reference to the mysql connector and hive connector, like FLINK-27268, it transform to an empty string. So i think it may need to be fix in connector self.

@Tan-JiaLiang
Copy link
Author

By the way, before this bugfix, the "SELECT count(*) FROM ${table}" transform to the operator will be like this:
"SELECT FROM ${table}"(source operator which transform to the clickhouse grammar) -> "count operator" -> "collect operator".

after this, transform will be like this:
"SELECT '' FROM ${table}" -> "count operator" -> "collect operator".

@itinycheng
Copy link
Owner

By the way, before this bugfix, the "SELECT count(*) FROM ${table}" transform to the operator will be like this: "SELECT FROM ${table}"(source operator which transform to the clickhouse grammar) -> "count operator" -> "collect operator".

after this, transform will be like this: "SELECT '' FROM ${table}" -> "count operator" -> "collect operator".

@Tan-JiaLiang
Yes, I've learned about the issue.
In my mind, it more reasonable to add a Rule for the SQL Optimizer(VolcanoPlanner) to slove the parse error of count(*), if we only replace empty with '' in connector it may cause more problems in the future, because we don't know the actual reason why selectFields is empty.

@Tan-JiaLiang
Copy link
Author

By the way, before this bugfix, the "SELECT count(*) FROM ${table}" transform to the operator will be like this: "SELECT FROM ${table}"(source operator which transform to the clickhouse grammar) -> "count operator" -> "collect operator".
after this, transform will be like this: "SELECT '' FROM ${table}" -> "count operator" -> "collect operator".

@Tan-JiaLiang Yes, I've learned about the issue. In my mind, it more reasonable to add a Rule for the SQL Optimizer(VolcanoPlanner) to slove the parse error of count(*), if we only replace empty with '' in connector it may cause more problems in the future, because we don't know the actual reason why selectFields is empty.

I agree with you. I had reported this bug to the flink official, and it had been fixed in 1.17.0. See FLINK-29558.

@itinycheng
Copy link
Owner

@Tan-JiaLiang Got it, thank u.

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.

None yet

2 participants