[GitHub] [flink] wangxlong commented on issue #10769: [FLINK-15479]Override explainSource method for JDBCTableSource

[GitBox <gi...@apache.org>]

wangxlong commented on issue #10769: [FLINK-15479]Override explainSource method for JDBCTableSource
URL: https://github.com/apache/flink/pull/10769#issuecomment-571025175
 
 
   Thank you for your advice@wuchong @godfreyhe @JingsongLi . It is my duty to add a jdbc project push down test to verify. I am ok to fix it in 1.11.
   From my side, there are two  aspects need to be discussed:
   1、How to fix it. I agree with @godfreyhe, we should fix the default implementation of explainSource in TableSource, just like follow:
   `		List<String> fieldNames = ((RowType) getProducedDataType().getLogicalType()).getFieldNames();
   		return TableConnectorUtils.generateRuntimeName(
   			getClass(),
   			fieldNames.stream().toArray(String[]::new));`
   2、How to add a test to verify it.
   I found there is a test to verify jdbc push down in JDBCTableSourceSinkFactoryTest#testJDBCWithFilter. I think the name of method maybe not good. We can change it to testJDBCFieldsProjection. On the basis of this test, we can add some code to verify source description just like follow:
   `		List<String> fieldNames = ((RowType) actual.getProducedDataType().getLogicalType()).getFieldNames();
   		String expectedSourceDescription = actual.getClass().getSimpleName()
   			+ "(" + String.join(", ", fieldNames.stream().toArray(String[]::new)) + ")";
   		assertEquals(expectedSourceDescription ,actual.explainSource());`
   Thank you all, I also comment in the relative jira. Looking forward to your reply.
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services