Revisit all TODO in AbstractKuduConnectorTest #11815
Comments
|
Is anybody working on this? If not can I take it? |
|
@chen-ni No one is working on this as far as I know. Please go for it. |
|
@ebyhr OK, thanks! |
|
@ebyhr Please correct me if I'm wrong:
|
|
@ebyhr I'm working on the
On one hand, we can indeed create table without specifying default partitions in this way, like: https://github.com/trinodb/trino/blob/master/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java#L260 // AbstractKuduConnectorTest.java
@Test
@Override
public void testExplainAnalyzeWithDeleteWithSubquery()
{
String tableName = "test_delete_" + randomTableSuffix();
// note that default partitions are not specified here
assertUpdate("CREATE TABLE " + tableName + " AS SELECT * FROM nation", 25);
assertExplainAnalyze("EXPLAIN ANALYZE DELETE FROM " + tableName + " WHERE regionkey IN (SELECT regionkey FROM region WHERE name LIKE 'A%' LIMIT 1)",
"SemiJoin.*");
assertUpdate("DROP TABLE " + tableName);
}On the other hand, the // AbstractKuduConnectorTest.java
@Override
public void testInsert()
{
// TODO Support these test once kudu connector can create tables with default partitions
throw new SkipException("TODO");
}But the // BaseConnectorTest.java
@Test
public void testInsert()
{
// ...
String query = "SELECT phone, custkey, acctbal FROM customer";
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_insert_", "AS " + query + " WITH NO DATA")) {
// ...
}
}So it doesn't make sense to me to skip the test because of "default partitions". I've also tried explicitly specifying default partitions while executing The test does fail, though. If I remove the overriding test and run the Which brings to my second question:
|
The connector creates a hidden
It's a bug. #12915 will fix the issue. |
There're more than 10 skipped tests in Kudu with below comment, but it would be better to add tests in connector specific way. Otherwise, it's hard to detect regression likes #11814. After supporting default partitions, we can just remove overridden tests.
The text was updated successfully, but these errors were encountered: