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

Support multiple planner output dialects #24

Closed
ryannedolan opened this issue May 31, 2023 · 11 comments
Closed

Support multiple planner output dialects #24

ryannedolan opened this issue May 31, 2023 · 11 comments
Labels
good first issue Good for newcomers

Comments

@ryannedolan
Copy link
Collaborator

The planner is currently hard-coded to output MySQL dialect, which Flink natively understands. In order to target additional runtimes, we need a way to generate SQL in other dialects. N.B. the planner is largely dialect-agnostic already, until you ask for the plan as a String.

@ryannedolan ryannedolan added the good first issue Good for newcomers label May 31, 2023
@HoganEdwardChu
Copy link
Contributor

Hi, I didn't mean to disturb you, I was watching with interest. The line that you mean is here?

SqlDialect OUTPUT_DIALECT = MysqlSqlDialect.DEFAULT; // closely resembles Flink SQL

@ryannedolan
Copy link
Collaborator Author

The line that you mean is here?

Yep. It looks like the hard-coded dialect is only used in a couple methods that return SQL strings. It might make sense to change those to return the underlying ScriptImplementer, which is dialect agnostic. Then callers could do rel.query(...).sql(dialect) and get whatever dialect they want.

@HoganEdwardChu
Copy link
Contributor

HoganEdwardChu commented Jun 5, 2023

Oh you mean https://github.com/HoganEdwardChu/Hoptimator/pull/1/files this way? I haven't seen all the code, so I don't know the specific details, so it might seem lacking(sorry for that) But than i think we have to get input dialect.

@ryannedolan
Copy link
Collaborator Author

this way?

Exactly, tho I'm not sure where to go from there. One way would be to add a dialect parameter to pipeline(), s.t. the operator can generate pipelines in whatever dialect it wants. For now, the operator can just use the MySQL dialect.

Looking ahead, we'd like to be able to implement alternative runtimes, e.g. using Spark SQL instead of Flink SQL. Ideally, those changes would be in the operator, not the planner. So this would be a step in that direction.

@HoganEdwardChu
Copy link
Contributor

HoganEdwardChu commented Jun 5, 2023

Thanks for the detailed and contextual explanation. I'll think about what you said too. (if there's a good direction, I'll try it and share it again too!) Also would try implementing alternative runtimes Spark SQL instead of Flink SQL.(operator part)

@ryannedolan
Copy link
Collaborator Author

I haven't given much thought to how we'd support multiple runtimes in the operator. There is a flink adapter, which is really the only place in the control plane that we assume the SQL will run on flink: https://github.com/linkedin/Hoptimator/blob/main/hoptimator-flink-adapter/src/main/java/com/linkedin/hoptimator/operator/flink/FlinkSqlJobReconciler.java#L57

As you can see, the flink adapter just passes the SQL from SqlJob to a FlinkDeployment (which is handled by the external flink-kubernetes-operator). Theoretically, we could do something similar for Spark or any other SQL runtime. I'm not sure how valuable that would be, but it seems possible.

@HoganEdwardChu
Copy link
Contributor

I guess I'll have to think about it a bit. If you have any good opinions on whether it is better to put a dialect in the pipeline, please proceed. I will continue to try. I think making another adapter should be postponed to a later date. I think the most important thing is to handle the flink job well.

@HoganEdwardChu
Copy link
Contributor

HoganEdwardChu commented Jun 7, 2023

@ryannedolan
Copy link
Collaborator Author

adding sqlDialect to pipeline

Makes sense to me!

@ryannedolan
Copy link
Collaborator Author

Thanks @HoganEdwardChu for #31

@HoganEdwardChu
Copy link
Contributor

Thanks!!!!!

I am trying to add another sql time like spark. If there is any valuable progress I will pr! @ryannedolan

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

No branches or pull requests

2 participants