twalthr commented on issue #9874: [FLINK-14240][table] Merge table config parameters(TableConfig#getConfiguration) into global job parameters(ExecutionConfig#getGlobalJobParameters) when running with the legacy planner. URL: https://github.com/apache/flink/pull/9874#issuecomment-542715175 Hi @dianfu, sorry if my `-1` was too direct. It should neither offend you nor the contributor. It is simply an expression that the current status of the PR cannot be merged due to the issues that I mentioned. FLIP-59 is still under discussion because it takes longer than expected to update the ConfigOption infrastructure. It is still targeted for 1.10. As you might have seen on the mailing list, we still try to reach consensus on FLIP-54 and split it into multiple parts such as FLIP-77. The solution in this PR makes it difficult to validate the configuration in the future as it encourages users to add arbitrary keys to the table config. We should clearly separate `global job configuration` (which are arbtirary user-defined properties available to functions) and `configuration` (which are Flink-defined properties available to Flink). For example, we could have a separate `Configuration` member variable in TableConfig for user-defined properties for clear separation. What do you think?
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
