Add support for INSERT OVERWRITE #116
Conversation
|
Thanks for the fix @becketqin. Won't we have similar issues with other DDL commands like |
|
Comment referring to |
| @@ -231,7 +238,7 @@ private static ExpectedResultTable readRowsFromFile(String path, boolean hasHead | |||
| } | |||
| } | |||
|
|
|||
| interface SerializableDynamicTableSink extends DynamicTableSink, Serializable { | |||
| interface SerializableDynamicTableSink extends DynamicTableSink, SupportsOverwrite, Serializable { | |||
There was a problem hiding this comment.
Should the SerializableDynamicTableSink also implement SupportsPartitioning to allow for partitioning?
There was a problem hiding this comment.
The purpose of the test is to make sure we allow all the statements starting with INSERT. So as long as both INSERT INTO and INSERT OVERWRITE got passed to the TableEnv, the logic we want to test is verified. Therefore, we don't need to cover all the possible syntaxes of INSERT because SqlPTransform does not deal with them except pass them through.
becketqin
force-pushed
the
sql_ptransform_support_insert_overwrite
branch
3 times, most recently
from
200393d
to
1e301e0
Compare
In our offline conversation, we finally concluded that we will add support for all the DDLs ( |
|
Had a discussion with @venkata91 offline again. What is not supported with this patch is the non-computing DDL only statements. After checking the code a bit more, I found supporting that is somewhat changing the design principle of the Flink runner. After all, the non-computing statements has nothing to do with Beam. Users should probably just use TableEnv to execute the statements, instead of run the statements through the Beam pipeline which is a detour. So we will keep the patch as is. |
becketqin
force-pushed
the
sql_ptransform_support_insert_overwrite
branch
from
1e301e0
to
ef27b46
Compare
Agreed! We can revisit this if needed in the future. |
The patch simply added support for INSERT OVERWRITE.
We should also support CREATE TABLE AS SELECT (CTAS). However, the
org.apache.flink.table.api.bridge.java.StreamStatementSetdoes not support CTAS yet. We need to make change to the OSS Flink first.