[FLINK-37902] batch support for ml_predict#27310
Conversation
airlock-confluentinc
bot
force-pushed
the
batch-ml-predict
branch
from
794dcda to
3a23af4
Compare
fsk119
left a comment
fsk119
left a comment
There was a problem hiding this comment.
Thanks for your contribution. I left some comments
| FlinkLogicalTableFunctionScan.class, | ||
| FlinkConventions.LOGICAL(), | ||
| FlinkConventions.BATCH_PHYSICAL(), | ||
| "PhysicalMLPredictTableFunctionRule:Batch")); |
There was a problem hiding this comment.
how about using the same name as vector search? BatchPhysicalMLPredictTableFunctionRule.
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.flink.table.planner.runtime.batch.table; |
There was a problem hiding this comment.
it's better we move this class to org.apache.flink.table.planner.runtime.batch.sql. Here is used for table api tests.
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.flink.table.planner.runtime.batch.table; |
|
|
||
| private void createScanTable(String tableName, List<Row> data) { | ||
| String dataId = TestValuesTableFactory.registerData(data); | ||
| String bounded = tEnv instanceof StreamExecutionEnvironment ? "false" : "true"; |
There was a problem hiding this comment.
tEnv.getConfig().get(ExecutionOptions.RUNTIME_MODE) == RuntimeExecutionMode.BATCH
| } | ||
|
|
||
| @Parameters(name = "backend = {0}, objectReuse = {1}, asyncOutputMode = {2}") | ||
| public static Collection<Object[]> parameters() { |
There was a problem hiding this comment.
Before change, we run tests with different statebackend. I prefer to keep the origin behaviour.
There was a problem hiding this comment.
I'm following VectorSearchITCaseBase which doesn't have these. Probably because there's no BatchWithStateTestBase
There was a problem hiding this comment.
In batch mode, operator doesn't need state 0.0
| @Override | ||
| public List<TableTestProgram> programs() { | ||
| return List.of( | ||
| SYNC_ML_PREDICT, ASYNC_UNORDERED_ML_PREDICT, SYNC_ML_PREDICT_WITH_RUNTIME_CONFIG); |
There was a problem hiding this comment.
Why don't add SYNC_ML_PREDICT_TABLE_API, SYNC_ML_PREDICT_TABLE_API...
There was a problem hiding this comment.
These are in semantic test. I'll add a batch semantic test
airlock-confluentinc
bot
force-pushed
the
batch-ml-predict
branch
from
3a23af4 to
06eacc5
Compare
3 participants
What is the purpose of the change
Add batch support for ml_predict function
Brief change log
Verifying this change
unit/it/restore test for stream/batch
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation