rdblue commented on a change in pull request #1893:
URL: https://github.com/apache/iceberg/pull/1893#discussion_r557016887
##########
File path:
flink/src/test/java/org/apache/iceberg/flink/TestFlinkTableSource.java
##########
@@ -75,32 +95,436 @@ public void clean() {
@Test
public void testLimitPushDown() {
- sql("INSERT INTO %s VALUES (1,'a'),(2,'b')", TABLE_NAME);
-
String querySql = String.format("SELECT * FROM %s LIMIT 1", TABLE_NAME);
String explain = getTableEnv().explainSql(querySql);
String expectedExplain = "LimitPushDown : 1";
- Assert.assertTrue("explain should contains LimitPushDown",
explain.contains(expectedExplain));
+ assertTrue("explain should contains LimitPushDown",
explain.contains(expectedExplain));
List<Object[]> result = sql(querySql);
- Assert.assertEquals("should have 1 record", 1, result.size());
- Assert.assertArrayEquals("Should produce the expected records",
result.get(0), new Object[] {1, "a"});
+ assertEquals("should have 1 record", 1, result.size());
+ assertArrayEquals("Should produce the expected records", result.get(0),
new Object[] {1, "a"});
AssertHelpers.assertThrows("Invalid limit number: -1 ",
SqlParserException.class,
() -> sql("SELECT * FROM %s LIMIT -1", TABLE_NAME));
- Assert.assertEquals("should have 0 record", 0, sql("SELECT * FROM %s LIMIT
0", TABLE_NAME).size());
+ assertEquals("should have 0 record", 0, sql("SELECT * FROM %s LIMIT 0",
TABLE_NAME).size());
- String sqlLimitExceed = String.format("SELECT * FROM %s LIMIT 3",
TABLE_NAME);
+ String sqlLimitExceed = String.format("SELECT * FROM %s LIMIT 4",
TABLE_NAME);
List<Object[]> resultExceed = sql(sqlLimitExceed);
- Assert.assertEquals("should have 2 record", 2, resultExceed.size());
+ assertEquals("should have 3 record", 3, resultExceed.size());
List<Object[]> expectedList = Lists.newArrayList();
expectedList.add(new Object[] {1, "a"});
expectedList.add(new Object[] {2, "b"});
- Assert.assertArrayEquals("Should produce the expected records",
resultExceed.toArray(), expectedList.toArray());
+ expectedList.add(new Object[] {3, null});
+ assertArrayEquals("Should produce the expected records",
resultExceed.toArray(), expectedList.toArray());
String sqlMixed = String.format("SELECT * FROM %s WHERE id = 1 LIMIT 2",
TABLE_NAME);
List<Object[]> mixedResult = sql(sqlMixed);
- Assert.assertEquals("should have 1 record", 1, mixedResult.size());
- Assert.assertArrayEquals("Should produce the expected records",
mixedResult.get(0), new Object[] {1, "a"});
+ assertEquals("should have 1 record", 1, mixedResult.size());
+ assertArrayEquals("Should produce the expected records",
mixedResult.get(0), new Object[] {1, "a"});
+ }
+
+ @Test
+ public void testNoFilterPushDown() {
+ String sql = String.format("SELECT * FROM %s ", TABLE_NAME);
+ String explain = getTableEnv().explainSql(sql);
+ assertFalse("explain should no contains FilterPushDown",
explain.contains(expectedFilterPushDownExplain));
+ }
+
+ @Test
+ public void testFilterPushDownEqual() {
+ String sqlLiteralRight = String.format("SELECT * FROM %s WHERE id = 1 ",
TABLE_NAME);
+ String explain = getTableEnv().explainSql(sqlLiteralRight);
+ String explainExpected = "FilterPushDown,the filters :ref(name=\"id\") ==
1]]], fields=[id, data])";
+ assertTrue("explain should contains FilterPushDown",
explain.contains(explainExpected));
+
+ List<Object[]> result = sql(sqlLiteralRight);
+ assertEquals("should have 1 record", 1, result.size());
+ assertArrayEquals("Should produce the expected record", result.get(0), new
Object[] {1, "a"});
+
+ assertEquals("Should create only one scan", 1, scanEventCount);
+ assertEquals("Should push down expected filter", "id = 1",
ExpressionsUtil.describe(lastScanEvent.filter()));
+
+ // filter not push down
+ String sqlEqualNull = String.format("SELECT * FROM %s WHERE data = NULL ",
TABLE_NAME);
+ String explainEqualNull = getTableEnv().explainSql(sqlEqualNull);
+ assertFalse("explain should not contains FilterPushDown",
explainEqualNull.contains(expectedFilterPushDownExplain));
+ }
+
+ @Test
+ public void testFilterPushDownEqualLiteralOnLeft() {
+ String sqlLiteralLeft = String.format("SELECT * FROM %s WHERE 1 = id ",
TABLE_NAME);
+ String explainLeft = getTableEnv().explainSql(sqlLiteralLeft);
+ String explainExpected = "FilterPushDown,the filters :ref(name=\"id\") ==
1]]], fields=[id, data])";
+ assertTrue("explain should contains FilterPushDown",
explainLeft.contains(explainExpected));
+
+ List<Object[]> resultLeft = sql(sqlLiteralLeft);
+ assertEquals("should have 1 record", 1, resultLeft.size());
+ assertArrayEquals("Should produce the expected record", resultLeft.get(0),
new Object[] {1, "a"});
+
+ assertEquals("Should create only one scan", 1, scanEventCount);
+ assertEquals("Should push down expected filter", "id = 1",
ExpressionsUtil.describe(lastScanEvent.filter()));
+ }
+
+ @Test
+ public void testFilterPushDownNoEqual() {
+ String sqlNE = String.format("SELECT * FROM %s WHERE id <> 1 ",
TABLE_NAME);
+ String explainNE = getTableEnv().explainSql(sqlNE);
+ String explainExpected = "FilterPushDown,the filters :ref(name=\"id\") !=
1]]], fields=[id, data])";
+ assertTrue("explain should contains FilterPushDown",
explainNE.contains(explainExpected));
+
+ List<Object[]> resultNE = sql(sqlNE);
+ assertEquals("should have 2 record", 2, resultNE.size());
+
+ List<Object[]> expectedNE = Lists.newArrayList();
+ expectedNE.add(new Object[] {2, "b"});
+ expectedNE.add(new Object[] {3, null});
+ assertArrayEquals("Should produce the expected record",
resultNE.toArray(), expectedNE.toArray());
+ assertEquals("Should create only one scan", 1, scanEventCount);
+ assertEquals("Should push down expected filter", "id != 1",
ExpressionsUtil.describe(lastScanEvent.filter()));
+
+ String sqlNotEqualNull = String.format("SELECT * FROM %s WHERE data <>
NULL ", TABLE_NAME);
+ String explainNotEqualNull = getTableEnv().explainSql(sqlNotEqualNull);
+ assertFalse("explain should not contains FilterPushDown",
explainNotEqualNull.contains(
+ expectedFilterPushDownExplain));
+ }
+
+ @Test
+ public void testFilterPushDownAnd() {
+ String sqlAnd = String.format("SELECT * FROM %s WHERE id = 1 AND data =
'a' ", TABLE_NAME);
+ String explainAnd = getTableEnv().explainSql(sqlAnd);
+ String explainExpected =
+ "FilterPushDown,the filters :ref(name=\"id\") == 1,ref(name=\"data\")
== \"a\"]]], fields=[id, data])";
+ assertTrue("explain should contains FilterPushDown",
explainAnd.contains(explainExpected));
+
+ List<Object[]> resultAnd = sql(sqlAnd);
+ assertEquals("should have 1 record", 1, resultAnd.size());
+ assertArrayEquals("Should produce the expected record", resultAnd.get(0),
new Object[] {1, "a"});
+
+ assertEquals("Should create only one scan", 1, scanEventCount);
+ assertEquals("Should push down expected filter", "(id = 1 AND data = 'a')",
+ ExpressionsUtil.describe(lastScanEvent.filter()));
+ }
+
+ @Test
+ public void testFilterPushDownOr() {
+ String sqlOr = String.format("SELECT * FROM %s WHERE id = 1 OR data = 'b'
", TABLE_NAME);
+ String explainOr = getTableEnv().explainSql(sqlOr);
+ String explainExpected =
+ "FilterPushDown,the filters :(ref(name=\"id\") == 1 or
ref(name=\"data\") == \"b\")]]], fields=[id, data])";
+ assertTrue("explain should contains FilterPushDown",
explainOr.contains(explainExpected));
+
+ List<Object[]> resultOr = sql(sqlOr);
+ assertEquals("should have 2 record", 2, resultOr.size());
+
+ List<Object[]> expectedOR = Lists.newArrayList();
+ expectedOR.add(new Object[] {1, "a"});
+ expectedOR.add(new Object[] {2, "b"});
+ assertArrayEquals("Should produce the expected record",
resultOr.toArray(), expectedOR.toArray());
+
+ assertEquals("Should create only one scan", 1, scanEventCount);
+ assertEquals("Should push down expected filter", "(id = 1 OR data = 'b')",
+ ExpressionsUtil.describe(lastScanEvent.filter()));
Review comment:
It looks like `describe` was moved just for tests. I don't think that
was needed. If you need to produce a string from an expression, why not just
use `toString`? That's what other assertions use, like the one for
`explainExpected` above.
Can you try reverting the `describe` change?
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
